fix(download): allow highly compressed responses up to a known file size #9146
Conversation
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
Qt 6.2+'s archive bomb check is cool and all, but it's really annoying when trying to retrieve files that compress really well over the wire. As we already know the expected file size before the request we can basically tell Qt to allow decompression of responses up to the expected file size and a bit to spare. Signed-off-by: Jyrki Gadinger <[email protected]>
nilsding
requested review from
Aiiaiiio,
camilasan,
i2h3 and
mgallien
as code owners
Member
Author
|
/backport to stable-4.0 |
mgallien
requested changes
Nov 27, 2025
Collaborator
mgallien
left a comment
mgallien
left a comment
There was a problem hiding this comment.
really nice automated test
easy to understand how it works
Unfortunately faking this with the usual methods won't do the trick: setting an override in our FakeQNAM returning a faked reply completely bypasses Qt's internal routines for QNetworkReply. Therefore: spin up a temporary local HTTP server for the time of the test and do real HTTP requests. We could try to explore how useful QHttpServer would be in other tests in the future as well, it might enable us for a cleaner way of setting up a fake/mock server for unit tests. For now this specific test can be seen as the initial exploration on how it could be used. Caveat: this test is only available if the QHttpServer component is present, chances are it may not run everywhere. Signed-off-by: Jyrki Gadinger <[email protected]>
nilsding
force-pushed
the
bugfix/noid/decompression-threshold
branch
from
1be1bf5 to
14b0258
Compare
mgallien
approved these changes
Nov 27, 2025
|
Artifact containing the AppImage: nextcloud-appimage-pr-9146.zip Digest: To test this change/fix you can download the above artifact file, unzip it, and run it. Please make sure to quit your existing Nextcloud app and backup your data. |
|
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.
Follow-up for #8333
We already know the expected file size before the request, so we pretty much just™ can tell Qt to allow the decompression of responses up to the expected file size (and a bit to spare).
To cover this behaviour through unit tests I unfortunately ran into a limitation when trying to fake the requests with the usual methods: setting an override in our FakeQNAM returning a faked reply completely bypasses Qt's internal routines for QNetworkReply.
Therefore: spin up a temporary local HTTP server for the time of the test and do real HTTP requests.
We could try to explore how useful QHttpServer would be in other tests in the future as well, it might enable us for a cleaner way of setting up a fake/mock server for unit tests. For now this specific test can be seen as the initial exploration on how it could be used.
Caveat: this test is only available if the QHttpServer component is present, chances are it may not run everywhere. Hope CI is still fine with that...
Tested locally on Linux + Windows (both virtual files and classic sync) as well. To run into this issue I did the following:
.htaccessof my AIO installation; even with Brotli the compression ratio is large enough to run into thisSetOutputFilter BROTLI_COMPRESSbefore this change: the client failed to download the file with the well-known
Decompression failed: The decompressed output exceeds the limits specified by QNetworkRequest::decompressedSafetyCheckThreshold()errorafter this change: the client downloaded the file just fine