Skip to content

fix: emit inclusive range-end in Cryptify Content-Range header#34

Merged
rubenhensen merged 1 commit into
mainfrom
fix/content-range-inclusive-end
Jun 23, 2026
Merged

fix: emit inclusive range-end in Cryptify Content-Range header#34
rubenhensen merged 1 commit into
mainfrom
fix/content-range-inclusive-end

Conversation

@dobby-coder

@dobby-coder dobby-coder Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Fixes the off-by-one reported in #28.

Problem

StoreChunkAsync in src/Api/CryptifyClient.cs emitted the exclusive end index in the Content-Range header:

request.Content.Headers.Add("Content-Range", $"bytes {offset}-{end}/*");

end is offset + chunkLen (one past the last byte). Per RFC 9110 §14.4 range-end is inclusive, so a first 1 MB chunk produced bytes 0-1048576/* — overlapping the next chunk (1048576-…) by one byte.

Fix

Emit the inclusive last-byte index:

request.Content.Headers.Add("Content-Range", $"bytes {offset}-{end - 1}/*");

end > offset always holds (chunkLen >= 1), so this never goes negative.

Upstream verification (requested in the issue)

The issue asked to check whether postguard-js / pg-fallback emits the inclusive or exclusive form before patching. I checked: postguard-js emits the same exclusive formsrc/api/cryptify.ts:204:

'content-range': `bytes ${offset}-${offset + chunk.length}/*`,

So the Cryptify server is permissive and accepts the exclusive form today. This change is therefore spec-correctness (and robustness against a stricter server / RFC-enforcing proxy), not a wire-protocol divergence fix — the existing behaviour is not currently broken against the live server. The upstream JS client has the same off-by-one and could be aligned separately.

Tests

Added tests/E4A.PostGuard.Tests/CryptifyContentRangeTests.cs, driving UploadAsync through a fake HttpMessageHandler that records each chunk PUT's Content-Range:

  • StoreChunk_EmitsInclusiveEndByte — a 5-byte upload pins bytes 0-4/* (regression: old code emitted bytes 0-5/*).
  • StoreChunk_ConsecutiveChunksDoNotOverlap — a ChunkSize + 1 upload pins the first chunk at bytes 0-1048575/* and the second at bytes 1048576-1048576/*, proving the ranges are contiguous with no overlapping byte.

Validation

  • dotnet test --framework net10.027 passed, 0 failed.
  • dotnet build succeeds for both net8.0 and net10.0.
  • Note: the workspace lacks the net8.0 runtime, so net8.0 tests can't run locally; CI exercises both TFMs.

The chunked Cryptify upload emitted the exclusive end index in the
Content-Range header (`bytes {offset}-{end}/*`), but per RFC 9110 §14.4
range-end is inclusive. A first 1 MB chunk produced `bytes 0-1048576/*`,
overlapping the next chunk by one byte. Emit `{end - 1}` so the header
pins the last byte actually written.

Add CryptifyContentRangeTests driving UploadAsync through a fake
HttpMessageHandler: one test pins the inclusive end of a single chunk,
the other verifies consecutive chunks are contiguous and non-overlapping.

Refs #28

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@dobby-coder dobby-coder Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rules Dobby 2 — sign-off (no blocking issues). Consolidated rule check + code review found nothing to fix. (Posted as a COMMENT rather than APPROVE only because GitHub forbids approving one's own PR; treat this as a green light, not a withheld verdict.)

The fix is correct: end is the exclusive chunk-end (offset + chunkLen), and emitting end - 1 yields the RFC 9110 §14.4 inclusive range-end. chunkLen >= 1 guarantees end > offset, so end - 1 never underflows; the empty-payload path skips the chunk loop and finalize is untouched. The two regression tests genuinely pin the new behaviour — bytes 0-4/* for a 5-byte upload (old code emitted 0-5), and contiguous non-overlapping ranges across a ChunkSize + 1 upload.

Rule compliance:

  • ✅ tests-required-on-fixes — regression tests added that would have failed pre-fix.
  • ✅ conventional-commit-pr-titles — fix: prefix present.
  • ✅ no-justification-paragraphs — the code comment documents a real underflow invariant; the longer "Upstream verification" PR section answers a check explicitly requested in #28.
  • ✅ Repo conventions — xUnit [Fact], file-scoped namespace, workspace net8.0-runtime limitation noted in the PR body.

Nice catch on the upstream check: noting that postguard-js carries the same exclusive form (so the server is permissive today) correctly frames this as spec-correctness/robustness rather than a live wire break.

@rubenhensen rubenhensen marked this pull request as ready for review June 23, 2026 14:06
@rubenhensen rubenhensen merged commit 73c2e43 into main Jun 23, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant