diff --git a/tests/test_data_origin_contract.py b/tests/test_data_origin_contract.py new file mode 100644 index 00000000..e7517100 --- /dev/null +++ b/tests/test_data_origin_contract.py @@ -0,0 +1,172 @@ +""" +HTTP contract tests for the data origin (`data.isamples.org`). + +WHY THIS FILE EXISTS +-------------------- +The Explorer's whole architecture rests on one assumption: DuckDB-WASM fetches +only the byte ranges a query touches. In August 2026 we discovered that had been +silently false — a cold load transferred **~74 MB instead of ~3 MB** because +DuckDB's range-support probe was answered in a way it did not accept, so it fell +back to downloading whole files (#345). + +That regression was invisible to every existing test. It was also *mis-cleared* +in June (`ISSUE_313_FINDINGS_2026-06-26.md`) by a `curl` probe that used **GET** +where DuckDB uses **HEAD** — the check looked right and proved nothing. + +These tests are deliberately cheap: no browser, no DuckDB, a handful of bytes on +the wire. They run before the Playwright smoke gate so this class of failure is +caught before anything downloads 74 MB to discover it. + +MAINTENANCE NOTE +---------------- +`tools/build_release_manifest.py` performs the ranged-GET probe but NOT the HEAD +probe — which is exactly why it could report a healthy origin while DuckDB was +broken. If you touch either, keep them in sync. +""" +import json +import os +import pathlib + +import pytest +import requests + +ORIGIN = os.environ.get("ISAMPLES_DATA_ORIGIN", "https://data.isamples.org") +PAGE_ORIGIN = "https://isamples.org" +MANIFEST = pathlib.Path(__file__).resolve().parent.parent / "isamples_202608_release_manifest.json" + +# The Worker 403s some default user agents; identify honestly. +UA = {"User-Agent": "isamples-ci-contract/1.0 (+https://isamples.org)"} +TIMEOUT = 30 + + +def _manifest_files(): + if not MANIFEST.exists(): + pytest.skip(f"release manifest not found at {MANIFEST}") + return json.loads(MANIFEST.read_text())["files"] + + +def _boot_critical_large_file(): + """The biggest boot-critical parquet — the one a full read actually hurts. + + Skips (rather than fails) if the origin does not serve it. Without this a + 404 from, say, a partially-seeded local test origin gets reported as + "the shim has widened", which is a confidently wrong diagnosis — the exact + failure mode this whole file exists to prevent. + """ + files = _manifest_files() + name = None + for candidate in ("isamples_202608_samples_map_lite_v3.parquet", + "isamples_202608_sample_facet_masks.parquet"): + if candidate in files: + name = candidate + break + if name is None: + name = max(files.items(), key=lambda kv: kv[1].get("size_bytes", 0))[0] + + probe = requests.head(f"{ORIGIN}/{name}", headers=UA, timeout=TIMEOUT) + if probe.status_code == 404: + pytest.skip(f"{ORIGIN} does not serve {name} (404) — not a contract failure") + return name, files[name]["size_bytes"] + + +def test_ranged_get_returns_206_with_exact_content_range(): + """The load-bearing property: partial GETs work and report the full size. + + This one passes today and has always passed — which is precisely why it was + not enough on its own. Keep it: if it ever breaks, range reads are dead. + """ + name, size = _boot_critical_large_file() + r = requests.get(f"{ORIGIN}/{name}", + headers={**UA, "Range": "bytes=0-0", "Origin": PAGE_ORIGIN}, + timeout=TIMEOUT) + assert r.status_code == 206, f"ranged GET returned {r.status_code}, not 206" + assert r.headers.get("Content-Range") == f"bytes 0-0/{size}", ( + f"Content-Range {r.headers.get('Content-Range')!r} disagrees with the " + f"manifest size {size}") + assert len(r.content) == 1, f"ranged GET returned {len(r.content)} bytes, expected 1" + + +def test_cors_exposes_headers_the_explorer_must_read(): + """Cross-origin JS cannot see Content-Range/Accept-Ranges unless exposed.""" + name, _ = _boot_critical_large_file() + r = requests.get(f"{ORIGIN}/{name}", + headers={**UA, "Range": "bytes=0-0", "Origin": PAGE_ORIGIN}, + timeout=TIMEOUT) + exposed = (r.headers.get("Access-Control-Expose-Headers") or "").lower() + for h in ("content-range", "accept-ranges", "content-length"): + assert h in exposed, f"{h} not in Access-Control-Expose-Headers ({exposed!r})" + assert r.headers.get("Access-Control-Allow-Origin") in ("*", PAGE_ORIGIN) + + +@pytest.mark.xfail( + reason="#345 shim not deployed yet — remove this marker once the Worker " + "change is live on data.isamples.org. Until then the Explorer " + "downloads whole files (~74 MB cold instead of ~3 MB).", + strict=False, +) +def test_duckdb_124_head_range_compatibility(): + """DuckDB-WASM 1.24.0's range-support probe must be answered with 206. + + *** THIS ASSERTS A DELIBERATE STANDARDS DIVERGENCE. *** + + RFC 9110 section 14.2: Range is defined only for GET, and a server MUST + IGNORE it on other methods including HEAD. A plain 200 here is the CORRECT + HTTP answer. But DuckDB-WASM 1.24.0 — the version Quarto's OJS runtime pins + — probes with exactly `HEAD` + `Range: bytes=0-` and treats anything other + than 206 as "this server cannot do partial reads", then downloads whole + files. + + So this test encodes a compatibility shim, not correct HTTP. It should be + DELETED, along with the Worker shim, once the Explorer no longer depends on + that probe (i.e. when it stops using the pinned duckdb-wasm and does its own + init on a conformant version). See #345. + """ + name, size = _boot_critical_large_file() + r = requests.head(f"{ORIGIN}/{name}", + headers={**UA, "Range": "bytes=0-", "Origin": PAGE_ORIGIN}, + timeout=TIMEOUT) + assert r.status_code == 206, ( + f"HEAD+Range returned {r.status_code}. DuckDB-WASM will fall back to full " + f"HTTP reads and the Explorer will transfer tens of MB on cold load.") + assert r.headers.get("Content-Range") == f"bytes 0-{size - 1}/{size}" + assert r.headers.get("Content-Length") == str(size) + assert not r.content, "HEAD must not return a body" + + +def test_shim_does_not_widen_to_other_ranged_heads(): + """The #345 shim must stay scoped to the exact probe shape. + + Any OTHER ranged HEAD must remain standards-correct (200, no Content-Range), + so the divergence cannot leak to other clients or harden into a contract we + did not intend to offer. + """ + name, _ = _boot_critical_large_file() + for rng in ("bytes=0-99", "bytes=100-199", "bytes=-100"): + r = requests.head(f"{ORIGIN}/{name}", + headers={**UA, "Range": rng, "Origin": PAGE_ORIGIN}, + timeout=TIMEOUT) + assert r.status_code == 200, ( + f"HEAD with {rng!r} returned {r.status_code}; the #345 shim has widened " + f"beyond the single DuckDB probe shape and is now diverging from RFC 9110 " + f"more than intended") + assert "Content-Range" not in r.headers, ( + f"HEAD with {rng!r} carried a Content-Range; see above") + + +def test_manifest_sizes_match_the_origin(): + """Catches the data/doc drift class: manifest says one size, origin serves another.""" + files = _manifest_files() + checked = 0 + for name, meta in files.items(): + if not name.endswith(".parquet") or "/" in name: + continue + size = meta.get("size_bytes") + if not size: + continue + r = requests.head(f"{ORIGIN}/{name}", headers=UA, timeout=TIMEOUT) + assert r.status_code == 200, f"{name}: HEAD returned {r.status_code}" + assert r.headers.get("Content-Length") == str(size), ( + f"{name}: manifest says {size} bytes, origin serves " + f"{r.headers.get('Content-Length')}") + checked += 1 + assert checked > 0, "no parquet entries checked — manifest shape may have changed" diff --git a/workers/data-isamples-org/deploy-canary.sh b/workers/data-isamples-org/deploy-canary.sh new file mode 100755 index 00000000..92e16bf6 --- /dev/null +++ b/workers/data-isamples-org/deploy-canary.sh @@ -0,0 +1,106 @@ +#!/usr/bin/env bash +# Deploy the #345 HEAD+Range shim to a NON-PRODUCTION canary and verify it. +# +# Safe by construction: uses wrangler.canary.toml, which has a different Worker +# name and NO routes, so it cannot land in front of data.isamples.org. The +# production wrangler.toml is never read or modified. +# +# ./deploy-canary.sh # deploy + verify + print the test URL +# ./deploy-canary.sh --verify # verify an already-deployed canary +# ./deploy-canary.sh --teardown # delete the canary +set -uo pipefail +cd "$(dirname "$0")" + +NAME="isamples-data-345-canary" +PROBE_FILE="isamples_202608_h3_summary_res4.parquet" +PROBE_SIZE=505651 +STAGING="https://rdhyee.github.io/isamplesorg.github.io" + +if [ "${1:-}" = "--teardown" ]; then + echo "Deleting canary Worker '$NAME'..." + npx wrangler delete --name "$NAME" + exit $? +fi + +# --- auth ------------------------------------------------------------------- +if ! npx wrangler whoami >/dev/null 2>&1; then + echo "Not logged in to Cloudflare." + echo "Run this in an interactive terminal first:" + echo + echo " npx wrangler login" + echo + echo "(or export CLOUDFLARE_API_TOKEN=... with Workers Scripts:Edit + R2 read)" + exit 1 +fi + +# --- deploy ----------------------------------------------------------------- +if [ "${1:-}" != "--verify" ]; then + echo "==> Deploying canary (NO routes, workers.dev only)" + npx wrangler deploy -c wrangler.canary.toml | tee /tmp/canary_deploy.log + echo +fi + +# The deploy output contains the workers.dev URL; recover it, else construct it. +URL=$(grep -oE 'https://[a-z0-9._-]*\.workers\.dev' /tmp/canary_deploy.log 2>/dev/null | head -1) +if [ -z "$URL" ]; then + SUB=$(npx wrangler whoami 2>/dev/null | grep -oE '[a-z0-9-]+\.workers\.dev' | head -1) + [ -n "$SUB" ] && URL="https://${NAME}.${SUB}" +fi +if [ -z "$URL" ]; then + echo "!! Could not determine the canary URL. Check /tmp/canary_deploy.log and pass it manually:" + echo " CANARY_URL=https://... $0 --verify" + URL="${CANARY_URL:-}" + [ -z "$URL" ] && exit 1 +fi + +echo "==> Canary URL: $URL" +echo + +# --- verify the handshake actually changed --------------------------------- +fail=0 +ok() { printf " ok %s\n" "$1"; } +no() { printf " FAIL %s (%s)\n" "$1" "$2"; fail=1; } + +echo "==> Verifying the DuckDB probe now gets 206" +S=$(curl -s -o /dev/null -w '%{http_code}' -I -H 'Range: bytes=0-' "$URL/$PROBE_FILE") +CR=$(curl -sI -H 'Range: bytes=0-' "$URL/$PROBE_FILE" | grep -i '^content-range:' | tr -d '\r' | cut -d' ' -f2-) +[ "$S" = "206" ] && ok "HEAD+Range 'bytes=0-' -> 206" || no "HEAD+Range -> 206" "got $S" +[ "$CR" = "bytes 0-$((PROBE_SIZE-1))/$PROBE_SIZE" ] && ok "Content-Range correct" \ + || no "Content-Range" "got '$CR'" + +echo +echo "==> Verifying the shim did NOT widen (these must stay standards-correct 200)" +for R in 'bytes=0-99' 'bytes=100-199' 'bytes=-100'; do + S=$(curl -s -o /dev/null -w '%{http_code}' -I -H "Range: $R" "$URL/$PROBE_FILE") + [ "$S" = "200" ] && ok "HEAD '$R' -> 200" || no "HEAD '$R' -> 200" "got $S" +done + +echo +echo "==> Verifying GET paths unchanged" +S=$(curl -s -o /dev/null -w '%{http_code}' -H 'Range: bytes=0-99' "$URL/$PROBE_FILE") +[ "$S" = "206" ] && ok "ranged GET -> 206" || no "ranged GET -> 206" "got $S" +S=$(curl -s -o /dev/null -w '%{http_code}' "$URL/$PROBE_FILE") +[ "$S" = "200" ] && ok "plain GET -> 200" || no "plain GET -> 200" "got $S" + +echo +if [ "$fail" -ne 0 ]; then + echo "VERIFICATION FAILED — do not promote to the data.isamples.org route." + exit 1 +fi +echo "All canary checks passed." +echo +echo "───────────────────────────────────────────────────────────────" +echo "Open the staging Explorer against the canary:" +echo +echo " $STAGING/explorer.html?data_base=$URL" +echo +echo "Measure it end-to-end (from the repo root):" +echo +echo " python tests/playwright/bandwidth_matrix.py $STAGING \\" +echo " --profiles unthrottled,3g-fast --budget 600 \\" +echo " --query \"?data_base=$URL\"" +echo +echo "Expect: ~3 MB instead of ~74 MB, and 0 'full HTTP read' fallbacks." +echo +echo "Tear down when done: $0 --teardown" +echo "───────────────────────────────────────────────────────────────" diff --git a/workers/data-isamples-org/src/index.js b/workers/data-isamples-org/src/index.js index 08bcaed3..709ba274 100644 --- a/workers/data-isamples-org/src/index.js +++ b/workers/data-isamples-org/src/index.js @@ -125,6 +125,40 @@ export default { if (request.method === 'HEAD') { headers.set('Content-Length', String(object.size)); + + // === #345 compatibility shim — a KNOWING, NARROW standards divergence === + // + // RFC 9110 §14.2: Range is defined only for GET, and a server MUST IGNORE + // Range on other methods including HEAD. So plain `200` here is CORRECT, + // and everything below is a deliberate exception, not a bug fix. + // + // Why we make it: DuckDB-WASM 1.24.0 (the version Quarto's OJS runtime + // pins) decides whether a server supports partial reads by sending + // exactly `HEAD` + `Range: bytes=0-` and requiring 206. On a 200 it logs + // "falling back to full HTTP read" and downloads WHOLE FILES. Measured on + // the live Explorer: 74 MB on a cold load instead of ~3 MB, and the facet + // panel taking 7 minutes on 3G instead of 1.5 (never, on slow 3G). + // + // Scope is deliberately as tight as it can be: + // - ONLY the exact probe shape `bytes=0-` (open-ended from zero) + // - any other ranged HEAD (bytes=0-99, bytes=100-199, bytes=-100) + // stays standards-correct at 200, so the divergence cannot leak to + // other clients or become an accidental contract + // + // REMOVAL PATH: delete this block once the Explorer no longer depends on + // that probe — i.e. when it stops using Quarto's pinned duckdb-wasm and + // does its own init on a version whose capability probe is conformant. + // Tracked in isamplesorg/isamplesorg.github.io#345. + // + // Note: if Workers Caching is ever enabled on this Worker, Cloudflare + // strips Range before invoking us and slices its own 206s — this shim + // would need re-testing (and may become unnecessary or ineffective). + const isDuckDbProbe = rangeHeader && /^bytes=0-$/.test(rangeHeader.trim()); + if (isDuckDbProbe && typeof object.size === 'number' && object.size > 0) { + headers.set('Content-Range', `bytes 0-${object.size - 1}/${object.size}`); + return new Response(null, { status: 206, headers }); + } + return new Response(null, { status: 200, headers }); } diff --git a/workers/data-isamples-org/test/range_contract.sh b/workers/data-isamples-org/test/range_contract.sh new file mode 100755 index 00000000..7daacd08 --- /dev/null +++ b/workers/data-isamples-org/test/range_contract.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash +# HTTP contract test for the data.isamples.org Worker, run against `wrangler dev --local`. +# +# Exists because of #345: the Worker answered 200 to a HEAD carrying a Range header, +# which is the exact probe DuckDB-WASM uses to decide whether a server supports +# partial reads. Answering 200 made it download whole files — 74 MB on a cold +# Explorer load instead of ~3 MB. +# +# Setup (once): +# curl -H 'User-Agent: isamples-worker-test/1.0' \ +# -o /tmp/test_res4.parquet \ +# https://data.isamples.org/isamples_202608_h3_summary_res4.parquet +# npx wrangler r2 object put isamples-ry/isamples_202608_h3_summary_res4.parquet \ +# --file=/tmp/test_res4.parquet --local +# +# Run: +# npx wrangler dev --local --port 8787 & +# ./test/range_contract.sh 8787 +set -uo pipefail + +PORT="${1:-8787}" +BASE="http://127.0.0.1:${PORT}" +KEY="isamples_202608_h3_summary_res4.parquet" +SIZE=505651 + +pass=0; fail=0 +check() { # check + if [ "$2" = "$3" ]; then printf " ok %-52s %s\n" "$1" "$3"; pass=$((pass+1)) + else printf " FAIL %-52s expected=%s actual=%s\n" "$1" "$2" "$3"; fail=$((fail+1)); fi +} +status() { curl -s -o /dev/null -w '%{http_code}' "$@"; } +header() { local h="$1"; shift; curl -sI "$@" | grep -i "^${h}:" | head -1 | cut -d' ' -f2- | tr -d '\r'; } +ghdr() { local h="$1"; shift; curl -s -D - -o /dev/null "$@" | grep -i "^${h}:" | head -1 | cut -d' ' -f2- | tr -d '\r'; } +bodylen() { curl -s -o /dev/null -w '%{size_download}' "$@"; } + +# #345 — a NARROW, deliberately nonstandard compatibility shim. +# +# RFC 9110 §14.2 is explicit: Range is defined only for GET, and a server MUST +# IGNORE Range on other methods including HEAD. So 200 is the CORRECT answer and +# these assertions encode a knowing divergence, scoped as tightly as possible: +# only the exact probe DuckDB-WASM 1.24.0 sends (`Range: bytes=0-`) is answered +# 206. Every other ranged HEAD stays standards-correct at 200, so the divergence +# cannot leak to other clients. Remove this shim when the Explorer no longer +# depends on that probe (see the removal path in the issue). +echo "=== #345 shim: ONLY the exact DuckDB probe (Range: bytes=0-) gets 206 ===" +check "probe HEAD status" "206" "$(status -I -H 'Range: bytes=0-' "$BASE/$KEY")" +check "probe HEAD Content-Range" "bytes 0-$((SIZE-1))/$SIZE" "$(header content-range -H 'Range: bytes=0-' "$BASE/$KEY")" +check "probe HEAD sends no body" "0" "$(bodylen -I -H 'Range: bytes=0-' "$BASE/$KEY")" + +echo +echo "=== the shim must NOT widen: other ranged HEADs stay standards-correct (200) ===" +check "HEAD bytes=0-99 status" "200" "$(status -I -H 'Range: bytes=0-99' "$BASE/$KEY")" +check "HEAD bytes=0-99 no CR" "" "$(header content-range -H 'Range: bytes=0-99' "$BASE/$KEY")" +check "HEAD suffix range status" "200" "$(status -I -H 'Range: bytes=-100' "$BASE/$KEY")" +check "HEAD mid-range status" "200" "$(status -I -H 'Range: bytes=100-199' "$BASE/$KEY")" + +echo +echo "=== must not regress: plain HEAD stays 200 with full Content-Length ===" +check "HEAD status" "200" "$(status -I "$BASE/$KEY")" +check "HEAD Content-Length" "$SIZE" "$(header content-length "$BASE/$KEY")" +check "HEAD Accept-Ranges" "bytes" "$(header accept-ranges "$BASE/$KEY")" + +echo +echo "=== must not regress: GET paths ===" +check "GET status" "200" "$(status "$BASE/$KEY")" +check "GET body size" "$SIZE" "$(bodylen "$BASE/$KEY")" +check "ranged GET status" "206" "$(status -H 'Range: bytes=0-99' "$BASE/$KEY")" +check "ranged GET body size" "100" "$(bodylen -H 'Range: bytes=0-99' "$BASE/$KEY")" +check "ranged GET Content-Range" "bytes 0-99/$SIZE" "$(ghdr content-range -H 'Range: bytes=0-99' "$BASE/$KEY")" + +echo +echo "=== must not regress: caching + CORS contract (the Worker's raison d'etre) ===" +check "immutable Cache-Control" "public, max-age=31536000, immutable" "$(header cache-control "$BASE/$KEY")" +check "CC same on HEAD+Range" "public, max-age=31536000, immutable" "$(header cache-control -H 'Range: bytes=0-' "$BASE/$KEY")" +check "CORS allow-origin" "*" "$(header access-control-allow-origin "$BASE/$KEY")" +check "exposes Content-Range" "Content-Length, Content-Range, Accept-Ranges, ETag" \ + "$(header access-control-expose-headers -H 'Range: bytes=0-' "$BASE/$KEY")" +check "OPTIONS preflight" "204" "$(status -X OPTIONS "$BASE/$KEY")" +check "404 for missing key" "404" "$(status "$BASE/no_such_file.parquet")" + +echo +echo "=== ETag must be stable across methods (cache correctness) ===" +E_GET=$(ghdr etag "$BASE/$KEY"); E_HEAD=$(header etag "$BASE/$KEY"); E_HR=$(header etag -H 'Range: bytes=0-' "$BASE/$KEY") +check "ETag HEAD == GET" "$E_GET" "$E_HEAD" +check "ETag HEAD+Range == GET" "$E_GET" "$E_HR" + +echo +echo "passed=$pass failed=$fail" +[ "$fail" -eq 0 ] diff --git a/workers/data-isamples-org/wrangler.canary.toml b/workers/data-isamples-org/wrangler.canary.toml new file mode 100644 index 00000000..a45e5278 --- /dev/null +++ b/workers/data-isamples-org/wrangler.canary.toml @@ -0,0 +1,39 @@ +# Canary config for testing the #345 HEAD+Range shim. +# +# WHY A SEPARATE FILE: the production wrangler.toml binds this Worker to the +# `data.isamples.org/*` ROUTE. There is no staging data host — a plain +# `wrangler deploy` therefore goes live for every consumer of that hostname +# immediately. Rather than ask anyone to comment out the routes block by hand +# (easy to forget, easy to half-revert, and a mistake is a production incident), +# the canary gets its own config with a DIFFERENT NAME and NO ROUTES. It is +# reachable only at its workers.dev URL. +# +# Deploy: +# npx wrangler deploy -c wrangler.canary.toml +# +# Tear down when finished: +# npx wrangler delete --name isamples-data-345-canary +# +# NOTE: same R2 bucket as production, but this Worker only ever reads. + +name = "isamples-data-345-canary" +main = "src/index.js" +compatibility_date = "2026-04-01" + +# Raymond.yee@gmail.com's account — owner of the isamples.org zone and the +# isamples-ry R2 bucket. +account_id = "75e8a095c424e5a4e18fd6f5e6145064" + +# DELIBERATELY NO `routes` KEY. +# Adding one here would put the canary in front of data.isamples.org, which is +# the exact thing this file exists to prevent. + +# Serve on ..workers.dev so it is addressable for testing. +workers_dev = true + +[observability] +enabled = true + +[[r2_buckets]] +binding = "BUCKET" +bucket_name = "isamples-ry"