Skip to content

Commit 79c06bc

Browse files
jacalataclaude
andcommitted
Preserve target port on http -> https promotion
The same-host check correctly allowed cross-scheme redirects, but the new_address was built by stripping "http://" off the old address, so the target's port was silently dropped. Enterprise on-prem installs that run HTTPS on a non-default port (e.g. 8443) ended up with a bogus stored address after the first redirect. Build new_address from the redirect target's hostname + port instead. Adds regression tests covering explicit target port, default-port normalization, and the unrelated-host non-promotion case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 65df951 commit 79c06bc

2 files changed

Lines changed: 63 additions & 1 deletion

File tree

tableauserverclient/server/endpoint/endpoint.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,15 @@ def _hostport(parsed):
254254
return ((parsed.hostname or "").lower(), parsed.port or default_http_port)
255255

256256
if old_parsed.scheme == "http" and _hostport(old_parsed) == _hostport(current_parsed):
257-
new_address = "https://" + old_address[len("http://") :]
257+
# Build new_address from the redirect target's netloc so the
258+
# target's port survives. Stripping "http://" off old_address
259+
# (its predecessor) silently dropped the target port and
260+
# broke enterprise on-prem deployments that run HTTPS on a
261+
# non-default port (e.g. 8443). Normalize an explicit 443
262+
# away since it is the HTTPS default.
263+
next_port = next_parsed.port
264+
port_suffix = f":{next_port}" if next_port and next_port != 443 else ""
265+
new_address = f"https://{next_host}{port_suffix}"
258266
self.parent_srv._server_address = new_address
259267
logger.info(f"Server redirected to HTTPS; updated server address to {new_address}")
260268
# Auth-material policy: the request `parameters` (headers, body,

test/test_redirect_handling.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,60 @@ def test_http_to_https_upgrade_promotes_stored_server_address(server: TSC.Server
269269
assert server._server_address == "https://test"
270270

271271

272+
def test_http_to_https_upgrade_preserves_explicit_target_port(server: TSC.Server) -> None:
273+
# Enterprise on-prem installs commonly run HTTPS on a non-default port
274+
# (e.g. 8443). If the redirect target carries an explicit port, the
275+
# promoted server address must keep it -- otherwise every subsequent
276+
# request goes to :443 and fails.
277+
assert server._server_address == "http://test"
278+
xml = _sign_in_xml()
279+
with requests_mock.mock() as m:
280+
m.post(
281+
server.auth.baseurl + "/signin",
282+
status_code=301,
283+
headers={"Location": "https://test:8443/api/3.6/auth/signin"},
284+
)
285+
m.post("https://test:8443/api/3.6/auth/signin", text=xml)
286+
server.auth.sign_in(TSC.TableauAuth("u", "p"))
287+
assert server._server_address == "https://test:8443"
288+
289+
290+
def test_http_to_https_upgrade_normalizes_default_ports() -> None:
291+
# http://host:80 -> https://host:443 with both ports at their scheme
292+
# defaults should collapse to "https://host" (no port suffix), matching
293+
# how a user would type it.
294+
s = TSC.Server("http://test:80", False)
295+
assert s._server_address == "http://test:80"
296+
xml = _sign_in_xml()
297+
with requests_mock.mock() as m:
298+
m.post(
299+
s.auth.baseurl + "/signin",
300+
status_code=301,
301+
headers={"Location": "https://test:443/api/3.6/auth/signin"},
302+
)
303+
m.post("https://test:443/api/3.6/auth/signin", text=xml)
304+
s.auth.sign_in(TSC.TableauAuth("u", "p"))
305+
assert s._server_address == "https://test"
306+
307+
308+
def test_http_to_https_upgrade_does_not_promote_to_different_host_with_port(server: TSC.Server) -> None:
309+
# Cross-host redirect: even to an https:8443 endpoint, do NOT rewrite
310+
# the stored server address. Same non-promotion contract as the
311+
# port-less different-host case; guards against a scenario where the
312+
# port-preserving fix accidentally widens the same-host check.
313+
assert server._server_address == "http://test"
314+
xml = _sign_in_xml()
315+
with requests_mock.mock() as m:
316+
m.post(
317+
server.auth.baseurl + "/signin",
318+
status_code=301,
319+
headers={"Location": "https://other-host:8443/api/3.6/auth/signin"},
320+
)
321+
m.post("https://other-host:8443/api/3.6/auth/signin", text=xml)
322+
server.auth.sign_in(TSC.TableauAuth("u", "p"))
323+
assert server._server_address == "http://test"
324+
325+
272326
def test_http_to_https_upgrade_does_not_promote_on_different_host(server: TSC.Server) -> None:
273327
# If the redirect target is on a different host, do NOT rewrite the stored
274328
# server address -- the redirect might be to a completely unrelated server

0 commit comments

Comments
 (0)