Skip to content

Commit 91a9cb9

Browse files
authored
Avoid adding Content-Type to non-body responses (#6266)
* Avoid adding Content-Type to non-body responses The current code sets the content-type header for all responses to the result's content_type property if upstream does not set a content_type. The default value for content_type is "application/octet-stream". For responses that do not have a body (like 204 No Content or 304 Not Modified), setting a content-type header is unnecessary and potentially misleading. Follow HTTP standards by only adding the content-type header to responses that actually contain a body. * Add pytest for ingress proxy * Preserve Content-Type header for HEAD requests in ingress API
1 parent 8f2b076 commit 91a9cb9

File tree

2 files changed

+156
-7
lines changed

2 files changed

+156
-7
lines changed

supervisor/api/ingress.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -253,18 +253,28 @@ async def _handle_request(
253253
skip_auto_headers={hdrs.CONTENT_TYPE},
254254
) as result:
255255
headers = _response_header(result)
256+
256257
# Avoid parsing content_type in simple cases for better performance
257258
if maybe_content_type := result.headers.get(hdrs.CONTENT_TYPE):
258259
content_type = (maybe_content_type.partition(";"))[0].strip()
259260
else:
260261
content_type = result.content_type
262+
263+
# Empty body responses (304, 204, HEAD, etc.) should not be streamed,
264+
# otherwise aiohttp < 3.9.0 may generate an invalid "0\r\n\r\n" chunk
265+
# This also avoids setting content_type for empty responses.
266+
if must_be_empty_body(request.method, result.status):
267+
# If upstream contains content-type, preserve it (e.g. for HEAD requests)
268+
if maybe_content_type:
269+
headers[hdrs.CONTENT_TYPE] = content_type
270+
return web.Response(
271+
headers=headers,
272+
status=result.status,
273+
)
274+
261275
# Simple request
262276
if (
263-
# empty body responses should not be streamed,
264-
# otherwise aiohttp < 3.9.0 may generate
265-
# an invalid "0\r\n\r\n" chunk instead of an empty response.
266-
must_be_empty_body(request.method, result.status)
267-
or hdrs.CONTENT_LENGTH in result.headers
277+
hdrs.CONTENT_LENGTH in result.headers
268278
and int(result.headers.get(hdrs.CONTENT_LENGTH, 0)) < 4_194_000
269279
):
270280
# Return Response

tests/api/test_ingress.py

Lines changed: 141 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,28 @@
11
"""Test ingress API."""
22

3-
from unittest.mock import AsyncMock, patch
3+
from collections.abc import AsyncGenerator
4+
from unittest.mock import AsyncMock, MagicMock, patch
45

5-
from aiohttp.test_utils import TestClient
6+
import aiohttp
7+
from aiohttp import hdrs, web
8+
from aiohttp.test_utils import TestClient, TestServer
9+
import pytest
610

11+
from supervisor.addons.addon import Addon
712
from supervisor.coresys import CoreSys
813

914

15+
@pytest.fixture(name="real_websession")
16+
async def fixture_real_websession(
17+
coresys: CoreSys,
18+
) -> AsyncGenerator[aiohttp.ClientSession]:
19+
"""Fixture for real aiohttp ClientSession for ingress proxy tests."""
20+
session = aiohttp.ClientSession()
21+
coresys._websession = session # pylint: disable=W0212
22+
yield session
23+
await session.close()
24+
25+
1026
async def test_validate_session(api_client: TestClient, coresys: CoreSys):
1127
"""Test validating ingress session."""
1228
with patch("aiohttp.web_request.BaseRequest.__getitem__", return_value=None):
@@ -86,3 +102,126 @@ async def test_validate_session_with_user_id(
86102
assert (
87103
coresys.ingress.get_session_data(session).user.display_name == "Some Name"
88104
)
105+
106+
107+
async def test_ingress_proxy_no_content_type_for_empty_body_responses(
108+
api_client: TestClient, coresys: CoreSys, real_websession: aiohttp.ClientSession
109+
):
110+
"""Test that empty body responses don't get Content-Type header."""
111+
112+
# Create a mock add-on backend server that returns various status codes
113+
async def mock_addon_handler(request: web.Request) -> web.Response:
114+
"""Mock add-on handler that returns different status codes based on path."""
115+
path = request.path
116+
117+
if path == "/204":
118+
# 204 No Content - should not have Content-Type
119+
return web.Response(status=204)
120+
elif path == "/304":
121+
# 304 Not Modified - should not have Content-Type
122+
return web.Response(status=304)
123+
elif path == "/100":
124+
# 100 Continue - should not have Content-Type
125+
return web.Response(status=100)
126+
elif path == "/head":
127+
# HEAD request - should have Content-Type (same as GET would)
128+
return web.Response(body=b"test", content_type="text/html")
129+
elif path == "/200":
130+
# 200 OK with body - should have Content-Type
131+
return web.Response(body=b"test content", content_type="text/plain")
132+
elif path == "/200-no-content-type":
133+
# 200 OK without explicit Content-Type - should get default
134+
return web.Response(body=b"test content")
135+
elif path == "/200-json":
136+
# 200 OK with JSON - should preserve Content-Type
137+
return web.Response(
138+
body=b'{"key": "value"}', content_type="application/json"
139+
)
140+
else:
141+
return web.Response(body=b"default", content_type="text/html")
142+
143+
# Create test server for mock add-on
144+
app = web.Application()
145+
app.router.add_route("*", "/{tail:.*}", mock_addon_handler)
146+
addon_server = TestServer(app)
147+
await addon_server.start_server()
148+
149+
try:
150+
# Create ingress session
151+
resp = await api_client.post("/ingress/session")
152+
result = await resp.json()
153+
session = result["data"]["session"]
154+
155+
# Create a mock add-on
156+
mock_addon = MagicMock(spec=Addon)
157+
mock_addon.slug = "test_addon"
158+
mock_addon.ip_address = addon_server.host
159+
mock_addon.ingress_port = addon_server.port
160+
mock_addon.ingress_stream = False
161+
162+
# Generate an ingress token and register the add-on
163+
ingress_token = coresys.ingress.create_session()
164+
with patch.object(coresys.ingress, "get", return_value=mock_addon):
165+
# Test 204 No Content - should NOT have Content-Type
166+
resp = await api_client.get(
167+
f"/ingress/{ingress_token}/204",
168+
cookies={"ingress_session": session},
169+
)
170+
assert resp.status == 204
171+
assert hdrs.CONTENT_TYPE not in resp.headers
172+
173+
# Test 304 Not Modified - should NOT have Content-Type
174+
resp = await api_client.get(
175+
f"/ingress/{ingress_token}/304",
176+
cookies={"ingress_session": session},
177+
)
178+
assert resp.status == 304
179+
assert hdrs.CONTENT_TYPE not in resp.headers
180+
181+
# Test HEAD request - SHOULD have Content-Type (same as GET)
182+
# per RFC 9110: HEAD should return same headers as GET
183+
resp = await api_client.head(
184+
f"/ingress/{ingress_token}/head",
185+
cookies={"ingress_session": session},
186+
)
187+
assert resp.status == 200
188+
assert hdrs.CONTENT_TYPE in resp.headers
189+
assert "text/html" in resp.headers[hdrs.CONTENT_TYPE]
190+
# Body should be empty for HEAD
191+
body = await resp.read()
192+
assert body == b""
193+
194+
# Test 200 OK with body - SHOULD have Content-Type
195+
resp = await api_client.get(
196+
f"/ingress/{ingress_token}/200",
197+
cookies={"ingress_session": session},
198+
)
199+
assert resp.status == 200
200+
assert hdrs.CONTENT_TYPE in resp.headers
201+
assert resp.headers[hdrs.CONTENT_TYPE] == "text/plain"
202+
body = await resp.read()
203+
assert body == b"test content"
204+
205+
# Test 200 OK without explicit Content-Type - SHOULD get default
206+
resp = await api_client.get(
207+
f"/ingress/{ingress_token}/200-no-content-type",
208+
cookies={"ingress_session": session},
209+
)
210+
assert resp.status == 200
211+
assert hdrs.CONTENT_TYPE in resp.headers
212+
# Should get application/octet-stream as default from aiohttp ClientResponse
213+
assert "application/octet-stream" in resp.headers[hdrs.CONTENT_TYPE]
214+
215+
# Test 200 OK with JSON - SHOULD preserve Content-Type
216+
resp = await api_client.get(
217+
f"/ingress/{ingress_token}/200-json",
218+
cookies={"ingress_session": session},
219+
)
220+
assert resp.status == 200
221+
assert hdrs.CONTENT_TYPE in resp.headers
222+
assert "application/json" in resp.headers[hdrs.CONTENT_TYPE]
223+
body = await resp.read()
224+
assert body == b'{"key": "value"}'
225+
226+
finally:
227+
await addon_server.close()

0 commit comments

Comments
 (0)