Skip to content

Commit 786fa05

Browse files
committed
fix: prevent CORS header duplication on middleware early-return responses
The 0.82.0 performance commit changed the response finalization path so that global response headers are unconditionally `extend`ed onto every response, including early-return responses from before-middleware. When ALLOW_CORS is used, this causes Access-Control-Allow-Origin (and other CORS headers) to appear twice — once from the middleware and once from the global defaults. Per the Fetch spec, browsers reject responses with duplicate Access-Control-Allow-Origin values, breaking preflight. Fix: replace `extend` (append) with `set_missing` (set-if-absent) when merging global response headers. This gives middleware-set headers precedence over global defaults, preventing duplication. Closes #1346 Made-with: Cursor
1 parent a54ff96 commit 786fa05

4 files changed

Lines changed: 219 additions & 1 deletion

File tree

integration_tests/cors_app.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
"""
2+
Standalone Robyn app with ALLOW_CORS enabled, used by CORS integration tests.
3+
Runs on a separate port (8083) so it doesn't conflict with base_routes.py.
4+
"""
5+
6+
import os
7+
8+
from robyn import ALLOW_CORS, Robyn
9+
10+
app = Robyn(__file__)
11+
12+
ALLOWED_ORIGINS = ["http://localhost:3000", "https://frontend.example.com"]
13+
ALLOW_CORS(app, origins=ALLOWED_ORIGINS)
14+
15+
16+
@app.get("/")
17+
def index():
18+
return "OK"
19+
20+
21+
@app.post("/data")
22+
def post_data(request):
23+
return "created"
24+
25+
26+
@app.get("/custom-header")
27+
def custom_header(request):
28+
from robyn import Response
29+
30+
return Response(
31+
status_code=200,
32+
headers={"x-custom": "hello"},
33+
description="custom",
34+
)
35+
36+
37+
if __name__ == "__main__":
38+
port = int(os.getenv("ROBYN_PORT", "8083"))
39+
app.start(port=port, _check_port=False)
Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
"""
2+
Integration tests for CORS preflight (OPTIONS) handling.
3+
4+
Regression test for https://github.com/sparckles/robyn/issues/1346
5+
These tests spin up a real Robyn server with ALLOW_CORS enabled and make
6+
actual HTTP requests — no TestClient.
7+
"""
8+
9+
import os
10+
import pathlib
11+
import socket
12+
import subprocess
13+
import time
14+
15+
import pytest
16+
import requests
17+
18+
CORS_PORT = 8083
19+
CORS_HOST = "127.0.0.1"
20+
CORS_BASE_URL = f"http://{CORS_HOST}:{CORS_PORT}"
21+
22+
ALLOWED_ORIGIN = "http://localhost:3000"
23+
DISALLOWED_ORIGIN = "http://evil.example.com"
24+
25+
26+
def _start_cors_server():
27+
app_path = os.path.join(pathlib.Path(__file__).parent.resolve(), "cors_app.py")
28+
env = os.environ.copy()
29+
env["ROBYN_HOST"] = CORS_HOST
30+
env["ROBYN_PORT"] = str(CORS_PORT)
31+
32+
process = subprocess.Popen(
33+
["python3", app_path],
34+
env=env,
35+
preexec_fn=os.setsid,
36+
)
37+
38+
timeout = 15
39+
start = time.time()
40+
while True:
41+
if time.time() - start > timeout:
42+
os.killpg(os.getpgid(process.pid), 9)
43+
raise ConnectionError(f"CORS server didn't start on {CORS_HOST}:{CORS_PORT}")
44+
try:
45+
sock = socket.create_connection((CORS_HOST, CORS_PORT), timeout=2)
46+
sock.close()
47+
break
48+
except Exception:
49+
time.sleep(0.5)
50+
51+
time.sleep(1)
52+
return process
53+
54+
55+
@pytest.fixture(scope="module")
56+
def cors_server():
57+
process = _start_cors_server()
58+
yield
59+
import signal
60+
61+
try:
62+
os.killpg(os.getpgid(process.pid), signal.SIGKILL)
63+
except ProcessLookupError:
64+
pass
65+
66+
67+
# ---------------------------------------------------------------------------
68+
# Preflight (OPTIONS) tests
69+
# ---------------------------------------------------------------------------
70+
71+
72+
def test_options_preflight_returns_204_with_cors_headers(cors_server):
73+
"""Browser sends OPTIONS preflight; server must return 204 with all CORS headers."""
74+
resp = requests.options(
75+
f"{CORS_BASE_URL}/data",
76+
headers={
77+
"Origin": ALLOWED_ORIGIN,
78+
"Access-Control-Request-Method": "POST",
79+
"Access-Control-Request-Headers": "Content-Type, Authorization",
80+
},
81+
)
82+
assert resp.status_code == 204
83+
assert resp.headers["Access-Control-Allow-Origin"] == ALLOWED_ORIGIN
84+
assert "POST" in resp.headers["Access-Control-Allow-Methods"]
85+
assert resp.headers["Access-Control-Allow-Credentials"] == "true"
86+
87+
88+
def test_options_preflight_no_duplicate_allow_origin(cors_server):
89+
"""
90+
Regression: ensure Access-Control-Allow-Origin appears exactly once.
91+
Duplicate values cause browsers to reject the preflight (Fetch spec).
92+
"""
93+
resp = requests.options(
94+
f"{CORS_BASE_URL}/",
95+
headers={
96+
"Origin": ALLOWED_ORIGIN,
97+
"Access-Control-Request-Method": "GET",
98+
},
99+
)
100+
raw_headers = resp.raw.headers if hasattr(resp.raw, "headers") else resp.headers
101+
102+
origin_values = raw_headers.getall("Access-Control-Allow-Origin", None) if hasattr(raw_headers, "getall") else None
103+
104+
if origin_values is not None:
105+
assert len(origin_values) == 1, (
106+
f"Access-Control-Allow-Origin appeared {len(origin_values)} times: {origin_values}"
107+
)
108+
109+
assert resp.headers["Access-Control-Allow-Origin"] == ALLOWED_ORIGIN
110+
111+
112+
def test_options_preflight_disallowed_origin_returns_403(cors_server):
113+
"""Origins not in the allowed list should be rejected."""
114+
resp = requests.options(
115+
f"{CORS_BASE_URL}/data",
116+
headers={
117+
"Origin": DISALLOWED_ORIGIN,
118+
"Access-Control-Request-Method": "POST",
119+
},
120+
)
121+
assert resp.status_code == 403
122+
123+
124+
# ---------------------------------------------------------------------------
125+
# Normal request tests (non-preflight)
126+
# ---------------------------------------------------------------------------
127+
128+
129+
def test_get_with_allowed_origin_has_cors_headers(cors_server):
130+
"""Normal GET from an allowed origin should carry CORS response headers."""
131+
resp = requests.get(
132+
f"{CORS_BASE_URL}/",
133+
headers={"Origin": ALLOWED_ORIGIN},
134+
)
135+
assert resp.status_code == 200
136+
allow_origin = resp.headers.get("Access-Control-Allow-Origin")
137+
assert allow_origin is not None
138+
assert "Access-Control-Allow-Methods" in resp.headers
139+
140+
141+
def test_get_without_origin_still_has_global_cors_headers(cors_server):
142+
"""Requests without Origin (e.g. curl, Postman) should still get global CORS headers."""
143+
resp = requests.get(f"{CORS_BASE_URL}/")
144+
assert resp.status_code == 200
145+
assert "Access-Control-Allow-Methods" in resp.headers
146+
147+
148+
def test_post_with_allowed_origin(cors_server):
149+
"""POST from allowed origin should succeed with CORS headers."""
150+
resp = requests.post(
151+
f"{CORS_BASE_URL}/data",
152+
headers={
153+
"Origin": ALLOWED_ORIGIN,
154+
"Content-Type": "application/json",
155+
},
156+
data="{}",
157+
)
158+
assert resp.status_code == 200
159+
160+
161+
def test_custom_response_headers_not_clobbered_by_globals(cors_server):
162+
"""Route-level headers set by the handler should not be overwritten by globals."""
163+
resp = requests.get(
164+
f"{CORS_BASE_URL}/custom-header",
165+
headers={"Origin": ALLOWED_ORIGIN},
166+
)
167+
assert resp.status_code == 200
168+
assert resp.headers.get("x-custom") == "hello"
169+
assert "Access-Control-Allow-Methods" in resp.headers

src/server.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ async fn index(
575575
.is_some_and(|paths| paths.contains(&request.url.path));
576576

577577
if !is_excluded {
578-
response.headers_mut().extend(&global_response_headers);
578+
response.headers_mut().set_missing(&global_response_headers);
579579
}
580580

581581
// After middleware

src/types/headers.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,16 @@ impl Headers {
177177
}
178178
}
179179

180+
/// Merge headers from `headers` into `self`, but only for keys not already present.
181+
/// This gives middleware-set headers precedence over global defaults,
182+
/// preventing duplicate `Access-Control-Allow-Origin` (and similar) violations.
183+
pub fn set_missing(&mut self, headers: &Headers) {
184+
for iter in headers.headers.iter() {
185+
let (key, values) = iter.pair();
186+
self.headers.entry(key.clone()).or_insert_with(|| values.clone());
187+
}
188+
}
189+
180190
pub fn from_actix_headers(req_headers: &HeaderMap) -> Self {
181191
let headers = Headers::default();
182192

0 commit comments

Comments
 (0)