Skip to content

Commit 658f5e7

Browse files
committed
Fix proxy full redirect
1 parent 40d73da commit 658f5e7

9 files changed

Lines changed: 168 additions & 10 deletions

File tree

nginx_proxy/Host.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ def __init__(self, hostname: str, port: int, scheme=None):
2828
self.scheme: set = scheme
2929
self.secured: bool = "https" in scheme or "wss" in scheme
3030
self.full_redirect: Union[Url, None] = None
31+
self.is_redirect: bool = False
3132
self.extras: Dict[str, Any] = {}
3233

3334
def set_external_parameters(self, host, port) -> None:

nginx_proxy/WebServer.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,14 @@ def _ensure_https_redirects(self, hosts: List[Host]) -> List[Host]:
9696
http_hosts = {(host.hostname, int(host.port)): host for host in hosts if int(host.port) == 80}
9797

9898
for host in hosts:
99-
if not host.secured or int(host.port) == 80:
99+
if host.is_redirect or not host.secured or int(host.port) == 80:
100100
continue
101101
redirect_target = Url({"https"}, host.hostname, int(host.port), "/")
102102
http_host = http_hosts.get((host.hostname, 80))
103103
if http_host is None:
104104
redirect_host = Host(host.hostname, 80)
105105
redirect_host.full_redirect = redirect_target
106+
redirect_host.update_extras_content("redirect_status_code", "308")
106107
# Added after redirect post-processing, so mark it explicitly for template rendering.
107108
redirect_host.is_redirect = True
108109
redirect_hosts.append(redirect_host)

nginx_proxy/post_processors/redirect_processor.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ def process_redirection(self, config: ProxyConfigData):
1010
for host in config.host_list():
1111
if host.isredirect():
1212
redirected_hosts[host.hostname] = host.full_redirect
13-
target = config.getHost(host.full_redirect.hostname)
13+
target = config.getHost(host.full_redirect.hostname, host.full_redirect.port)
1414
if target is not None:
1515
if target.hostname == host.hostname:
1616
host.full_redirect = None

nginx_proxy/pre_processors/redirect_processor.py

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,25 @@ def process_redirection(backend: BackendTarget, environments: map, vhost_map: Di
2323
if len(split) == 2:
2424
_sources, target = split
2525
sources = [Url.parse(source) for source in _sources.split(",")]
26-
target = Url.parse(target, default_port=80)
27-
if single_host:
28-
if target.hostname is None:
29-
target = single_host
26+
target = Url.parse(target)
27+
if target.hostname is None and single_host:
28+
target.hostname = hosts[0].hostname
3029
elif target.hostname is None:
3130
print("Unknown target to redirect with PROXY_FULL_REDIRECT" + redirect_entry)
3231
continue
32+
target.port = int(target.port) if target.port is not None else None
33+
if target.port is None and target.hostname in vhost_map:
34+
target_host = vhost_map[target.hostname].get(443) or vhost_map[target.hostname].get(80)
35+
if target_host is not None:
36+
target.port = target_host.port
37+
target.scheme = {"https"} if target_host.secured else {"http"}
38+
if target.port is None:
39+
target.port = 443 if "https" in target.scheme or "wss" in target.scheme else 80
40+
if not target.scheme:
41+
target.scheme = {"https"} if target.port == 443 else {"http"}
3342
for source in sources:
3443
if source.hostname is not None:
35-
port = 80 if source.port is None else source.port
44+
port = 80 if source.port is None else int(source.port)
3645
if source.hostname not in vhost_map:
3746
host = Host(source.hostname, port)
3847
host.full_redirect = target

tests/integration/conftest.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,6 @@ def nginx_proxy_container(docker_client: docker.DockerClient, test_network, dock
132132
name=container_name,
133133
environment={
134134
"LETSENCRYPT_API": "https://acme-staging-v02.api.letsencrypt.org/directory",
135-
"DHPARAM_SIZE": "256",
136135
"VHOSTS_TEMPLATE_DIR": "/app/vhosts_template",
137136
"CHALLENGE_DIR": "/etc/nginx/acme-challenges",
138137
"DOCKER_SWARM": swarm_mode,

tests/integration/test_nginx_proxy.py

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,68 @@ def test_http_to_https_redirect_preserves_query_string(nginx_request, docker_cli
200200
stop_backend(backend)
201201

202202

203+
@pytest.mark.parametrize("backend_type", ["container", "service"])
204+
def test_proxy_full_redirect_to_https_target_response(
205+
nginx_request,
206+
docker_client,
207+
test_network,
208+
swarm_mode,
209+
backend_type,
210+
request,
211+
):
212+
"""
213+
Test that PROXY_FULL_REDIRECT returns a 301 to the existing HTTPS target
214+
while preserving the original request URI.
215+
"""
216+
if not is_reachable(swarm_mode, backend_type):
217+
pytest.skip("Backend discovery not available for this swarm mode/backend type combination.")
218+
219+
suffix = f"{backend_type}.{swarm_mode}.{datetime.now(timezone.utc).strftime('%H%M%S%f')}"
220+
target_host = f"{suffix}.full-redirect-target.example.com"
221+
source_host = f"{suffix}.full-redirect-source.example.com"
222+
env = {
223+
"VIRTUAL_HOST": f"https://{target_host} -> :8080",
224+
"VIRTUAL_PORT": "8080",
225+
"PROXY_FULL_REDIRECT": f"{source_host} -> {target_host}",
226+
}
227+
backend = None
228+
229+
try:
230+
backend = start_backend(
231+
docker_client,
232+
test_network,
233+
env,
234+
backend_type=backend_type,
235+
pytest_request=request,
236+
sleep=False,
237+
)
238+
239+
request_uri = "/v2/_catalog?n=50&last=abc"
240+
url = f"http://{source_host}{request_uri}"
241+
242+
response = None
243+
ex = None
244+
for _ in range(20):
245+
try:
246+
ex = None
247+
response = nginx_request.get(url, timeout=2, allow_redirects=False)
248+
if response.status_code == 301:
249+
break
250+
except (KeyboardInterrupt, SystemExit):
251+
raise
252+
except Exception as e:
253+
ex = e
254+
time.sleep(1)
255+
256+
assert ex is None
257+
assert response is not None
258+
assert response.status_code == 301
259+
assert response.headers.get("Location") == f"https://{target_host}{request_uri}"
260+
finally:
261+
if backend:
262+
stop_backend(backend)
263+
264+
203265
@pytest.mark.parametrize("backend_type", ["container", "service"])
204266
@pytest.mark.parametrize(
205267
"virtual_host_base, request_path",

tests/integration/test_webserver_events.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,55 @@ def test_webserver_add_container_with_ssl_integration(
352352
stop_backend(backend)
353353

354354

355+
@pytest.mark.parametrize("backend_type", ["container", "service"])
356+
def test_proxy_full_redirect_to_https_target_integration(
357+
nginx_proxy_container: docker.models.containers.Container,
358+
docker_client: docker.DockerClient,
359+
test_network: docker.models.networks.Network,
360+
backend_type: str,
361+
):
362+
"""
363+
Test that PROXY_FULL_REDIRECT resolves a bare target to the existing HTTPS vhost
364+
and renders a documented 301 redirect without appending :80.
365+
"""
366+
suffix = uuid.uuid4().hex[:6]
367+
target_host = f"{backend_type}.full-redirect-target-{suffix}.example.com"
368+
source_host = f"{backend_type}.full-redirect-source-{suffix}.example.com"
369+
env = {
370+
"VIRTUAL_HOST": f"https://{target_host} -> :8080",
371+
"VIRTUAL_PORT": "8080",
372+
"PROXY_FULL_REDIRECT": f"{source_host} -> {target_host}",
373+
}
374+
375+
backend = start_backend(docker_client, test_network, env, backend_type=backend_type, sleep=False)
376+
try:
377+
redirect_server = None
378+
for _ in range(25):
379+
config_str = get_nginx_config_from_container(nginx_proxy_container[0])
380+
config = HttpBlock.parse(config_str)
381+
redirect_server = next((s for s in config.servers if source_host in s.server_names), None)
382+
if redirect_server is not None:
383+
redirect_loc = next((loc for loc in redirect_server.locations if loc.path == "/"), None)
384+
if redirect_loc and redirect_loc.return_code == f"301 https://{target_host}$request_uri":
385+
break
386+
time.sleep(1)
387+
388+
config_str = get_nginx_config_from_container(nginx_proxy_container[0])
389+
config = HttpBlock.parse(config_str)
390+
target_servers = [s for s in config.servers if target_host in s.server_names]
391+
source_servers = [s for s in config.servers if source_host in s.server_names]
392+
393+
assert any("443" in s.listen for s in target_servers), f"HTTPS target server not found. Config:\n{config_str}"
394+
assert len(source_servers) == 1, f"Expected one redirect source server. Config:\n{config_str}"
395+
396+
redirect_loc = next((loc for loc in source_servers[0].locations if loc.path == "/"), None)
397+
assert redirect_loc is not None, f"Redirect location not found. Config:\n{config_str}"
398+
assert redirect_loc.return_code == f"301 https://{target_host}$request_uri"
399+
finally:
400+
if backend:
401+
stop_backend(backend)
402+
403+
355404
@pytest.mark.parametrize("backend_type", ["container", "service"])
356405
def test_webserver_add_two_containers_with_same_virtual_host_integration(
357406
nginx_proxy_container,

tests/unit/test_webserver_events.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,43 @@ def test_webserver_add_container_with_ssl(docker_client: DockerTestClient, nginx
412412
assert http_redirect_location.return_code == "308 https://ssl.example.com$request_uri"
413413

414414

415+
def test_proxy_full_redirect_uses_existing_https_target(docker_client: DockerTestClient, nginx: DummyNginx):
416+
target_hostname = "redirect-target.example.com"
417+
source_hostname = "redirect-source.example.com"
418+
alternate_source = "redirect-source-www.example.com"
419+
env = {
420+
"VIRTUAL_HOST": f"https://{target_hostname}",
421+
"PROXY_FULL_REDIRECT": f"{source_hostname},{alternate_source} -> {target_hostname}",
422+
}
423+
424+
docker_client.containers.run("nginx:alpine", name="full_redirect_https", environment=env, network="frontend")
425+
426+
target_servers = expect_servers(nginx, target_hostname, 2)
427+
assert next((s for s in target_servers if "443" in s.listen), None) is not None
428+
429+
for hostname in (source_hostname, alternate_source):
430+
redirect_server = expect_server(nginx, hostname)
431+
redirect_location = next((l for l in redirect_server.locations if l.path == "/"), None)
432+
assert redirect_location is not None
433+
assert redirect_location.return_code == f"301 https://{target_hostname}$request_uri"
434+
435+
436+
def test_proxy_full_redirect_preserves_http_target_scheme(docker_client: DockerTestClient, nginx: DummyNginx):
437+
target_hostname = "redirect-http-target.example.com"
438+
source_hostname = "redirect-http-source.example.com"
439+
env = {
440+
"VIRTUAL_HOST": target_hostname,
441+
"PROXY_FULL_REDIRECT": f"{source_hostname} -> {target_hostname}",
442+
}
443+
444+
docker_client.containers.run("nginx:alpine", name="full_redirect_http", environment=env, network="frontend")
445+
446+
redirect_server = expect_servers(nginx, source_hostname, 1)[0]
447+
redirect_location = next((l for l in redirect_server.locations if l.path == "/"), None)
448+
assert redirect_location is not None
449+
assert redirect_location.return_code == f"301 http://{target_hostname}$request_uri"
450+
451+
415452
def test_webserver_ssl_does_not_override_explicit_http_location(docker_client: DockerTestClient, nginx: DummyNginx):
416453
container_name = "ssl_http_container"
417454
hostname = "ssl-http.example.com"

vhosts_template/default.conf.jinja2

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,8 @@ server {
8484
alias {{ config.challenge_dir }};
8585
try_files $uri =404;{% endif %}
8686
}
87-
location / { {% if server.is_redirect %}
88-
return 308 https://{{ server.full_redirect.hostname }}{% if server.full_redirect.port and server.full_redirect.port != 443 %}:{{ server.full_redirect.port }}{% endif %}$request_uri;{% else %}
87+
location / { {% if server.is_redirect %}{% set redirect_scheme = "https" if "https" in server.full_redirect.scheme or "wss" in server.full_redirect.scheme else "http" %}
88+
return {{ server.extras.redirect_status_code if server.extras.redirect_status_code else "301" }} {{ redirect_scheme }}://{{ server.full_redirect.hostname }}{% if server.full_redirect.port and ((redirect_scheme == "https" and server.full_redirect.port != 443) or (redirect_scheme == "http" and server.full_redirect.port != 80)) %}:{{ server.full_redirect.port }}{% endif %}$request_uri;{% else %}
8989
return 308 https://$host$request_uri;{% endif %}
9090
}
9191
}{% endif %}{% endfor %}

0 commit comments

Comments
 (0)