Fixing 404s on subdomains with dashes on it #692
Conversation
| @@ -327,7 +327,7 @@ protected function get_service_nginx() : array { | |||
| 'traefik.port=8080', | |||
| 'traefik.protocol=https', | |||
| 'traefik.docker.network=proxy', | |||
| "traefik.frontend.rule=HostRegexp:{$this->hostname},{subdomain:[a-z.-_]+}.{$this->hostname}{$domains}", | |||
| "traefik.frontend.rule=HostRegexp:{$this->hostname},{subdomain:[A-Za-z0-9-_]+}.{$this->hostname}{$domains}", | |||
There was a problem hiding this comment.
I think this fix has some unintended consequences.
The original regex [a-z.-_] is quite confusing, but it is two groups: a through z and . through _. If you look at the ASCII table between . and _ (positions 46-95) you'll see all numbers, uppercase latin letters and a bunch of symbols as well, including the . and _, but excluding the - symbol, which is at position 45.
I'm not sure why the original regex is so permissive in the first place:
php > var_dump( preg_match( '#^[a-z.-_]+$#', 'what/a.weird_sub^domain' ) );
int(1)
And yet fails to match:
php > var_dump( preg_match( '#^[a-z.-_]+$#', 'perfectly-legit' ) );
int(0)
While much more restrictive, the proposed newer version fails to match the dot symbol:
php > var_dump( preg_match( '#^[A-Za-z0-9-_]+$#', 'foo.bar' ) );
int(0)
This might be useful for deeper-level subdomains (language subdomains on a Multisite install for example). It also still matches the underscore symbol, which is not valid for hostnames/subdomains. It also looks like case sensitivity is handled by the HostRegexp implementation itself traefik/traefik#3930
There was a problem hiding this comment.
I think the original regex is accidental, and it should have been [a-z.\-_]+, but the escaping was missed.
There was a problem hiding this comment.
Good catch with the case sensitive implementation and I totally forgot to include the dot, this one [a-z.\-]+ works but it doesn't match numbers, do you guys think I should use [a-z0-9.\-] (removing the underscore) instead? It's common to see some people that add numbers on testing sites like test-site-1.domain.com, etc. @rmccue @kovshenin
Fixed the regex to allow dashes on subdomains, previous regex didn't march them. This was reported on issue #265