Skip to content

Commit f5fd496

Browse files
ZingzyZingzy
authored
Merge pull request #328 from spoo-me/fix/metadata-head-read-cap
fix(metadata): read to the end of the head, not a fixed prefix
2 parents 12271cf + cb2815f commit f5fd496

3 files changed

Lines changed: 94 additions & 8 deletions

File tree

‎infrastructure/safe_fetch.py‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,21 @@ def _bracket(ip: str) -> str:
120120

121121

122122
async def _read_body(
123-
resp: httpx.Response, max_bytes: int, truncate_over_cap: bool
123+
resp: httpx.Response,
124+
max_bytes: int,
125+
truncate_over_cap: bool,
126+
stop_after: bytes | None = None,
124127
) -> bytearray:
125128
buf = bytearray()
129+
searched = 0
126130
async for chunk in resp.aiter_bytes():
127131
buf += chunk
132+
if stop_after is not None:
133+
# Resume one marker-width back: it can straddle two chunks.
134+
found = buf.find(stop_after, max(0, searched - len(stop_after)))
135+
if found != -1:
136+
return buf[: found + len(stop_after)]
137+
searched = len(buf)
128138
if len(buf) > max_bytes:
129139
if truncate_over_cap:
130140
return buf[:max_bytes]
@@ -141,16 +151,20 @@ async def fetch_public(
141151
max_bytes: int = 1_048_576,
142152
max_redirects: int = 3,
143153
truncate_over_cap: bool = False,
154+
stop_after: bytes | None = None,
144155
user_agent: str = DEFAULT_USER_AGENT,
145156
) -> FetchedBody:
146157
"""Fetch *url* with SSRF guards. ``accept_content`` are content-type
147158
prefixes (e.g. ``("image/",)``); ``reject_content`` are substrings that
148159
fail even when a prefix matched (e.g. ``("svg",)``).
149160
150161
``truncate_over_cap=True`` returns the first ``max_bytes`` instead of
151-
failing when the body exceeds the cap — right for HTML meta parsing
152-
(tags live in <head>; github.com's homepage alone is >512KB), wrong
153-
for images (a truncated image is not a valid image)."""
162+
failing when the body exceeds the cap, which is right for HTML meta
163+
parsing and wrong for images (a truncated image is not a valid image).
164+
165+
``stop_after`` ends the read at a marker, so an HTML caller pays for
166+
the head rather than the cap. Heads are not reliably small: youtube
167+
puts ~700KB of inline JSON before its meta tags."""
154168
# httpx timeouts are per-operation and reset each chunk, so the body
155169
# read below is bounded by an explicit wall-clock ceiling instead.
156170
hop_deadline = timeout * 3
@@ -214,7 +228,7 @@ async def fetch_public(
214228
# wall-clock ceiling a slow-drip server can't evade.
215229
try:
216230
buf = await asyncio.wait_for(
217-
_read_body(resp, max_bytes, truncate_over_cap),
231+
_read_body(resp, max_bytes, truncate_over_cap, stop_after),
218232
timeout=hop_deadline,
219233
)
220234
except (asyncio.TimeoutError, TimeoutError) as exc:

‎routes/api_v1/metadata.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@
3939

4040
router = APIRouter(tags=["Metadata"])
4141

42-
_FETCH_MAX_BYTES = 524_288 # tags must be in the head; 512KB is generous
42+
# Heads are not reliably small (youtube: ~700KB before its meta tags),
43+
# so the read stops at </head> and this is only the backstop.
44+
_FETCH_MAX_BYTES = 1_048_576
45+
_HEAD_END = b"</head>"
4346
_FETCH_TIMEOUT = 5.0
4447

4548
_metadata_limit, _metadata_key = dynamic_limit(
@@ -112,8 +115,8 @@ async def get_metadata(
112115
accept_content=("text/html", "application/xhtml"),
113116
timeout=_FETCH_TIMEOUT,
114117
max_bytes=_FETCH_MAX_BYTES,
115-
# Big pages are fine — tags live in <head>, parse the prefix.
116118
truncate_over_cap=True,
119+
stop_after=_HEAD_END,
117120
user_agent=settings.meta_tags.fetch_user_agent,
118121
)
119122
except FetchTransientError as exc:

‎tests/unit/infrastructure/test_safe_fetch.py‎

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@
22

33
from __future__ import annotations
44

5-
from unittest.mock import AsyncMock, patch
5+
from unittest.mock import AsyncMock, MagicMock, patch
66

77
import pytest
88

99
from infrastructure.safe_fetch import (
1010
FetchHardError,
1111
_is_public,
12+
_read_body,
1213
fetch_public_image,
1314
resolve_public_ip,
1415
)
@@ -129,3 +130,71 @@ async def _fake_send(self, request, **kwargs):
129130
):
130131
await fetch_public_image("https://example.com/a.png")
131132
assert captured["accept_encoding"] == "identity"
133+
134+
135+
class TestReadBodyStopMarker:
136+
"""``stop_after`` ends the read at </head> so an HTML caller pays for the
137+
head, not the cap. Heads are not reliably small: youtube puts ~700KB of
138+
inline JSON before its meta tags, which a 512KB cap silently cut off."""
139+
140+
@staticmethod
141+
def _resp(chunks):
142+
resp = MagicMock()
143+
144+
async def aiter_bytes():
145+
for chunk in chunks:
146+
yield chunk
147+
148+
resp.aiter_bytes = aiter_bytes
149+
return resp
150+
151+
@pytest.mark.asyncio
152+
async def test_read_stops_at_the_marker(self):
153+
body = await _read_body(
154+
self._resp([b"<head><title>x</title></head>", b"<body>" + b"z" * 5000]),
155+
max_bytes=1_000_000,
156+
truncate_over_cap=True,
157+
stop_after=b"</head>",
158+
)
159+
assert bytes(body) == b"<head><title>x</title></head>"
160+
161+
@pytest.mark.asyncio
162+
async def test_marker_split_across_chunks_is_still_found(self):
163+
body = await _read_body(
164+
self._resp([b"<head>a</he", b"ad><body>ignored"]),
165+
max_bytes=1_000_000,
166+
truncate_over_cap=True,
167+
stop_after=b"</head>",
168+
)
169+
assert bytes(body) == b"<head>a</head>"
170+
171+
@pytest.mark.asyncio
172+
async def test_tags_past_the_old_512kb_cap_survive(self):
173+
"""The youtube shape: a huge head, then the tags."""
174+
head = b"<head>" + b"j" * 700_000 + b'<meta property="og:title"></head>'
175+
body = await _read_body(
176+
self._resp([head, b"<body>" + b"z" * 600_000]),
177+
max_bytes=1_048_576,
178+
truncate_over_cap=True,
179+
stop_after=b"</head>",
180+
)
181+
assert b"og:title" in bytes(body)
182+
183+
@pytest.mark.asyncio
184+
async def test_a_missing_marker_still_honours_the_cap(self):
185+
body = await _read_body(
186+
self._resp([b"x" * 900, b"y" * 900]),
187+
max_bytes=1_000,
188+
truncate_over_cap=True,
189+
stop_after=b"</head>",
190+
)
191+
assert len(body) == 1_000
192+
193+
@pytest.mark.asyncio
194+
async def test_without_a_marker_the_whole_body_is_read(self):
195+
body = await _read_body(
196+
self._resp([b"<head></head>", b"<body>tail"]),
197+
max_bytes=1_000,
198+
truncate_over_cap=True,
199+
)
200+
assert bytes(body) == b"<head></head><body>tail"

0 commit comments

Comments
 (0)