From 62202f93070b6e2c95b5575963f39658dafaaa8e Mon Sep 17 00:00:00 2001 From: junderw Date: Thu, 19 Aug 2021 07:29:39 +0900 Subject: [PATCH 1/2] Redirect using 308 for non-GET requests --- README.md | 1 + nginx.tmpl | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 779affa..78af1b4 100644 --- a/README.md +++ b/README.md @@ -292,6 +292,7 @@ Note that the `Mozilla-Old` policy should use a 1024 bits DH key for compatibili The default behavior for the proxy when port 80 and 443 are exposed is as follows: * If a container has a usable cert, port 80 will redirect to 443 for that container so that HTTPS is always preferred when available. + * This redirect will use a 301 code for GET requests and 308 code for any other http method (POST/HEAD/PUT etc.). A 308 redirect is a more recent version of 301 permanent redirect that maintains the http method. Previously, 301 redirects would all be converted into GET requests. * If the container does not have a usable cert, a 503 will be returned. Note that in the latter case, a browser may get an connection error as no certificate is available to establish a connection. A self-signed or generic cert named `default.crt` and `default.key` will allow a client browser to make a SSL connection (likely w/ a warning) and subsequently receive a 500. diff --git a/nginx.tmpl b/nginx.tmpl index 6df80f6..834638a 100644 --- a/nginx.tmpl +++ b/nginx.tmpl @@ -276,11 +276,20 @@ server { } location / { - {{ if eq $external_https_port "443" }} - return 301 https://$host$request_uri; - {{ else }} - return 301 https://$host:{{ $external_https_port }}$request_uri; - {{ end }} + if ($request_method = GET) { + {{ if eq $external_https_port "443" }} + return 301 https://$host$request_uri; + {{ else }} + return 301 https://$host:{{ $external_https_port }}$request_uri; + {{ end }} + } + if ($request_method != GET) { + {{ if eq $external_https_port "443" }} + return 308 https://$host$request_uri; + {{ else }} + return 308 https://$host:{{ $external_https_port }}$request_uri; + {{ end }} + } } } {{ end }} From 160068d27bba7bcf775ad37700037c273e3a6725 Mon Sep 17 00:00:00 2001 From: junderw Date: Thu, 19 Aug 2021 16:47:44 +0900 Subject: [PATCH 2/2] Add tests --- test/test_ssl/test_redirect.py | 34 +++++++++++++++++++++++++++++++++ test/test_ssl/test_redirect.yml | 14 ++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 test/test_ssl/test_redirect.py create mode 100644 test/test_ssl/test_redirect.yml diff --git a/test/test_ssl/test_redirect.py b/test/test_ssl/test_redirect.py new file mode 100644 index 0000000..6fcabad --- /dev/null +++ b/test/test_ssl/test_redirect.py @@ -0,0 +1,34 @@ +import pytest + +# These tests are to test that GET is 301 and other methods all use 308 +# Permanent Redirects +# https://github.com/nginx-proxy/nginx-proxy/pull/1737 +def test_web1_GET_301(docker_compose, nginxproxy): + r = nginxproxy.get('http://web1.nginx-proxy.tld', allow_redirects=False) + assert r.status_code == 301 + assert r.headers['Location'] == 'https://web1.nginx-proxy.tld/' + +def test_web1_POST_308(docker_compose, nginxproxy): + r = nginxproxy.post('http://web1.nginx-proxy.tld', allow_redirects=False) + assert r.status_code == 308 + assert r.headers['Location'] == 'https://web1.nginx-proxy.tld/' + +def test_web1_PUT_308(docker_compose, nginxproxy): + r = nginxproxy.put('http://web1.nginx-proxy.tld', allow_redirects=False) + assert r.status_code == 308 + assert r.headers['Location'] == 'https://web1.nginx-proxy.tld/' + +def test_web1_HEAD_308(docker_compose, nginxproxy): + r = nginxproxy.head('http://web1.nginx-proxy.tld', allow_redirects=False) + assert r.status_code == 308 + assert r.headers['Location'] == 'https://web1.nginx-proxy.tld/' + +def test_web1_DELETE_308(docker_compose, nginxproxy): + r = nginxproxy.delete('http://web1.nginx-proxy.tld', allow_redirects=False) + assert r.status_code == 308 + assert r.headers['Location'] == 'https://web1.nginx-proxy.tld/' + +def test_web1_OPTIONS_308(docker_compose, nginxproxy): + r = nginxproxy.options('http://web1.nginx-proxy.tld', allow_redirects=False) + assert r.status_code == 308 + assert r.headers['Location'] == 'https://web1.nginx-proxy.tld/' diff --git a/test/test_ssl/test_redirect.yml b/test/test_ssl/test_redirect.yml new file mode 100644 index 0000000..62694bd --- /dev/null +++ b/test/test_ssl/test_redirect.yml @@ -0,0 +1,14 @@ +web1: + image: web + expose: + - "81" + environment: + WEB_PORTS: "81" + VIRTUAL_HOST: "web1.nginx-proxy.tld" + +sut: + image: nginxproxy/nginx-proxy:test + volumes: + - /var/run/docker.sock:/tmp/docker.sock:ro + - ../lib/ssl/dhparam.pem:/etc/nginx/dhparam/dhparam.pem:ro + - ./certs:/etc/nginx/certs:ro