fix(evidence): bind HTTP validators to final URL
This commit is contained in:
@@ -54,3 +54,12 @@ Four review findings were closed in a separate follow-up commit:
|
|||||||
conditional headers; a 304 reuses only previously verified cached bytes and identity. The LRU
|
conditional headers; a 304 reuses only previously verified cached bytes and identity. The LRU
|
||||||
content cache has an explicit byte bound (`max_cache_bytes`). Validators are not forwarded
|
content cache has an explicit byte bound (`max_cache_bytes`). Validators are not forwarded
|
||||||
across redirect origins.
|
across redirect origins.
|
||||||
|
|
||||||
|
### Conditional cache binding correction
|
||||||
|
|
||||||
|
The conditional cache now binds bytes and validators to both the canonical provenance key and the
|
||||||
|
exact final effective representation URL. Redirect traversal recomputes request headers per hop:
|
||||||
|
validators are sent only when that exact URL matches the cached final URL, never merely because a
|
||||||
|
redirect retains an origin. A same-origin path change therefore downloads and replaces the body.
|
||||||
|
The adapter accepts 304 only when the exact request carried a bound ETag or Last-Modified validator;
|
||||||
|
unsolicited and cross-origin 304 responses are permanent protocol errors.
|
||||||
|
|||||||
@@ -11,6 +11,9 @@ from tht.ports.evidence import EvidenceSourceError
|
|||||||
class Handler(BaseHTTPRequestHandler):
|
class Handler(BaseHTTPRequestHandler):
|
||||||
etag_requests = 0
|
etag_requests = 0
|
||||||
etag_body_responses = 0
|
etag_body_responses = 0
|
||||||
|
redirect_target = "/redirected-v1"
|
||||||
|
redirect_request_validators = []
|
||||||
|
final_request_validators = []
|
||||||
|
|
||||||
def do_GET(self):
|
def do_GET(self):
|
||||||
if self.path.startswith("/etag"):
|
if self.path.startswith("/etag"):
|
||||||
@@ -46,6 +49,24 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
"Location", f"http://user:password@127.0.0.1:{self.server.server_port}/etag"
|
"Location", f"http://user:password@127.0.0.1:{self.server.server_port}/etag"
|
||||||
)
|
)
|
||||||
self.end_headers()
|
self.end_headers()
|
||||||
|
elif self.path == "/stable-redirect":
|
||||||
|
type(self).redirect_request_validators.append(self.headers.get("If-None-Match"))
|
||||||
|
self.send_response(302)
|
||||||
|
self.send_header("Location", type(self).redirect_target)
|
||||||
|
self.end_headers()
|
||||||
|
elif self.path in {"/redirected-v1", "/redirected-v2"}:
|
||||||
|
type(self).final_request_validators.append(
|
||||||
|
(self.path, self.headers.get("If-None-Match"))
|
||||||
|
)
|
||||||
|
etag = '"v1"' if self.path.endswith("v1") else '"v2"'
|
||||||
|
if self.headers.get("If-None-Match") == etag:
|
||||||
|
self.send_response(304)
|
||||||
|
self.end_headers()
|
||||||
|
return
|
||||||
|
self.send_response(200)
|
||||||
|
self.send_header("ETag", etag)
|
||||||
|
self.end_headers()
|
||||||
|
self.wfile.write(self.path.encode())
|
||||||
else:
|
else:
|
||||||
self.send_response(200)
|
self.send_response(200)
|
||||||
self.send_header("Last-Modified", "Wed, 21 Oct 2015 07:28:00 GMT")
|
self.send_header("Last-Modified", "Wed, 21 Oct 2015 07:28:00 GMT")
|
||||||
@@ -60,6 +81,9 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
def server_url():
|
def server_url():
|
||||||
Handler.etag_requests = 0
|
Handler.etag_requests = 0
|
||||||
Handler.etag_body_responses = 0
|
Handler.etag_body_responses = 0
|
||||||
|
Handler.redirect_target = "/redirected-v1"
|
||||||
|
Handler.redirect_request_validators = []
|
||||||
|
Handler.final_request_validators = []
|
||||||
server = ThreadingHTTPServer(("127.0.0.1", 0), Handler)
|
server = ThreadingHTTPServer(("127.0.0.1", 0), Handler)
|
||||||
thread = threading.Thread(target=server.serve_forever, daemon=True)
|
thread = threading.Thread(target=server.serve_forever, daemon=True)
|
||||||
thread.start()
|
thread.start()
|
||||||
@@ -246,3 +270,62 @@ def test_http_closes_response_when_streaming_fails():
|
|||||||
with pytest.raises(EvidenceSourceError):
|
with pytest.raises(EvidenceSourceError):
|
||||||
list(source.discover())
|
list(source.discover())
|
||||||
assert response.closed
|
assert response.closed
|
||||||
|
|
||||||
|
|
||||||
|
def test_http_binds_validators_to_exact_final_redirect_url(server_url):
|
||||||
|
source = HttpManifestEvidenceSource(
|
||||||
|
[server_url + "/stable-redirect"], allow_private_hosts=True
|
||||||
|
)
|
||||||
|
first = next(iter(source.discover()))
|
||||||
|
second = next(iter(source.discover()))
|
||||||
|
|
||||||
|
assert first == second
|
||||||
|
assert Handler.redirect_request_validators == [None, None]
|
||||||
|
assert Handler.final_request_validators == [
|
||||||
|
("/redirected-v1", None),
|
||||||
|
("/redirected-v1", '"v1"'),
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_http_redirect_path_change_fetches_and_replaces_body(server_url):
|
||||||
|
source = HttpManifestEvidenceSource(
|
||||||
|
[server_url + "/stable-redirect"], allow_private_hosts=True
|
||||||
|
)
|
||||||
|
first = next(iter(source.discover()))
|
||||||
|
assert source.acquire(first).content == b"/redirected-v1"
|
||||||
|
|
||||||
|
Handler.redirect_target = "/redirected-v2"
|
||||||
|
with pytest.raises(EvidenceSourceError):
|
||||||
|
source.acquire(first)
|
||||||
|
current = next(iter(source.discover()))
|
||||||
|
|
||||||
|
assert source.acquire(current).content == b"/redirected-v2"
|
||||||
|
assert ("/redirected-v2", None) in Handler.final_request_validators
|
||||||
|
|
||||||
|
|
||||||
|
def test_http_rejects_unsolicited_304_without_bound_validator(monkeypatch):
|
||||||
|
monkeypatch.setattr(socket, "getaddrinfo", lambda *args, **kwargs: [
|
||||||
|
(socket.AF_INET, socket.SOCK_STREAM, 6, "", ("93.184.216.34", 443)),
|
||||||
|
])
|
||||||
|
source = HttpManifestEvidenceSource(["https://example.test/doc"])
|
||||||
|
response = FakeResponse(peer="93.184.216.34")
|
||||||
|
response.status_code = 304
|
||||||
|
source._session = FakeSession(response)
|
||||||
|
|
||||||
|
with pytest.raises(EvidenceSourceError) as caught:
|
||||||
|
list(source.discover())
|
||||||
|
assert not caught.value.retryable
|
||||||
|
assert response.closed
|
||||||
|
|
||||||
|
|
||||||
|
def test_http_rejects_cross_origin_304_for_cached_provenance(server_url):
|
||||||
|
source = HttpManifestEvidenceSource([server_url + "/etag"], allow_private_hosts=True)
|
||||||
|
item = next(iter(source.discover()))
|
||||||
|
response = FakeResponse()
|
||||||
|
response.status_code = 304
|
||||||
|
source._session = FakeSession(response)
|
||||||
|
|
||||||
|
with pytest.raises(EvidenceSourceError) as caught:
|
||||||
|
source._download("https://other.example/doc", item.uri)
|
||||||
|
assert not caught.value.retryable
|
||||||
|
assert response.closed
|
||||||
|
|||||||
@@ -57,7 +57,8 @@ class HttpManifestEvidenceSource:
|
|||||||
self._session = requests.Session()
|
self._session = requests.Session()
|
||||||
self._session.trust_env = False
|
self._session.trust_env = False
|
||||||
self._cache: OrderedDict[str, AcquiredDocument] = OrderedDict()
|
self._cache: OrderedDict[str, AcquiredDocument] = OrderedDict()
|
||||||
self._validators: dict[str, tuple[str | None, str | None]] = {}
|
# provenance -> (exact final effective URL, ETag, Last-Modified)
|
||||||
|
self._validators: dict[str, tuple[str, str | None, str | None]] = {}
|
||||||
self._cache_bytes = 0
|
self._cache_bytes = 0
|
||||||
|
|
||||||
def __repr__(self) -> str:
|
def __repr__(self) -> str:
|
||||||
@@ -137,12 +138,14 @@ class HttpManifestEvidenceSource:
|
|||||||
return EvidenceSourceErrorCategory.TRANSIENT
|
return EvidenceSourceErrorCategory.TRANSIENT
|
||||||
return EvidenceSourceErrorCategory.PERMANENT
|
return EvidenceSourceErrorCategory.PERMANENT
|
||||||
|
|
||||||
def _conditional_headers(self, provenance: str) -> dict[str, str]:
|
def _conditional_headers(self, provenance: str, request_url: str) -> dict[str, str]:
|
||||||
cached = self._cache.get(self._source_id(provenance))
|
cached = self._cache.get(self._source_id(provenance))
|
||||||
validators = self._validators.get(provenance)
|
validators = self._validators.get(provenance)
|
||||||
if cached is None or validators is None:
|
if cached is None or validators is None:
|
||||||
return {}
|
return {}
|
||||||
etag, last_modified = validators
|
final_url, etag, last_modified = validators
|
||||||
|
if request_url != final_url:
|
||||||
|
return {}
|
||||||
headers = {}
|
headers = {}
|
||||||
if etag:
|
if etag:
|
||||||
headers["If-None-Match"] = etag
|
headers["If-None-Match"] = etag
|
||||||
@@ -153,6 +156,7 @@ class HttpManifestEvidenceSource:
|
|||||||
def _remember(
|
def _remember(
|
||||||
self,
|
self,
|
||||||
provenance: str,
|
provenance: str,
|
||||||
|
final_url: str,
|
||||||
document: AcquiredDocument,
|
document: AcquiredDocument,
|
||||||
validators: tuple[str | None, str | None],
|
validators: tuple[str | None, str | None],
|
||||||
) -> None:
|
) -> None:
|
||||||
@@ -162,7 +166,7 @@ class HttpManifestEvidenceSource:
|
|||||||
self._cache_bytes -= len(old.content)
|
self._cache_bytes -= len(old.content)
|
||||||
self._cache[source_id] = document
|
self._cache[source_id] = document
|
||||||
self._cache_bytes += len(document.content)
|
self._cache_bytes += len(document.content)
|
||||||
self._validators[provenance] = validators
|
self._validators[provenance] = (final_url, *validators)
|
||||||
while self._cache and self._cache_bytes > self.max_cache_bytes:
|
while self._cache and self._cache_bytes > self.max_cache_bytes:
|
||||||
evicted_id, evicted = self._cache.popitem(last=False)
|
evicted_id, evicted = self._cache.popitem(last=False)
|
||||||
self._cache_bytes -= len(evicted.content)
|
self._cache_bytes -= len(evicted.content)
|
||||||
@@ -172,9 +176,9 @@ class HttpManifestEvidenceSource:
|
|||||||
|
|
||||||
def _download(self, transport_url: str, provenance: str) -> AcquiredDocument:
|
def _download(self, transport_url: str, provenance: str) -> AcquiredDocument:
|
||||||
current = transport_url
|
current = transport_url
|
||||||
headers = self._conditional_headers(provenance)
|
|
||||||
try:
|
try:
|
||||||
for redirect_count in range(self.max_redirects + 1):
|
for redirect_count in range(self.max_redirects + 1):
|
||||||
|
headers = self._conditional_headers(provenance, current)
|
||||||
allowed = self._resolve_allowed(current)
|
allowed = self._resolve_allowed(current)
|
||||||
response = None
|
response = None
|
||||||
try:
|
try:
|
||||||
@@ -194,25 +198,18 @@ class HttpManifestEvidenceSource:
|
|||||||
self._validate_url_shape(destination)
|
self._validate_url_shape(destination)
|
||||||
except ValueError as error:
|
except ValueError as error:
|
||||||
raise self._safe_error("redirect") from error
|
raise self._safe_error("redirect") from error
|
||||||
current_origin = urlsplit(current)
|
# The next iteration binds validators to the exact destination URL.
|
||||||
destination_origin = urlsplit(destination)
|
|
||||||
if (
|
|
||||||
current_origin.scheme,
|
|
||||||
current_origin.hostname,
|
|
||||||
current_origin.port,
|
|
||||||
) != (
|
|
||||||
destination_origin.scheme,
|
|
||||||
destination_origin.hostname,
|
|
||||||
destination_origin.port,
|
|
||||||
):
|
|
||||||
headers = {}
|
|
||||||
# Resolve every hop independently. Validators are never forwarded across
|
|
||||||
# origins, where even an opaque ETag would become cross-origin state.
|
|
||||||
current = destination
|
current = destination
|
||||||
continue
|
continue
|
||||||
if response.status_code == 304:
|
if response.status_code == 304:
|
||||||
cached = self._cache.get(self._source_id(provenance))
|
cached = self._cache.get(self._source_id(provenance))
|
||||||
if cached is None:
|
binding = self._validators.get(provenance)
|
||||||
|
if (
|
||||||
|
cached is None
|
||||||
|
or not headers
|
||||||
|
or binding is None
|
||||||
|
or binding[0] != current
|
||||||
|
):
|
||||||
raise self._safe_error("conditional_response")
|
raise self._safe_error("conditional_response")
|
||||||
self._cache.move_to_end(cached.source.source_id)
|
self._cache.move_to_end(cached.source.source_id)
|
||||||
return cached
|
return cached
|
||||||
@@ -273,7 +270,7 @@ class HttpManifestEvidenceSource:
|
|||||||
media_type=media_type,
|
media_type=media_type,
|
||||||
acquired_at=datetime.now(UTC),
|
acquired_at=datetime.now(UTC),
|
||||||
)
|
)
|
||||||
self._remember(provenance, document, (etag, last_modified))
|
self._remember(provenance, current, document, (etag, last_modified))
|
||||||
return document
|
return document
|
||||||
|
|
||||||
def discover(self):
|
def discover(self):
|
||||||
|
|||||||
Reference in New Issue
Block a user