Problem/Motivation
Follow up from #2221699: HTTP_HOST header cannot be trusted.
If a developer uncomments the trusted_host_patterns
sections in default.settings.php
and uses a different set in settings.local.php
, the ones in settings.local.php
will be ignored.
This is because the local include should be last in the file:
Keep this code block at the end of this file to take full effect.
Proposed resolution
Move the local settings include to the bottom of default.settings.php
Remaining tasks
None.
User interface changes
None.
API changes
None.
Beta phase evaluation
Issue category | Bug |
---|---|
Issue priority | Normal |
Unfrozen changes | No |
Prioritized changes | Yes. This improves security. |
Disruption | None. |
Comment | File | Size | Author |
---|---|---|---|
#1 | trusted-host-2419225-1.patch | 1.62 KB | mikeker |
Comments
Comment #1
mikeker CreditAttribution: mikeker commentedAttached patch pushes the
settings.local.php
processing to the bottom ofdefault.settings.php
.Comment #2
mikeker CreditAttribution: mikeker commentedDuh...
Comment #3
mikeker CreditAttribution: mikeker commentedComment #4
dawehnerUrgs ... I wonder whether we can test this to ensure it doesn't happen again?
Comment #5
mpdonadio#2416563: Follow-up to "HTTP_HOST header cannot be trusted" removed trusted_host_patterns from sites/example.settings.local.php.
Updated IS to reflect true bug...
Comment #6
mikeker CreditAttribution: mikeker commented@dawehner: I suppose we could use a regex to test that the local settings file directive is at the end of
default.settings.php
. I'm concerned that would be a pretty brittle test, though it is for rarely changed code...Thoughts?
Comment #8
webchickHm. Yeah, I can't really think of a useful test that would work even if something else completely different was appended to the bottom next time. And we might do just as well with making the last line of the file "# No, seriously, don't ever put anything else at the bottom of this file." ;)
For now, I'm comfortable considering this a one-off since the documentation does say that, and just wasn't followed in that issue. If it happens again, maybe we can add some capital letters and ASCII snowmen or whatever to the docs, too. ;)
Committed and pushed to 8.0.x. Thanks!
Comment #9
mikeker CreditAttribution: mikeker commentedSomething like this? :)
Comment #10
webchickLOL :)