hoaproject / Router

The Hoa\Router library.
https://hoa-project.net/
28 stars 11 forks source link

use http_host when its defined #29

Closed thehawk970 closed 9 years ago

thehawk970 commented 9 years ago

When i use subdomain on hoa like : '(?<user>.*)@/' i use it on an nginx configuration on default configuration and SERVER_NAME has not mention of subdomain you have the export below :

Array
(
    [USER] => www-data
    [HOME] => /var/www
    [FCGI_ROLE] => RESPONDER
    [QUERY_STRING] => q=
    [REQUEST_METHOD] => GET
    [CONTENT_TYPE] => 
    [CONTENT_LENGTH] => 
    [SCRIPT_FILENAME] => /maestria-web/Public/index.php
    [SCRIPT_NAME] => /index.php
    [REQUEST_URI] => /
    [DOCUMENT_URI] => /index.php
    [DOCUMENT_ROOT] => /maestria-web/Public
    [SERVER_PROTOCOL] => HTTP/1.1
    [GATEWAY_INTERFACE] => CGI/1.1
    [SERVER_SOFTWARE] => nginx/1.4.6
    [REMOTE_ADDR] => 10.0.2.2
    [REMOTE_PORT] => 59023
    [SERVER_ADDR] => 10.0.2.15
    [SERVER_PORT] => 80
    [SERVER_NAME] => maestria.dev
    [REDIRECT_STATUS] => 200
    [HTTP_HOST] => foo.maestria.dev:8080
    [HTTP_USER_AGENT] => Mozilla/5.0 (Windows NT 6.3; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0
    [HTTP_ACCEPT] => text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
    [HTTP_ACCEPT_LANGUAGE] => fr-FR,fr;q=0.8,en-US;q=0.5,en;q=0.3
    [HTTP_ACCEPT_ENCODING] => gzip, deflate
    [HTTP_DNT] => 1
    [HTTP_CONNECTION] => keep-alive
    [PHP_SELF] => /index.php
    [REQUEST_TIME_FLOAT] => 1426860307.3234
    [REQUEST_TIME] => 1426860307
)

The current code getDomain did't have subdomain mention and don't able to route the subdomain information, and my patch work correctly on my vm. I think i don't introduce an BC-BREAK but i did't have an apache for test it.

Hywan commented 9 years ago

First, we should read this: http://stackoverflow.com/questions/2297403/http-host-vs-server-name. Second, I wonder if we should use an elseif instead of an if here, because SERVER_NAME will erase HTTP_HOST. I think the opposite behavior is expected.

I assign @Pierozi on this one. Asking @hoaproject/hoackers feedbacks too.

Pierozi commented 9 years ago

back to this commit have make the BC-BREAK https://github.com/hoaproject/Router/commit/1c626f079689ad80eaad885e7142f499a2b772e2

@Hywan why have you switch to SERVER_NAME instead of HTTP_HOST ?

Acutally, for both Apache & Nginx, SERVER_NAME environement variable are alaways first match in settings.

# $_SERVER['SERVER_NAME'] === "sub.foo.app"
server_name     sub.foo.app other.foo.app;

# $_SERVER['SERVER_NAME'] === "foo.app"
server_name     foo.app bar.foo.app;

# $_SERVER['SERVER_NAME'] === "*.foo.app"
server_name     *.foo.app foo.app;
Hywan commented 9 years ago

Why first match? I switched to SERVER_NAME because in some cases HTTP_HOST was not correct.

Pierozi commented 9 years ago

@Hywan Don't know why first match, but even if you use only one wildcard match, SERVER_NAME will be *.foo.app instead of sub.foo.app expected.

thehawk970 commented 9 years ago

I think its depend on server configuration ... maybe introduce an parameter somewhere for use it no ? router.domain.match = 'http_host;server_name no ?

Pierozi commented 9 years ago

@camael24 we can't trust SERVER_NAME because is multiple as server side, and unique in php. is good only if what do you expect are in first place.

That should be good to have the @Hywan case of not correct.

We need determined which issue are more often. But with SERVER_NAME, all wildcard definition can't work. It's already big deal.

Hywan commented 9 years ago

Ok so we should go back to HTTP_HOST and then see if the issue happens again :-p?

thehawk970 commented 9 years ago

@Hywan : J'ai rectifié le elseif tu me diras si c'est bon que je rebase le tout :)

Hywan commented 9 years ago

I think we should even remove the SERVER_NAME support.

thehawk970 commented 9 years ago

Done :dancer:

Pierozi commented 9 years ago

And use empty instead isset or just set $domain = $_SERVER['HTTP_HOST']; like it was before no ?

Hywan commented 9 years ago

@camael24 According to what @Pierozi is saying, we should not use SERVER_NAME.

thehawk970 commented 9 years ago

HTTP_HOST are always defined in the array, i.e we never get an error if the key not exist ?

Hywan commented 9 years ago

@camael24 I don't know. We have to check in all cases.

Hywan commented 9 years ago

@camael24 Also check if the HTTP_HOST contains the port or not.

thehawk970 commented 9 years ago

If its follow the spec yes it could (sauf si c'est le port par défaut) :dancer: http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.23

Hywan commented 9 years ago

@camael24 ping?

Hywan commented 9 years ago

@Pierozi ping :-)? With @osaris we don't know what to do with this PR: Do we close it?

Hywan commented 9 years ago

Rebased and merged into https://github.com/hoaproject/Router/commit/9f4fb3669a3802f0993b8a2fe9943de842a62fe0.

Hywan commented 9 years ago

Released in https://github.com/hoaproject/Router/releases/tag/2.15.08.03.