fix(storage): fall back to range GET when proxy strips Content-Length

[tylere]

Summary

• The data proxy omits Content-Length from HEAD responses, so getObjectInfo() always sees ContentLength as undefined → 0, and ObjectSummary displays 0 B for every object
• When ContentLength is absent (or 0) from the HEAD, issue a GET with Range: bytes=0-0 and read the actual total from the Content-Range response header (bytes 0-0/TOTAL)
• Failures in the fallback are swallowed so objects on proxies that don't support range requests still return gracefully with size: 0

Fixes #509

Test plan

• Existing getObjectInfo tests still pass
• New test: falls back to range GET when HEAD omits Content-Length
• New test: falls back to range GET when ContentLength is 0
• New test: keeps size as 0 when the range GET also fails
• Manual: visit https://source.coop/geospatialml/fmow/index.html after deploy — Size should show ~5 KB instead of 0 B

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
source-coop-ui	Ready	Preview	Aug 31, 2026 6:14am
source-cooperative	Ready	Preview	Aug 31, 2026 6:14am

[claude]

Claude finished @alukach's task in 1m 52s —— View job

---

Review complete

• Gather context (diff, changed files)
• Review src/lib/storage/s3.ts changes for correctness/security
• Review src/lib/storage/s3.test.ts changes
• Apply ponytail over-engineering pass
• Post final review

---

❌ Changes requested — see findings below.

• src/lib/storage/s3.ts:211-213 — When the range GET doesn't return a parseable Content-Range (e.g. a proxy that ignores the Range header and returns the full object with 200), the code still fully drains rangeResponse.Body via for await. On this platform's typical large geospatial files, that means silently streaming the entire object over the wire just to fail to compute a size — on every page view of that object, since the whole point of this fallback is a proxy that already misbehaves on headers. Prefer destroying the stream when the response wasn't a partial (206)/didn't include Content-Range, instead of consuming it:

  const match = rangeResponse.ContentRange?.match(/\/(\d+)$/);
  if (match) {
    size = parseInt(match[1], 10);
    // Drain the 1-byte partial body to release the connection.
    for await (const _ of rangeResponse.Body as Readable) {
      // no-op
    }
  } else {
    // No Content-Range means the proxy likely ignored the Range header and
    // sent the full body — destroy rather than download it.
    (rangeResponse.Body as Readable)?.destroy();
  }

• src/lib/storage/s3.ts:192 — if (!size) treats a legitimately empty (0-byte) object the same as a missing Content-Length, triggering an extra range GET for every empty file. Since S3 range GETs on a 0-byte object return 416, this is caught and falls back to 0 anyway, so it's not incorrect — just a wasted round-trip on every HEAD of an empty file. Minor; not blocking.

Everything else looks solid: the regex/parseInt extraction of the total from Content-Range is correct, the fallback is properly scoped to only fire when ContentLength is falsy, and failures are swallowed so proxies without range support still degrade gracefully to size: 0. Tests cover the three relevant paths (missing Content-Length, Content-Length: 0, and range GET failure) and correctly assert both the request shape and the parsed result.

Simplify (ponytail): nothing to report — the fallback is a minimal try/catch + regex, no unneeded abstractions or dependencies.

---

💰 Estimated review cost: $0.31 · 1m52s · 15 turns

[alukach]

@claude address the PR review (#511 (comment)) and also consider if we could avoid interacting with the request body in entirety and instead only review the response headers

[claude]

Claude finished @alukach's task in 1m 42s —— View job

---

Addressed the review

• Read the review feedback and current s3.ts implementation
• Fix: destroy the range-GET body instead of draining/downloading it
• Update/verify tests
• Push
• Run test suite — couldn't; see note below

Change (src/lib/storage/s3.ts:206-214, commit 02274a6):

-          // Content-Range format: "bytes 0-0/TOTAL_SIZE"
+          // Content-Range format: "bytes 0-0/TOTAL_SIZE". That header is all
+          // we need, so destroy the body instead of reading it — a proxy that
+          // ignores Range and returns the full object shouldn't be downloaded
+          // just to size it.
           const match = rangeResponse.ContentRange?.match(/\/(\d+)$/);
           if (match) {
             size = parseInt(match[1], 10);
           }
-          for await (const _ of rangeResponse.Body as Readable) {
-            // drain the 1-byte body to avoid connection leaks
-          }
+          (rangeResponse.Body as Readable | undefined)?.destroy();

This covers both asks:

• The review's blocking finding: a proxy that ignores Range and returns a full 200 no longer gets fully streamed to compute a size — destroy() tears down the stream/socket regardless of what came back.
• Your follow-up question ("avoid interacting with the request body in entirety, only review headers"): rather than conditionally draining-vs-destroying based on whether Content-Range parsed, the simplest fix is to never read the body at all in either case — we only ever needed the Content-Range header, so destroy() unconditionally. That's a smaller diff than branching on partial (206) vs. full (200) responses and gets the same result: zero bytes of object data are read just to size it.

The review's second point (if (!size) also retrying on a legitimate 0-byte object) was called out as non-blocking, so left as-is — a wasted round-trip on an empty file's HEAD isn't worth the extra branching.

Note: I couldn't run npm test/jest in this session — the sandbox declined to approve any test-runner command (npx jest, npm test, ./node_modules/.bin/jest all required approval that wasn't available non-interactively). The existing test getObjectInfo falls back to a range GET when HEAD omits Content-Length mocks Body: Readable.from(...), which supports .destroy(), so it should still pass, but this needs a maintainer's CI run (or local npm test) to confirm. If you'd like me to be able to run tests directly in future runs, the --allowedTools list would need Bash(npm test:*) or similar added.
· branch