Repository: stac-utils/stac-fastapi
Pull request #782 (2025-01-16 10:51:32)

Support multiple proxy servers in Forwarded header parsing
**Related Issue(s):**
(none, since I directly created the Pull request for the fix)

**Description:**
According to the [Forwarded HTTP header spec](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded), multiple proxy servers can add values to the `Forwarded` header by creating a `comma` separated list:

> If there are multiple proxy servers between the client and server, they may each specify their own forwarding information. This can be done by adding a new Forwarded header to the end of the header block, or by appending the information to the end of the last Forwarded header in a comma-separated list.

The existing `ProxyHeaderMiddleware` however doesn't account for this, and instead raises a `ValueError` in such cases, because it only splits the string on `;` but not on `,` before that.

Therefore, a valid `Forwarded` header of `b"proto=https;host=test:1234,proto=https;host=second-server:1111"` produces a: `ValueError: too many values to unpack (expected 2)` because it essentially runs:

```python
part = "host=test:1234,proto=https"  # because it doesn't split on `,` first
key, value = part.split("=")  # ValueError: too many values to unpack (expected 2)
```

I've encountered this when deploying our STAC API server on GCP Cloud run, behind a GCP Load balancer.
GCP doesn't even actually set the `host` but the middleware in place still causes all requests to fail with a ValueError.

(The header on GCP that we receive is `b'for="85.193.181.55";proto=https,for="85.193.181.55";proto=https'`)

**PR Checklist:**

- [X] `pre-commit` hooks pass locally
- [X] Tests pass (run `make test`)
- [X] Documentation has been updated to reflect changes, if applicable, and docs build successfully (run `make docs`)
- [X] Changes are added to the [CHANGELOG](https://github.com/stac-utils/stac-fastapi/blob/main/CHANGES.md).


lukasbindreiter: > @lukasbindreiter hope you don't mind, I've pushed some change directly in your branch
> 
> Let me know how it looks for you and then I could merge and release this tomorrow

@vincentsarago 
Awesome, thanks a lot!

I've made some small changes to it:
- use compiled regexes
- avoid `groupdict` and instead call `group` directly to avoid unnecessary dict creations

(worth it I think in this case, its an easy optimization thats twice as fast)
![image](https://github.com/user-attachments/assets/3c34a997-faba-48be-8838-22ba1cc2fd37)

lukasbindreiter: @vincentsarago everything looks ready from my side - would be awesome if you could take another look, and then merge it if everything is fine. Thanks a lot!

Patch:
diff --git a/CHANGES.md b/CHANGES.md
index 67cee3e..4290fa1 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -2,11 +2,15 @@
 
 ## [Unreleased]
 
-## Changed
+### Changed
 
 * use `string` type instead of python `datetime.datetime` for datetime parameter in `BaseSearchGetRequest`, `ItemCollectionUri` and `BaseCollectionSearchGetRequest` GET models
 * rename `filter` to `filter_expr` for `FilterExtensionGetRequest` and `FilterExtensionPostRequest` attributes to avoid conflict with python filter method
 
+### Fixed
+
+* Support multiple proxy servers in the `forwarded` header in `ProxyHeaderMiddleware` ([#782](https://github.com/stac-utils/stac-fastapi/pull/782))
+
 ## [3.0.5] - 2025-01-10
 
 ### Removed
diff --git a/stac_fastapi/api/stac_fastapi/api/middleware.py b/stac_fastapi/api/stac_fastapi/api/middleware.py
index 2ba3ef5..0b11923 100644
--- a/stac_fastapi/api/stac_fastapi/api/middleware.py
+++ b/stac_fastapi/api/stac_fastapi/api/middleware.py
@@ -1,5 +1,6 @@
 """Api middleware."""
 
+import contextlib
 import re
 import typing
 from http.client import HTTP_PORT, HTTPS_PORT
@@ -44,6 +45,10 @@ class CORSMiddleware(_CORSMiddleware):
         )
 
 
+_PROTO_HEADER_REGEX = re.compile(r"proto=(?P<proto>http(s)?)")
+_HOST_HEADER_REGEX = re.compile(r"host=(?P<host>[\w.-]+)(:(?P<port>\d{1,5}))?")
+
+
 class ProxyHeaderMiddleware:
     """Account for forwarding headers when deriving base URL.
 
@@ -68,11 +73,13 @@ class ProxyHeaderMiddleware:
                         proto == "https" and port != HTTPS_PORT
                     ):
                         port_suffix = f":{port}"
+
                 scope["headers"] = self._replace_header_value_by_name(
                     scope,
                     "host",
                     f"{domain}{port_suffix}",
                 )
+
         await self.app(scope, receive, send)
 
     def _get_forwarded_url_parts(self, scope: Scope) -> Tuple[str]:
@@ -87,31 +94,23 @@ class ProxyHeaderMiddleware:
             else:
                 domain = header_host_parts[0]
                 port = None
-        forwarded = self._get_header_value_by_name(scope, "forwarded")
-        if forwarded is not None:
-            parts = forwarded.split(";")
-            for part in parts:
-                if len(part) > 0 and re.search("=", part):
-                    key, value = part.split("=")
-                    if key == "proto":
-                        proto = value
-                    elif key == "host":
-                        host_parts = value.split(":")
-                        domain = host_parts[0]
-                        try:
-                            port = int(host_parts[1]) if len(host_parts) == 2 else None
-                        except ValueError:
-                            # ignore ports that are not valid integers
-                            pass
+
+        if forwarded := self._get_header_value_by_name(scope, "forwarded"):
+            for proxy in forwarded.split(","):
+                if (proto_expr := _PROTO_HEADER_REGEX.search(proxy)) and (
+                    host_expr := _HOST_HEADER_REGEX.search(proxy)
+                ):
+                    proto = proto_expr.group("proto")
+                    domain = host_expr.group("host")
+                    port_str = host_expr.group("port")  # None if not present in the match
+
         else:
             domain = self._get_header_value_by_name(scope, "x-forwarded-host", domain)
             proto = self._get_header_value_by_name(scope, "x-forwarded-proto", proto)
             port_str = self._get_header_value_by_name(scope, "x-forwarded-port", port)
-            try:
-                port = int(port_str) if port_str is not None else None
-            except ValueError:
-                # ignore ports that are not valid integers
-                pass
+
+        with contextlib.suppress(ValueError):  # ignore ports that are not valid integers
+            port = int(port_str) if port_str is not None else port
 
         return (proto, domain, port)
 


Test patch:
diff --git a/stac_fastapi/api/tests/test_middleware.py b/stac_fastapi/api/tests/test_middleware.py
index 00e7f80..45f6551 100644
--- a/stac_fastapi/api/tests/test_middleware.py
+++ b/stac_fastapi/api/tests/test_middleware.py
@@ -155,6 +155,34 @@ def test_replace_header_value_by_name(
             },
             ("https", "test", 1234),
         ),
+        (
+            {
+                "scheme": "http",
+                "server": ["testserver", 80],
+                "headers": [
+                    (
+                        b"forwarded",
+                        # two proxy servers added an entry, we want to use the last one
+                        b"proto=https;host=test:1234,proto=https;host=second-server:1111",
+                    )
+                ],
+            },
+            ("https", "second-server", 1111),
+        ),
+        (
+            {
+                "scheme": "http",
+                "server": ["testserver", 80],
+                "headers": [
+                    (
+                        b"forwarded",
+                        # check when host and port are inverted
+                        b"host=test:1234;proto=https",
+                    )
+                ],
+            },
+            ("https", "test", 1234),
+        ),
     ],
 )
 def test_get_forwarded_url_parts(