Keep the debug-history archive compressed on download - #4898
Open
chalfontchubby wants to merge 3 commits into
Open
Keep the debug-history archive compressed on download#4898chalfontchubby wants to merge 3 commits into
chalfontchubby wants to merge 3 commits into
Conversation
The bulk download arrived as a bare .tar rather than the gzip tarball that was sent, because browsers that unarchive downloads do it by extension and .tgz is one they recognise. That is fatal twice over. Measured on a real 15-snapshot history: 32MB of yaml, 4.6MB as a tarball. Unpacked it is both over GitHub's 25MB attachment limit and a file type GitHub rejects outright - .zip, .gz and .tgz are accepted, .tar is not - so the bug report this feature exists to gather cannot be attached at all. Compression is doing essential work here, and the download has to reach the user still compressed. There is no server-side way to decline the unarchiving: it keys on the extension, not on the content type or Content-Disposition. Serving it as application/octet-stream with nosniff was not enough on its own, though both are kept as defence in depth and bring this route in line with the single-snapshot download, which already served octet-stream. So the archive is named predbat_debug_history.tgz.dmp. The unarchiving leaves .dmp alone, GitHub accepts it, and the body is an ordinary gzip tarball that "tar xzf" reads whatever it is called, so nothing needs renaming to open it. The Debug panel link stays a plain "Download all" - the file is meant to be opaque and handed straight to an issue. The details are documented for the few who need to open one themselves. Tests assert the content type, nosniff, the .tgz.dmp filename, and separately that the body still starts with the gzip magic number, so serving it as opaque binary cannot quietly become serving something that is not an archive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The updated docs include an incomplete tar xzf extraction command (missing filename) and one nearby note still refers to .tgz rather than .tgz.dmp, which can mislead users.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes the /debug_history_download_all bulk debug-history download so it remains gzip-compressed when saved by browsers that auto-unpack archives based on filename extension, keeping the download small enough and in an attachment-friendly form for GitHub issue uploads.
Changes:
- Serve the bulk download as opaque binary (
application/octet-stream) withX-Content-Type-Options: nosniffand a.tgz.dmpfilename to prevent browser auto-unarchiving. - Update the web UI label to keep “Download all” user-facing while changing the on-wire filename.
- Add/adjust unit tests and docs to assert/describe the new headers/filename behavior and gzip-ness of the body.
File summaries
| File | Description |
|---|---|
| docs/web-interface.md | Documents the new bulk-download filename and rationale. |
| docs/customisation.md | Updates debug-history documentation to describe the .tgz.dmp download behavior. |
| apps/predbat/web.py | Changes the bulk download response headers/content-type and download filename to keep archives compressed on save. |
| apps/predbat/tests/test_web_debug_history_routes.py | Extends tests to assert opaque-binary response, nosniff, .tgz.dmp filename, and gzip magic bytes. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
chalfontchubby
added
the
BOT_CLEANUP
springfall2008
removed
the
BOT_CLEANUP
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change directly addresses the reported browser behavior, keeps the download usable for GitHub attachments, and is covered by targeted unit tests that verify headers, filename, and gzip payload integrity.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
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
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.
The problem
The debug-history bulk download (
/debug_history_download_all, added for #4417) arrives as a bare.tarrather than the gzip tarball that was sent. Browsers that unarchive downloads do it by extension, and.tgzis one they recognise. Reproduced on a live deployment.That breaks the feature's whole purpose, twice over. Measured on a real 15-snapshot history:
.tgz).tar)GitHub's attachment limit is 25 MB, and its allowed archive types are
.zip,.gzand.tgz—.taris not on the list. So the expanded download is rejected outright, and would be too large even if it weren't. Compression is doing essential work here; the download has to reach the user still compressed.The fix
There is no server-side way to decline the unarchiving — it keys on the extension, not on the content type or
Content-Disposition. Serving asapplication/octet-streamwithnosniffwas tried first and was not enough on its own (verified against a real browser, not just in tests). Both are kept as defence in depth, and they bring this route in line with the single-snapshot download, which already servedoctet-stream.So the archive is named
predbat_debug_history.tgz.dmp. The unarchiving leaves.dmpalone, GitHub accepts it, and the body is an ordinary gzip tarball thattar xzfreads whatever it is called — nothing needs renaming to open it.The Debug panel link stays a plain Download all: the file is meant to be opaque and handed straight to an issue. The details are documented in
customisation.mdandweb-interface.mdfor the few who need to open one themselves.Testing
Verified on a live deployment: the download now lands as a 5.4 MB
.dmp, unpacked withtar xzf, no.tarproduced.Unit tests assert the content type, the
nosniffheader, the.tgz.dmpfilename, and — separately — that the body still starts with the gzip magic number, so serving it as opaque binary cannot quietly become serving something that is not an archive.Full quick suite and pre-commit green.
🤖 Generated with Claude Code