lots of random tests and debugging to try and figure out whats going on

batpad · batpad · commit 2d37c3ec4943 · 2025-09-30T17:01:03.000+05:30
diff --git a/src/stac_auth_proxy/middleware/ProcessLinksMiddleware.py b/src/stac_auth_proxy/middleware/ProcessLinksMiddleware.py
@@ -45,6 +45,12 @@ def transform_json(self, data: dict[str, Any], request: Request) -> dict[str, An
         req_base_url = get_base_url(request)
         parsed_req_url = urlparse(req_base_url)
         parsed_upstream_url = urlparse(self.upstream_url)
+        
+        # DEBUG: Log URL processing
+        logger.info(f"DEBUG ProcessLinks: req_base_url = {req_base_url}")
+        logger.info(f"DEBUG ProcessLinks: parsed_req_url.netloc = {parsed_req_url.netloc}")
+        logger.info(f"DEBUG ProcessLinks: upstream_url = {self.upstream_url}")
+        logger.info(f"DEBUG ProcessLinks: parsed_upstream_url.netloc = {parsed_upstream_url.netloc}")
 
         for link in get_links(data):
             try:
@@ -95,6 +101,8 @@ def _update_link(
 
         # Replace the upstream host with the client's host
         if parsed_link.netloc == upstream_url.netloc:
+            logger.info(f"DEBUG _update_link: Original link href = {link['href']}")
+            logger.info(f"DEBUG _update_link: Replacing netloc {parsed_link.netloc} with {request_url.netloc}")
             parsed_link = parsed_link._replace(netloc=request_url.netloc)._replace(
                 scheme=request_url.scheme
             )
@@ -118,4 +126,6 @@ def _update_link(
             urlunparse(parsed_link),
         )
 
-        link["href"] = urlunparse(parsed_link)
+        new_href = urlunparse(parsed_link)
+        logger.info(f"DEBUG _update_link: Final transformed href = {new_href}")
+        link["href"] = new_href
diff --git a/src/stac_auth_proxy/utils/requests.py b/src/stac_auth_proxy/utils/requests.py
@@ -189,16 +189,27 @@ def get_base_url(request: Request) -> str:
     forwarded_proto = request.headers.get("X-Forwarded-Proto")
     forwarded_path = request.headers.get("X-Forwarded-Path")
 
+    # DEBUG: Log request details
+    logger.info(f"DEBUG get_base_url: request.url = {request.url}")
+    logger.info(f"DEBUG get_base_url: request.url.netloc = {request.url.netloc}")
+    logger.info(f"DEBUG get_base_url: Host header = {request.headers.get('host')}")
+    logger.info(f"DEBUG get_base_url: X-Forwarded-Host = {forwarded_host}")
+    logger.info(f"DEBUG get_base_url: X-Forwarded-Proto = {forwarded_proto}")
+
     if forwarded_host:
         # Use forwarded headers to reconstruct the original client URL
         scheme = forwarded_proto or request.url.scheme
         netloc = forwarded_host
         # Use forwarded path if available, otherwise use request base URL path
         path = forwarded_path or request.base_url.path
+        logger.info(f"DEBUG get_base_url: Using forwarded headers - netloc = {netloc}")
     else:
         # Fall back to the request's base URL if no forwarded headers
         scheme = request.url.scheme
         netloc = request.url.netloc
         path = request.base_url.path
+        logger.info(f"DEBUG get_base_url: Using request URL - netloc = {netloc}")
 
-    return f"{scheme}://{netloc}{path}"
+    result = f"{scheme}://{netloc}{path}"
+    logger.info(f"DEBUG get_base_url: Final result = {result}")
+    return result
diff --git a/tests/test_process_links.py b/tests/test_process_links.py
@@ -639,3 +639,160 @@ def test_transform_with_forwarded_headers(headers, expected_base_url):
     # but not include the forwarded path in the response URLs
     assert transformed["links"][0]["href"] == f"{expected_base_url}/proxy/collections"
     assert transformed["links"][1]["href"] == f"{expected_base_url}/proxy"
+
+
+@pytest.mark.parametrize(
+    "upstream_url,request_host,request_port,scheme,input_links,expected_links",
+    [
+        # Test case 1: Non-standard port (should work correctly)
+        (
+            "http://montandon-eoapi-stac:8080",
+            "stac-test.whydidweevendothis.com",
+            1234,
+            "http",
+            [
+                {"rel": "self", "href": "http://montandon-eoapi-stac:8080/collections"},
+            ],
+            [
+                "http://stac-test.whydidweevendothis.com:1234/collections",
+            ],
+        ),
+        # Test case 2: Standard port HTTP (80) - BUG: should NOT include upstream port
+        (
+            "http://montandon-eoapi-stac:8080",
+            "stac-test.whydidweevendothis.com",
+            80,
+            "http",
+            [
+                {"rel": "self", "href": "http://montandon-eoapi-stac:8080/collections"},
+            ],
+            [
+                "http://stac-test.whydidweevendothis.com/collections",  # Should NOT have :8080
+            ],
+        ),
+        # Test case 3: Standard port HTTPS (443) - BUG: should NOT include upstream port
+        (
+            "http://montandon-eoapi-stac:8080",
+            "stac-test.whydidweevendothis.com",
+            443,
+            "https",
+            [
+                {"rel": "self", "href": "http://montandon-eoapi-stac:8080/collections"},
+            ],
+            [
+                "https://stac-test.whydidweevendothis.com/collections",  # Should NOT have :8080
+            ],
+        ),
+        # Test case 4: Standard port but NO server info (simulate real-world missing port)
+        (
+            "http://montandon-eoapi-stac:8080",
+            "stac-test.whydidweevendothis.com",
+            None,  # No port specified - simulate real-world scenario
+            "https",
+            [
+                {"rel": "self", "href": "http://montandon-eoapi-stac:8080/collections"},
+            ],
+            [
+                "https://stac-test.whydidweevendothis.com/collections",  # Should NOT have :8080
+            ],
+        ),
+    ],
+)
+def test_transform_upstream_links_standard_ports_bug(
+    upstream_url, request_host, request_port, scheme, input_links, expected_links
+):
+    """Test that demonstrates the port handling bug with standard ports.
+    
+    This test demonstrates the issue where standard ports (80, 443) cause
+    the upstream port to appear in the final URLs incorrectly.
+    """
+    middleware = ProcessLinksMiddleware(
+        app=None, upstream_url=upstream_url, root_path=None
+    )
+    
+    # Simulate request coming through standard port (80 or 443)
+    # For standard ports, browsers don't include port in Host header and ASGI scope
+    if request_port is None:
+        # Simulate real-world scenario where port is not specified anywhere
+        host_header = request_host  # No port in Host header
+        server_info = (request_host, 443)  # Server might default to 443 for HTTPS
+    elif request_port in [80, 443]:
+        host_header = request_host  # No port in Host header for standard ports
+        server_info = (request_host, request_port)  # But server info might be available
+    else:
+        host_header = f"{request_host}:{request_port}"  # Non-standard ports include port
+        server_info = (request_host, request_port)
+    
+    request_scope = {
+        "type": "http",
+        "scheme": scheme,
+        "path": "/test",
+        "headers": [
+            (b"host", host_header.encode()),
+            (b"content-type", b"application/json"),
+        ],
+        "server": server_info,
+    }
+
+    data = {"links": input_links}
+    transformed = middleware.transform_json(data, Request(request_scope))
+
+    for i, expected in enumerate(expected_links):
+        actual = transformed["links"][i]["href"]
+        print(f"Test case: port {request_port}")
+        print(f"Expected: {expected}")
+        print(f"Actual:   {actual}")
+        assert actual == expected, f"Port {request_port} test failed: expected {expected}, got {actual}"
+
+
+def test_transform_upstream_links_no_host_port_edge_case():
+    """Test the specific edge case that causes the bug.
+    
+    This test simulates what happens when:
+    1. Browser accesses http://localhost (no port in Host header)
+    2. Server is running on port 8000 internally
+    3. Upstream URL has port 8080
+    4. The bug: final URLs incorrectly show :8080 instead of no port
+    """
+    middleware = ProcessLinksMiddleware(
+        app=None, 
+        upstream_url="http://montandon-eoapi-stac:8080", 
+        root_path=None
+    )
+    
+    # Simulate the exact scenario that fails:
+    # - Host header: just "localhost" (no port)
+    # - Server port: 8000 (internal container port)
+    # - Scheme: http
+    # - Expected result: http://localhost/collections (no port)
+    # - Bug: gets http://localhost:8080/collections (upstream port!)
+    request_scope = {
+        "type": "http",
+        "scheme": "http",
+        "path": "/collections",
+        "query_string": b"",
+        "headers": [
+            (b"host", b"localhost"),  # NO PORT in host header
+            (b"content-type", b"application/json"),
+        ],
+        "server": ("127.0.0.1", 8000),  # Internal server port
+    }
+
+    # Test with a link from upstream that has port 8080
+    data = {
+        "links": [
+            {"rel": "self", "href": "http://montandon-eoapi-stac:8080/collections"},
+        ]
+    }
+    
+    # This should transform to http://localhost/collections (no port)
+    transformed = middleware.transform_json(data, Request(request_scope))
+    actual = transformed["links"][0]["href"]
+    expected = "http://localhost/collections"
+    
+    print(f"Edge case test:")
+    print(f"Expected: {expected}")
+    print(f"Actual:   {actual}")
+    
+    # This test should FAIL and demonstrate the bug
+    assert actual == expected, f"Edge case failed: expected {expected}, got {actual}"
