Skip to content

Commit

Permalink
fix: Remove default_server listen option from fallback server
Browse files Browse the repository at this point in the history
  • Loading branch information
rhansen committed Apr 7, 2023
1 parent 7322a2d commit c58186c
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 4 deletions.
17 changes: 13 additions & 4 deletions nginx.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,15 @@ proxy_set_header Proxy "";
{{- /*
* If needed, create a catch-all fallback server to send an error code to
* clients that request something from an unknown vhost.
*
* This server must appear first in the generated config because nginx uses
* the first `server` directive to handle requests that don't match any of
* the other `server` directives. An alternative approach would be to add
* the `default_server` option to the `listen` directives inside this
* `server`, but some users inject a custom `server` directive that uses
* `default_server`. Using `default_server` here would cause nginx to fail
* to start for those users. See
* <https://github.com/nginx-proxy/nginx-proxy/issues/2212>.
*/}}
{{- block "fallback_server" $globals }}
{{- $globals := . }}
Expand Down Expand Up @@ -403,15 +412,15 @@ server {
server_name _; # This is just an invalid value which will never trigger on a real hostname.
server_tokens off;
{{- if $fallback_http }}
listen {{ $globals.external_http_port }} default_server;
listen {{ $globals.external_http_port }}; {{- /* Do not add `default_server` (see comment above). */}}
{{- if $globals.enable_ipv6 }}
listen [::]:{{ $globals.external_http_port }} default_server;
listen [::]:{{ $globals.external_http_port }}; {{- /* Do not add `default_server` (see comment above). */}}
{{- end }}
{{- end }}
{{- if $fallback_https }}
listen {{ $globals.external_https_port }} ssl http2 default_server;
listen {{ $globals.external_https_port }} ssl http2; {{- /* Do not add `default_server` (see comment above). */}}
{{- if $globals.enable_ipv6 }}
listen [::]:{{ $globals.external_https_port }} ssl http2 default_server;
listen [::]:{{ $globals.external_https_port }} ssl http2; {{- /* Do not add `default_server` (see comment above). */}}
{{- end }}
{{- end }}
{{ $globals.access_log }}
Expand Down
5 changes: 5 additions & 0 deletions test/test_fallback.data/custom-fallback.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
server {
server_name __;
listen 80 default_server;
return 418;
}
14 changes: 14 additions & 0 deletions test/test_fallback.data/custom-fallback.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
services:
sut:
image: nginxproxy/nginx-proxy:test
volumes:
- /var/run/docker.sock:/tmp/docker.sock:ro
- ./custom-fallback.conf:/etc/nginx/conf.d/zzz-custom-fallback.conf:ro
http-only:
image: web
expose:
- "83"
environment:
WEB_PORTS: "83"
VIRTUAL_HOST: http-only.nginx-proxy.test
HTTPS_METHOD: nohttps
5 changes: 5 additions & 0 deletions test/test_fallback.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ def _get(url):
("nohttps-on-app.yml", "https://http-only.nginx-proxy.test/", None, CONNECTION_REFUSED_RE),
("nohttps-on-app.yml", "http://unknown.nginx-proxy.test/", 503, None),
("nohttps-on-app.yml", "https://unknown.nginx-proxy.test/", None, CONNECTION_REFUSED_RE),
# Custom nginx config that has a `server` directive that uses `default_server` and simply
# returns 418. Nginx should successfully start (in particular, the `default_server` in the
# custom config should not conflict with the fallback server generated by nginx-proxy) and nginx
# should prefer that server for handling requests for unknown vhosts.
("custom-fallback.yml", "http://unknown.nginx-proxy.text/", 418, None),
])
def test_fallback(get, url, want_code, want_err_re):
if want_err_re is None:
Expand Down

0 comments on commit c58186c

Please sign in to comment.