quality/testdays-web
6
0
Fork 0
Code Issues 10 Pull requests 1 Projects Releases Packages Wiki Activity Actions

Remove _ForwardedHeaderFix middleware
All checks were successful
Run tests / test (pull_request) Successful in 32s
Details

Browse source 
- likely not required anymore with X-Forwarded-For set to 2
This commit is contained in:
Jaroslav Groman 2026-06-18 12:17:24 +00:00
commit a0fc826122
3 changed files with 18 additions and 176 deletions
Download patch file Download diff file Expand all files Collapse all files

69
testdays/app.py
View file

@ -17,66 +17,6 @@ from . import config
from . import extensions
class _ForwardedHeaderFix:
"""Normalize duplicated ``X-Forwarded-*`` values before ProxyFix.
When the OpenShift HAProxy router uses the default ``append`` mode for
``haproxy.router.openshift.io/set-forwarded-headers``, every
``X-Forwarded-*`` header gets its value doubled, e.g.
``X-Forwarded-Proto: https,https`` or
``X-Forwarded-Host: app.example.com, app.example.com``.
ProxyFix itself parses comma-separated values correctly via
``parse_list_header``, but the raw environ keys are left untouched.
This causes problems for:
- **werkzeug >= 3.1.7** host validation: if ``x_host`` is enabled,
ProxyFix writes the clean value into ``HTTP_HOST``, but
``host_is_trusted()`` may still see the raw duplicated value and
reject it (empty string in 3.1.8, ``SecurityError`` with
``trusted_hosts``).
- **gunicorn** ``secure_scheme_headers``: does exact string matching
(``"https" != "https,https"``), silently downgrading to HTTP.
This middleware strips the duplication from all five headers and logs a
warning so the infra team knows the upstream proxy is misbehaving.
The proper upstream fix is setting the OpenShift route annotation
``haproxy.router.openshift.io/set-forwarded-headers: Replace``.
"""
# WSGI environ keys that correspond to X-Forwarded-* headers.
_FORWARDED_KEYS = (
"HTTP_X_FORWARDED_FOR",
"HTTP_X_FORWARDED_PROTO",
"HTTP_X_FORWARDED_HOST",
"HTTP_X_FORWARDED_PORT",
"HTTP_X_FORWARDED_PREFIX",
)
def __init__(self, app):
self.app = app
self._logger = logging.getLogger("testdays.proxy")
def __call__(self, environ, start_response):
for key in self._FORWARDED_KEYS:
raw = environ.get(key, "")
if "," in raw:
parts = [p.strip() for p in raw.split(",")]
if len(set(parts)) == 1:
# All values are identical: HAProxy append-mode duplication.
normalized = parts[0]
self._logger.warning(
"Received duplicated %s header: %r "
"(normalized to %r)",
key,
raw,
normalized,
)
environ[key] = normalized
# else: different values in the chain -- leave untouched.
return self.app(environ, start_response)
class _WerkzeugRequestFilter(logging.Filter):
"""Drop Werkzeug's per-request log lines but keep startup/error messages."""
@ -323,11 +263,9 @@ def create_app(config_obj=None):
# --- Reverse proxy middleware ---
if os.getenv("IS_OPENSHIFT"):
# Apply ProxyFix first (inner), then wrap with _ForwardedHeaderFix
# (outer). WSGI middlewares execute outside-in, so the outermost
# wrapper runs first on incoming requests. _ForwardedHeaderFix must
# normalize duplicated X-Forwarded-* values (e.g. "https,https")
# *before* ProxyFix parses them.
# ProxyFix uses parse_list_header() internally, so it correctly
# handles duplicated values produced by HAProxy's append mode
# (e.g. "https,https") without extra normalization.
app.wsgi_app = ProxyFix(
app.wsgi_app,
x_for=int(app.config["PROXYFIX_X_FOR"]),
@ -336,7 +274,6 @@ def create_app(config_obj=None):
x_port=int(app.config["PROXYFIX_X_PORT"]),
x_prefix=int(app.config["PROXYFIX_X_PREFIX"]),
)
app.wsgi_app = _ForwardedHeaderFix(app.wsgi_app)
if app.config.get("SHOW_DB_URI"):
app.logger.debug("using DBURI: %s", app.config["SQLALCHEMY_DATABASE_URI"])

9
testdays/config.py
View file

@ -49,13 +49,12 @@ class Config(object):
# Each value = number of proxies trusted for that header (0 = ignore).
# Overridable via env vars or settings.py.
#
# Note: enabling x_host (setting to 1) requires _ForwardedHeaderFix to
# normalize duplicated X-Forwarded-Host values first. Without it,
# werkzeug >= 3.1.7 rejects the comma-separated hostname as invalid
# (returns empty string or raises SecurityError with trusted_hosts).
# ProxyFix uses parse_list_header() internally, so duplicated values
# from HAProxy's append mode (e.g. "https,https") are handled correctly
# as a 2-element list -- no extra normalization needed.
PROXYFIX_X_FOR = 2
PROXYFIX_X_PROTO = 2
PROXYFIX_X_HOST = 0 # see note above before enabling
PROXYFIX_X_HOST = 0
PROXYFIX_X_PORT = 0
PROXYFIX_X_PREFIX = 0

116
tests/unit/test_proxy_fix.py
View file

@ -3,10 +3,8 @@ Unit tests for ProxyFix middleware integration.
Verifies that werkzeug's ProxyFix is applied when IS_OPENSHIFT is set,
config values are passed through, and it is not applied otherwise.
Also tests that duplicated X-Forwarded-* headers are normalized.
"""
import logging
import os
from unittest import mock
@ -14,7 +12,7 @@ import pytest
from jinjax.middleware import ComponentsMiddleware
from werkzeug.middleware.proxy_fix import ProxyFix
from testdays.app import create_app, _ForwardedHeaderFix
from testdays.app import create_app
from testdays.config import Testing
from tests.conftest import TEST_SETTINGS_PATH
@ -39,19 +37,17 @@ class TestProxyFixMiddleware:
"""
GIVEN IS_OPENSHIFT is set in environment
WHEN the app is created
THEN _ForwardedHeaderFix should be outermost, wrapping a ProxyFix
THEN ProxyFix should be the outermost middleware
"""
assert isinstance(openshift_app.wsgi_app, _ForwardedHeaderFix)
assert isinstance(openshift_app.wsgi_app.app, ProxyFix)
assert isinstance(openshift_app.wsgi_app, ProxyFix)
def test_proxyfix_not_applied_when_is_openshift_unset(self, app):
"""
GIVEN IS_OPENSHIFT is not set in environment
WHEN the app is created
THEN neither ProxyFix nor _ForwardedHeaderFix should be in the stack
THEN ProxyFix should not be in the middleware stack
"""
assert not isinstance(app.wsgi_app, ProxyFix)
assert not isinstance(app.wsgi_app, _ForwardedHeaderFix)
def test_proxyfix_uses_config_defaults(self, openshift_app):
"""
@ -59,7 +55,7 @@ class TestProxyFixMiddleware:
WHEN the app is created
THEN ProxyFix should have x_for=2, x_proto=2, others=0
"""
pf = openshift_app.wsgi_app.app # unwrap _ForwardedHeaderFix
pf = openshift_app.wsgi_app
assert pf.x_for == 2
assert pf.x_proto == 2
assert pf.x_host == 0
@ -81,110 +77,20 @@ class TestProxyFixMiddleware:
config_obj.PROXYFIX_X_PREFIX = 1
with mock.patch.dict(os.environ, {"IS_OPENSHIFT": "1"}):
app = create_app(config_obj)
pf = app.wsgi_app.app # unwrap _ForwardedHeaderFix
pf = app.wsgi_app
assert pf.x_for == 2
assert pf.x_proto == 2
assert pf.x_host == 1
assert pf.x_port == 1
assert pf.x_prefix == 1
def test_forwarded_header_fix_in_middleware_stack(self, openshift_app):
def test_proxyfix_wraps_components_middleware(self, openshift_app):
"""
GIVEN IS_OPENSHIFT is set
WHEN the app is created
THEN the middleware stack should be:
_ForwardedHeaderFix -> ProxyFix -> ComponentsMiddleware (WhiteNoise)
ProxyFix -> ComponentsMiddleware (WhiteNoise)
"""
header_fix = openshift_app.wsgi_app
assert isinstance(header_fix, _ForwardedHeaderFix)
proxy_fix = header_fix.app
assert isinstance(proxy_fix, ProxyFix)
assert isinstance(proxy_fix.app, ComponentsMiddleware)
class TestForwardedHeaderFix:
"""Tests for _ForwardedHeaderFix header normalization and logging."""
@pytest.fixture(autouse=True)
def _setup_middleware(self):
"""Create a _ForwardedHeaderFix wrapping a stub WSGI app that
captures the (possibly rewritten) environ values."""
self.captured = {}
def inner_app(environ, start_response):
for key in _ForwardedHeaderFix._FORWARDED_KEYS:
self.captured[key] = environ.get(key)
start_response("200 OK", [])
return [b""]
self.mw = _ForwardedHeaderFix(inner_app)
@pytest.mark.parametrize(
"header_key, raw, expected",
[
# Identical duplicates (HAProxy append mode) -- normalized
("HTTP_X_FORWARDED_PROTO", "https,https", "https"),
("HTTP_X_FORWARDED_PROTO", "https , https", "https"),
("HTTP_X_FORWARDED_PROTO", "http,http,http", "http"),
("HTTP_X_FORWARDED_HOST", "app.example.com, app.example.com",
"app.example.com"),
("HTTP_X_FORWARDED_FOR", "1.2.3.4, 1.2.3.4", "1.2.3.4"),
("HTTP_X_FORWARDED_PORT", "443,443", "443"),
# Single values -- passed through unchanged
("HTTP_X_FORWARDED_PROTO", "https", "https"),
("HTTP_X_FORWARDED_HOST", "app.example.com", "app.example.com"),
# Legitimate multi-hop chain (different values) -- left untouched
("HTTP_X_FORWARDED_FOR", "1.2.3.4, 10.0.0.1", "1.2.3.4, 10.0.0.1"),
],
ids=[
"proto-duplicated",
"proto-duplicated-with-spaces",
"proto-tripled",
"host-duplicated",
"for-duplicated",
"port-duplicated",
"proto-single",
"host-single",
"for-multihop-passthrough",
],
)
def test_normalizes_header(self, header_key, raw, expected):
"""Identical duplicates are collapsed; distinct multi-hop values pass through."""
self.mw({header_key: raw}, lambda *a: None)
assert self.captured[header_key] == expected
@pytest.mark.parametrize(
"header_key, raw, expect_warning",
[
("HTTP_X_FORWARDED_PROTO", "https,https", True),
("HTTP_X_FORWARDED_HOST", "a.example.com, a.example.com", True),
("HTTP_X_FORWARDED_PROTO", "https", False),
("HTTP_X_FORWARDED_HOST", None, False),
# Legitimate multi-hop -- different values, no warning
("HTTP_X_FORWARDED_FOR", "1.2.3.4, 10.0.0.1", False),
],
ids=["proto-duplicated", "host-duplicated", "proto-single",
"host-absent", "for-multihop-no-warning"],
)
def test_warning_on_duplicated_header(self, caplog, header_key,
raw, expect_warning):
"""A warning is logged only for identical duplicate values."""
env = {header_key: raw} if raw is not None else {}
# Attach caplog's handler directly to the logger so capture works
# regardless of root logger state (which _configure_logging may alter).
logger = logging.getLogger("testdays.proxy")
logger.addHandler(caplog.handler)
try:
with caplog.at_level(logging.WARNING, logger="testdays.proxy"):
self.mw(env, lambda *a: None)
finally:
logger.removeHandler(caplog.handler)
found = any(
f"Received duplicated {header_key} header" in msg
for msg in caplog.messages
)
assert found is expect_warning, (
f"expect_warning={expect_warning} but log messages: {caplog.messages}"
)
pf = openshift_app.wsgi_app
assert isinstance(pf, ProxyFix)
assert isinstance(pf.app, ComponentsMiddleware)