search

zendframework / zend-uri

Uri component from Zend Framework
BSD 3-Clause "New" or "Revised" License
117 stars 24 forks source link

make host lowercase #29

Closed samsonasik closed 5 years ago

samsonasik commented 5 years ago

Based on https://tools.ietf.org/html/rfc3986#section-3.2.2 , the host subcomponent is case-insensitive, so the "www.EXAMPLE.com" is same with "www.example.com". I think it should be normalized to lowercase within Uri::setHost() call.

samsonasik commented 5 years ago

Is there anything I can do to get it merged? Thank you.

weierophinney commented 5 years ago

I'm not sure what this change buys us.

Case insensitivity here simply means that, for purposes of equivalency, different cases are the same. It does not mean that there MUST be a single representation of the host name.

Casting it to lower case has some ramifications, particularly when we consider unicode domain names, as calling strtolower() on those may actually cause breakage. Additionally, when it comes to logging, changing the case may make matching the URI to one logged by the web server harder.

As such, I'm closing this at this point. If developers need to do URI hostname comparisons, they should ensure they use case-insensitive comparison operations.

samsonasik commented 5 years ago

If we check Zend\Diactoros\Uri, there is strtolower against host, is concistency something can be considered?

weierophinney commented 5 years ago

If we check Zend\Diactoros\Uri, there is strtolower against host, is concistency something can be considered?

Hmmm... I swear I checked that yesterday, but somehow I must have missed the strtolower() call (it's at line 318 for those who are curious).

Re-opening. I still maintain it could be considered a BC break, but we'll do another minor release to call out the change, as it does make it more consistent.

weierophinney commented 5 years ago

Thanks, @samsonasik! Merged to develop for an immediate 2.7.0 release, with a CHANGELOG entry calling out the change in behavior.

bmxmale commented 5 years ago

@weierophinney you was right with breakage. Simple case from Magento 2.3. Host is not always domain or IP address. When we using docker we can call host via container names.

Example:

host = MAGENTO2_varnish

When we call strtolower then we getting error:

[2019-03-12 11:10:23] main.CRITICAL: Unable to connect to magento2_varnish:80 . Error #0: stream_socket_client(): unable to connect to magento2_varnish:80 (php_network_getaddresses: getaddrinfo failed: Name or service not known) {"method":"GET","url":"http:/","invalidateInfo":{"server":"[object] (Zend\\Uri\\Uri: http://magento2_varnish:80/)","tagsPattern":".*"}} []

So MAGENTO2_varnish != magento2_varnish.

If anyone ( like me ) using aliases on magento then this patch break eg. Varnish Purge request.