Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redirect non-GET methods using 308 instead of 301 #1737

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
19 changes: 14 additions & 5 deletions nginx.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
34 changes: 34 additions & 0 deletions test/test_ssl/test_redirect.py
Original file line number Diff line number Diff line change
@@ -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/'
14 changes: 14 additions & 0 deletions test/test_ssl/test_redirect.yml
Original file line number Diff line number Diff line change
@@ -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