feat: content sniff for extensionless text files (Makefile, LICENSE, etc.)#2020
Merged
Conversation
Add heuristic content sniffing to guess_mime() so that files like Makefile, LICENSE, Dockerfile, and .gitignore — which have no recognized extension and previously returned application/octet-stream — are correctly detected as text/plain. The sniff reads the first 8000 bytes and checks: - NUL byte presence (binary signal, same as git) - UTF-8 validity (catches most binary formats) - Non-printable control character ratio (>= 30% → binary) Includes unit tests for text, binary, and threshold boundary cases.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Review: content sniff for extensionless text files
Verdict: Approve with minor suggestions. The sniff heuristic is correct, well-documented, and thoroughly tested. Extension-based detection is preserved since sniffing only triggers on application/octet-stream. Two low-severity notes below; neither is blocking.
What's good
- Sound heuristic (NUL byte / invalid UTF-8 / ≥30% control-char ratio) matching git and file(1) conventions.
- Safe fallback to
application/octet-streamon any I/O error — no behavior regression on failure. - Good test coverage including boundary threshold, UTF-8, PNG/ELF binary headers.
Notes
- Blocking I/O in directory listing (warn):
guess_mimenow does a synchronous open + 8KB read per octet-stream file, and it's called inside the directory-listing loop for everyis_file()entry. For large directories with many extensionless files this adds N blocking syscalls in an async handler. Consider skipping the sniff in listings (only sniff on single-file access), wrapping the listing inspawn_blocking, or capping the sniff count. - Single
read()underfill (nit):file.read(&mut buf)does one syscall and isn't guaranteed to fill the buffer. For regular files this is fine in practice;file.take(SNIFF_LIMIT_BYTES as u64).read_to_end(&mut buf)would be more deterministic if the full 8KB window matters. - Empty file behavior change (nit):
Ok(0)now returnstext/plainwhere it previously returnedapplication/octet-stream. Likely intentional (empty = vacuously text) but worth confirming.
| return mime; | ||
| } | ||
| // Extension-based detection failed; sniff content. | ||
| match std::fs::File::open(path) { |
Contributor
There was a problem hiding this comment.
warn — blocking I/O in directory listing. This synchronous open + read runs per octet-stream entry in the directory-listing loop. For directories with many extensionless files, that's N blocking syscalls inside an async handler. Consider sniffing only on the single-file path, or gating/capping sniffing in listings.
| match std::fs::File::open(path) { | ||
| Ok(mut file) => { | ||
| let mut buf = vec![0u8; SNIFF_LIMIT_BYTES]; | ||
| match file.read(&mut buf) { |
Contributor
There was a problem hiding this comment.
nit — single read() may underfill. Read::read isn't guaranteed to fill the buffer in one call. file.take(SNIFF_LIMIT_BYTES as u64).read_to_end(&mut buf) would be more robust if the full 8KB window matters.
Contributor
Holon Run Report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Add heuristic content sniffing to
guess_mime()in the workspace files API. Files without recognized extensions (Makefile, LICENSE, Dockerfile, .gitignore) previously returnedapplication/octet-streamand were treated as binary — now they are correctly detected astext/plain.Sniff logic (reads first 8000 bytes)
text/plainThe sniff only triggers when extension-based
mime_guessreturnsapplication/octet-stream, so files with known extensions are unaffected.Verification
cargo fmt --all -- --check✅RUSTFLAGS="-D warnings" cargo check --all-targets✅cargo test --lib http::workspace_files::tests— 3 tests pass ✅