Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_checkForHttpHostError does not check for $config->httpHosts #1180

Open
noelboss opened this issue May 26, 2020 · 9 comments
Open

_checkForHttpHostError does not check for $config->httpHosts #1180

noelboss opened this issue May 26, 2020 · 9 comments

Comments

@noelboss
Copy link

_checkForHttpHostError() function in wire/core/admin.php does only check for $config->httpHost and does not check for $config->httpHosts yet the error message states
'Please update your $config->httpHosts setting in /site/config.php'

Something does not add up. Either _checkForHttpHostError() should check for $config->httpHosts or the error message should read differently.

IMHO, the two settings very similar and not immediately clear what the differences are and whar …

@ryancramerdesign
Copy link
Member

@noelboss The $config->httpHosts is a whitelist of allowed hosts. Whatever is in $config->httpHost is a value from the httpHosts (plural) that was found to be consistent with what's in $_SERVER['HTTP_HOST']. Or, if the current HTTP_HOST doesn't exist in the config->httpHosts setting, then it'll use the first one in the config->httpHosts setting as the config->httpHost. Keep in mind that HTTP_HOST comes from user input (request HTTP headers), so you can't just use it—you have to validate it against a whitelist.

The error message is telling you how to fix it, so you'll want to do what it says. You are seeing that error because your $config->httpHosts setting lacks the host that you are using your site at, and so you need to update the setting in your /site/config.php file, per the instructions in the error message.

The other possibility is that you've manually set the $config->httpHost somewhere, like your /site/config.php file, and it's got the wrong host name in it. If that's the case, you should remove that and instead update your $config->httpHosts array to have the correct hostname(s).

@noelboss
Copy link
Author

noelboss commented Jun 4, 2020

Our site is publicly accessible on ourdomain.com but runs behind a proxy backend.our-proxy.com

I have configured the following:

$config->httpHost = 'ourdomain.com';
$config->httpHosts = [
	'ourdomain.com',
	'backend.our-proxy.com',
];

But when I login, I get the error message

Unrecognized HTTP host:'backend.our-proxy.com' - Please update your $config->httpHosts setting in /site/config.php - read more

As I said, when generating this message, _checkForHttpHostError() never even looks at $config->httpHosts … So unless I'm misunderstanding something here, I still think it's a bug… 🤷

https://github.com/processwire/processwire/blob/51629cdd5f381d3881133baf83e1bd2d9306f867/wire/core/admin.php#L49

@teppokoivula
Copy link

teppokoivula commented Jun 5, 2020

@noelboss, ProcessWire populates $config->httpHost earlier in the boot process, so that's not the problem — but it gets confused here since you've manually defined both httpHost and httpHosts.

Just drop the httpHost (singular) setting and everything should work as expected :)

@noelboss
Copy link
Author

noelboss commented Jun 10, 2020

@teppokoivula No, it does not work as expected. When I add only

$config->httpHosts = [
	'ourdomain.com',
	'backend.our-proxy.com',
];

I end up having backend.our-proxy.com urls in the frontend when using $page->httpUrl for links

httpUrl links are only correct when I add $config->httpHost = 'ourdomain.com'; manually and then I get the error message even if I have configured it as above…¨

it'll use the first one in the config->httpHosts setting as the config->httpHost

This seems to not be true, but instead it seems to use $_SERVER['HTTP_HOST'] – that one would be backend.our-proxy.com

@teppokoivula
Copy link

Right — my bad! My earlier suggestion wasn't quite right for your use case.

If the site is requested (behind the scenes) using backend.our-proxy.com, ProcessWire will indeed automatically choose that one as current httpHost (see ProcessWire::getHttpHost()), and this in turn means that $page->httpUrl and possibly other similar methods use this for the host part.

On the other hand if ProcessWire doesn't find current host from the httpHosts array, it will pick the first one instead — but this will also result in a warning (_checkForHttpHostError()).

If I got your setup right, it seems that you're referring to your site (publicly) with one domain, and requesting it behind the scenes (via a proxy) with another. If so, I'm not sure how to resolve this at the moment; if there's support for this in the core, I'm not aware of it.

For your use case being able to specify both httpHosts and httpHost manually would indeed seem like a viable solution, but as you've noticed, that's currently not supported — though I'm not sure if there are any other problems with it other than the error message in admin.

Whether this could be supported is definitely a question to @ryancramerdesign. From my point of view it seems like a sensible thing, and checking httpHosts in _checkForHttpHostError() also seems like a pretty safe change, but Ryan knows best :)

@ryancramerdesign
Copy link
Member

@noelboss @teppokoivula If I understand your case correctly, in your environment PHP/Apache doesn't know the actual hostname you want to use, so ProcessWire doesn't have data to compare against. I've not seen a setup like that before, but I think you are going to have to tell PHP manually by setting $_SERVER['HTTP_HOST']. I think you can make it work with your environment by adding this to your /site/config.php file:

$config->httpHost = 'ourdomain.com';
$config->httpHosts = [$config->httpHost]; 
$_SERVER['HTTP_HOST'] = $config->httpHost;

@noelboss
Copy link
Author

That's exactly what I did to make it work – but shouldn't that work too:

$config->httpHost = 'ourdomain.com';
$config->httpHosts = [
	'ourdomain.com',
	'backend.our-proxy.com',
];

I don't see any reason why this should not work based on the doc's and the error description. I specifically set $config->httpHost for PW to know what host to use…

I'll leave it up to you :) thanks for your time…

@ryancramerdesign
Copy link
Member

@noelboss The purpose of that bit of code in _checkForHttpHost error is to save you in situations where the site has moved from one server to another (like dev or staging to live, changing domain entirely, or running from multiple domains). So the only way it can really do that well is if it has some environment data to compare to (that doesn't come from PW), like your $_SERVER[HTTP_HOST] or $_SERVER[SERVER_NAME]. If it lets a match between a $config->httpHost and $config->httpHosts bypass the error, then the site gets migrated, including the config.php with wrong host settings, and no error is ever rendered. So it silently runs at the wrong host for months, rendering URLs, sending emails with the wrong host, etc. until someone finally discovers it. In that situation, it's like the _checkForHttpHost() serves no purpose.

On the other hand, this is the first case I've come across where there apparently isn't any way to get the actual HTTP host from the server's environmental data. But if that's not an isolated situation, then we would want to add additional logic to detect it, or update the docs on how to get around it. I'm wondering if it's possible your actual HTTP host is still in the server's environment data, but just maybe in else. Do you see the correct HTTP host anywhere if you execute a php script like this that displays all the $_SERVER and $_ENV data?

test.php

<pre><?php 
echo htmlentities(print_r($_SERVER, true)); 
echo htmlentities(print_r($_ENV, true)); 

@daniel-fahl
Copy link

daniel-fahl commented Nov 17, 2020

Wouldn't it be an option to check the X_FORWARDED_HOST header if it exists, and use that to match against the httpHosts array instead of the HTTP_HOST? That way a setup running behind a proxy could still make it transparent for ProcessWire which host is actually accessed by the user.

I'd also really like that solution - overriding a users header data (HTTP_HOST in this case) just doesn't really feel right, does it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants