Skip to content

Compute ETag from (mtime, size) to skip whole-file load per request#212

Merged
swhitty merged 1 commit into
swhitty:mainfrom
ianegordon:ian/tvt-290-filehttphandler-loads-whole-file-into-memory-to-compute-etag
Apr 28, 2026
Merged

Compute ETag from (mtime, size) to skip whole-file load per request#212
swhitty merged 1 commit into
swhitty:mainfrom
ianegordon:ian/tvt-290-filehttphandler-loads-whole-file-into-memory-to-compute-etag

Conversation

@ianegordon

@ianegordon ianegordon commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Note that this PR exchanges performance for accuracy. It changes ETag to be based on modification time & size rather than a proper SHA. This technique is also used by Apache and nginx.

This PR would also simplify updating the directory handler to adopt streaming as the file handler already does.

Summary

  • Replace SHA-256-over-contents ETag with "<hex-mtime>-<hex-size>", matching nginx (ngx_http_set_etag in src/http/ngx_http_core_module.c) and Apache HTTPD's default FileETag MTime Size.
  • FileHTTPHandler and DirectoryHTTPHandler no longer call Data(contentsOf:) solely to compute an ETag.
  • Strong validator (no W/ prefix) so If-Range keeps working for FileHTTPHandler's Range path.

Closes TVT-290.

Test plan

  • New HTTPCacheControlTests (6 tests): nil for missing file; quoted strong format with hex-hex inner; stable for unchanged file; differs on size change; differs on mtime change; collides when mtime+size match but contents differ (the property that distinguishes a metadata ETag from a content-hash ETag).
  • swift test — 435 tests pass.
  • swift build — clean, no warnings.

🤖 Generated with Claude Code

nginx (ngx_http_set_etag) and Apache HTTPD's default FileETag MTime Size
both derive a static-file ETag from the same metadata. Doing the same
here lets FileHTTPHandler and DirectoryHTTPHandler skip the per-request
Data(contentsOf:) + SHA-256 load.

Closes TVT-290

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

codecov Bot commented Apr 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.74%. Comparing base (5295843) to head (8fd1c5d).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
FlyingFox/Sources/HTTPCacheControl.swift 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #212      +/-   ##
==========================================
- Coverage   92.75%   92.74%   -0.02%     
==========================================
  Files          70       70              
  Lines        3645     3653       +8     
==========================================
+ Hits         3381     3388       +7     
- Misses        264      265       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@swhitty swhitty merged commit 05b6b66 into swhitty:main Apr 28, 2026
12 of 13 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.

2 participants