Closed
Conversation
simbabque
requested changes
Oct 8, 2022
Contributor
simbabque
left a comment
simbabque
left a comment
There was a problem hiding this comment.
This is very impressive work. Thank you for that. Could you please remove those leftover commented out debug messages?
I think we should also check why the Brotli test skips in CI.
Corion
pushed a commit
to Corion/HTTP-Message
that referenced
this pull request
Oct 8, 2022
Corion
pushed a commit
to Corion/HTTP-Message
that referenced
this pull request
Oct 8, 2022
Corion
force-pushed
the
decompress-limits
branch
from
debddbd to
caf0b64
Compare
Corion
pushed a commit
to Corion/HTTP-Message
that referenced
this pull request
Oct 8, 2022
Corion
force-pushed
the
decompress-limits
branch
from
caf0b64 to
01a181e
Compare
Author
|
The Brotli test skips in CI because |
Member
That also skips in |
oalders
requested changes
Oct 9, 2022
Corion
pushed a commit
to Corion/HTTP-Message
that referenced
this pull request
Oct 9, 2022
Member
|
I will probably fix the merge conflict and look at merging this tomorrow, so if anyone has objections, now would be a good time to bring them up. |
added 13 commits
October 11, 2022 19:06
This creates one (more) copy of the content if we limit the output because Zlib and Bzip2 want to remove the consumed input from the input string. Also, this moves away from IO::Uncompress::Gunzip and IO::Uncompress::Bzip2 in favour of Compress::Raw::Zlib and Compress::Raw::Bzip2 because I found no way to convince IO::Uncompress::Gunzip::gunzip to pass through the appropriate limiting options. The API is extended (but not yet documented) in three ways: 1) A global variable, $HTTP::Message::MAX_BODY_SIZE to limit the maximum size of ->decoded_content 2) An accessor, ->max_body_size, which can be set for individual HTTP::Responses 3) An optional parameter to ->decoded_content, which certainly is the most preferrable option but requires cooperation from all locations where ->decoded_content is called.
* Eliminate use of wantarray() in ->max_body_size * Eliminate use of vars.pm
…Zlib The Bufsize parameter was introduced in 2.060, so using 2.061 should be fairly safe here
... mostly to pacify the gods of CI # Conflicts: # Changes
Corion
force-pushed
the
decompress-limits
branch
from
9039091 to
4fda7fc
Compare
Member
|
Thanks so much for revisiting this, @Corion! I've rebased and merged at the command line. New release will be on CPAN shortly. |
oalders
pushed a commit
that referenced
this pull request
Oct 12, 2022
Limit decoded body size by manually decoding the compressed content This creates one (more) copy of the content if we limit the output because Zlib and Bzip2 want to remove the consumed input from the input string. Also, this moves away from IO::Uncompress::Gunzip and IO::Uncompress::Bzip2 in favour of Compress::Raw::Zlib and Compress::Raw::Bzip2 because I found no way to convince IO::Uncompress::Gunzip::gunzip to pass through the appropriate limiting options. The API is extended (but not yet documented) in three ways: 1) A global variable, $HTTP::Message::MAX_BODY_SIZE to limit the maximum size of ->decoded_content 2) An accessor, ->max_body_size, which can be set for individual HTTP::Responses 3) An optional parameter to ->decoded_content, which certainly is the most preferrable option but requires cooperation from all locations where ->decoded_content is called. Output the Compress::Raw::Zlib version in case a test fails We might be fine with version 2.061... Up our prerequisite to 2.061 for the time being... Update META.json as well... Amend changes * Eliminate use of wantarray() in ->max_body_size * Eliminate use of vars.pm Also handle Brotli (de)compression Reindent to match source Only run zipbomb tests if we have a recent version of Compress::Raw::Zlib The Bufsize parameter was introduced in 2.060, so using 2.061 should be fairly safe here Remove debugging comments, remove misleading comments In #181 , #181 (review) Add Changes blurb ... mostly to pacify the gods of CI
oalders
pushed a commit
that referenced
this pull request
Oct 12, 2022
oalders
pushed a commit
that referenced
this pull request
Oct 18, 2022
Limit decoded body size by manually decoding the compressed content This creates one (more) copy of the content if we limit the output because Zlib and Bzip2 want to remove the consumed input from the input string. Also, this moves away from IO::Uncompress::Gunzip and IO::Uncompress::Bzip2 in favour of Compress::Raw::Zlib and Compress::Raw::Bzip2 because I found no way to convince IO::Uncompress::Gunzip::gunzip to pass through the appropriate limiting options. The API is extended (but not yet documented) in three ways: 1) A global variable, $HTTP::Message::MAX_BODY_SIZE to limit the maximum size of ->decoded_content 2) An accessor, ->max_body_size, which can be set for individual HTTP::Responses 3) An optional parameter to ->decoded_content, which certainly is the most preferrable option but requires cooperation from all locations where ->decoded_content is called. Output the Compress::Raw::Zlib version in case a test fails We might be fine with version 2.061... Up our prerequisite to 2.061 for the time being... Update META.json as well... Amend changes * Eliminate use of wantarray() in ->max_body_size * Eliminate use of vars.pm Also handle Brotli (de)compression Reindent to match source Only run zipbomb tests if we have a recent version of Compress::Raw::Zlib The Bufsize parameter was introduced in 2.060, so using 2.061 should be fairly safe here Remove debugging comments, remove misleading comments In #181 , #181 (review) Add Changes blurb ... mostly to pacify the gods of CI
oalders
pushed a commit
that referenced
this pull request
Oct 18, 2022
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
3 participants
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.
This supercedes #42