Silverstripe-framework: BASE_URL / Director.alternate_base_url issues & inconsistency

[kinglozzer]

Affects 3.6.3. See also https://github.com/silverstripe/silverstripe-framework/issues/5109.

Issue 1:

Director:
  alternate_base_url: 'http://mysite.test'

This results in “Undefined index: path” here: https://github.com/silverstripe/silverstripe-framework/blob/3.6.3/control/Session.php#L358

Adding a trailing slash fixes the issue. I can’t find any documentation for Director.alternate_base_url which indicates whether or not a trailing slash should be present.

Issue 2:

define('BASE_URL', 'http://mysite.test');

Visiting http://mysite.test?flush results in a redirection to the wrong URL: http://mysite.test/http:/mysite.test/?flush=&flushtoken=1234

This is because ParameterConfirmationToken treats the BASE_URL constant as a relative path, not an absolute URL. BASE_URL appears to be treated like a relative path in most cases, though the code docs for it suggest that it should be an absolute URL. The code to calculate BASE_PATH if it’s not defined will also always return a directory name.

Issue 3:

BASE_URL is used as a fallback for Director.alternate_base_url when calling Director::baseURL(), so they should be consistent. Adding a trailing slash to BASE_URL results in double-slashes in URLs, and removing the trailing slash from Director.alternate_base_url results in issue 1 above.

Suggested action:

To save a massive refactor to support relative/absolute urls and trailing/non-trailing slashes for both, I’m advocating mostly documentation changes and simple patches.

• Update the code docs for BASE_URL to indicate that it should be a relative path to the document root from the domain name (with no trailing slash);
• Add documentation for Director.alternate_base_url to indicate that a trailing slash should be present;
• Bonus round: patch Director::baseURL() to force a trailing slash

[dhensby]

BASE_URL and alternative_base_url have been pretty inconsistent and all round terrible for a while. It's part of the reason I originally had SS_HOST in SS 4 to remove this weird ambiguity around whether these vars should be absolute or not. see #6337

[whereisaaron]

This is still as issue because although we have SS_BASE_URL in SS4, this setting is only a fallback. So there still appears no .env setting to tell SS the correct protocol, domain, and path to use for the BASE tag.

SS seldom gets the BASE tag correct because it doesn't know about load balancers, CDNs, and WAFs that are terminating TLS and/or proxying to an origin domain name and/or rewriting the base path. There ought to be a .env setting that can actually override the SS's, frequently incorrect, guess at the protocol and/or domain name.

[whereisaaron]

This was our standard fix for SS3, we included this for every site to fix the protocol and domain in the base href, without also breaking the cookie path.

Config::inst()->update('Director', 'alternate_base_url', (rtrim(getenv('WEBSITE_URL'), '/') . '/'));
// Need to set cookie_path to dodge a bug in inst_start() in framework/control/Session.php
Config::inst()->update('Session', 'cookie_path', '/');

[dhensby]

@whereisaaron this is possible behind load balancers and other layers that terminate SSL - you just have to make sure that your load balancer / proxy is correctly whitelisted with SilverStripe and that the correct X-Forwarded-* headers are passed along

[dhensby]

That doesn't change the fact that the base tag is a fallback, that's correct. This is because using it as a source of truth will break other things like forcing www / https as well as figuring out the actual domain name being used.

If you don't want to have SS_BASE_URL as a fallback you should be using Director.alternate_base_url in the config (as you are) though you'd want to set this in yaml for performance reasons and not PHP.

[whereisaaron]

Thanks @dhensby I can see you want this to offer the force www/https/domain features, but in our stack we don't see that as the application's role. That is all handled externally before requests get to the application containers. We'd prefer the configuration to be able to be the source of truth 😄

Thanks for the confirmation that Director.alternate_base_url is the best workaround. AFAIK the SS YAML syntax doesn't let you reference environment variables? And you need to use PHP for that? We could inject a whole YAML config file at run time, but an environment variable would be preferable. It would be handy if there was SS_ALTERNATE_BASE_URL environment setting so we could easily inject the correct base URL into the containers at launch.

[dhensby]

Well, yes, I agree, I don't see it as the application's roll either, but whilst those features exist they have to work.

The problem with making SS_BASE_URL the source of truth it that it can lead to problems when accessing the site from another domain or protocol (eg: If I access over https but the SS_BASE_URL is http, then the base tag would be written with http causing mixed content issues).

In SS4 I believe you can use env vars to populate config by wrapping them in back ticks. see https://github.com/silverstripe/silverstripe-framework/blob/4.0.0/src/Control/Director.php#L96

[tractorcow]

Alternate_base_url is a relic intended only for testing to override constants. Maybe since we are using environmental vars now we should encourage this instead of this config.

[whereisaaron]

Thanks @dhensby, does the back tick syntax also work in SS4 YAML config? Does the syntax work any @config item, or only some special ones? I couldn't spot any documentation for the back tick syntax?

[kinglozzer]

The back tick syntax currently only works for Injector service definitions (https://docs.silverstripe.org/en/4/developer_guides/extending/injector/#using-constants-as-variables): Director.default_base_url is a one-off special case where we convert the backticks “manually”

[dhensby]

ah :(

[robbieaverill]

This came up internally again today, I think we need to support it for all config rather than just in injector definitions

[michalkleiner]

Came across this now with the SSL termination on a proxy. The 4.1.1 version doesn't seem to respect the X-Forwarded-* headers.

[dhensby]

Came across this now with the SSL termination on a proxy. The 4.1.1 version doesn't seem to respect the X-Forwarded-* headers.

have you set up trusted proxies? see https://docs.silverstripe.org/en/4/developer_guides/security/secure_coding/#request-hostname-forgery

[michalkleiner]

Ah, no, I haven't - need to look at that one. Thanks!

[kinglozzer]

This won’t be fixed for SS3

[whereisaaron]

Hi @kinglozzer, this is still an open issue for SS4. Although the original report was for SS3, it still hasn't been fixed in SS4 either.

[warslett]

I'm having a nightmare today launching our first site since making the switch to SS4. Our sites sit behind an nginx proxy which does the SSL stuff so Silverstripe thinks it is using http when to the world it is using https. This causes lots of obvious problems which we used to get around in SS3 with alternate_protocol. The introduction of environment variables is a step in the right direction but here is the bit I can't understand:

# vendor/silverstripe/framework/src/includes/constants.php
104         if ($base) {
105 
106             // Strip relative path from SS_BASE_URL
107             return rtrim(parse_url($base, PHP_URL_PATH), '/');
108         }

So even though we explicity declare the correct SS_BASE_URL (something like https://example.com) silverstripe passes it to parse_url and extracts only the path. This means that BASE_URL is always defined as an empty string which means when SS goes to generate the base tag it has to fall back to it's old way of trying to figure it out itself which doesn't work. I can't understand why we are setting the BASE_URL to the url path. Surely a BASE_URL includes the full URL including scheme and host? Am I missing something?

[warslett]

this is my ugly hack in mysite/_config.php

    Director::config()->set('alternate_base_url', Environment::getEnv('SS_BASE_URL'));

This has at least got me live today

[robbieaverill]

Hi @warslett,

You need to pass through a X-Forwarded-Host request header from nginx when using it as a reverse proxy.

See docs: https://docs.silverstripe.org/en/4/developer_guides/security/secure_coding/#request-hostname-forgery

[warslett]

Thank you @robbieaverill we will look into that. Any idea as to why SS is stripping the host and protocol out of the BASE_URL in constants.php?