Gzip compression/inflation handling - #338
Conversation
a3e7f04 to
45d6c31
Compare
gaobinlong
left a comment
gaobinlong
left a comment
There was a problem hiding this comment.
Thanks @spenserw , awesome work, left some comments, and change log is needed I think.
| end | ||
|
|
||
| def compress_request(body) | ||
| io = StringIO.new |
There was a problem hiding this comment.
StringIO.new creates a UTF-8 buffer, the GzipWriter is told to use ASCII-8BIT (binary) encoding, but the underlying StringIO buffer remains UTF-8, so when gzip writes binary bytes into it, io.string returns a UTF-8-tagged string containing binary data — which can blow up downstream when something tries to interpret or concatenate it as text. Consider using binary-mode buffer by io = StringIO.new(''.b) ?
There was a problem hiding this comment.
really nice catch, wouldn't want any nasty surprises in the future... updated
|
|
||
| def compress_request(body) | ||
| io = StringIO.new | ||
| gzip_writer = if RUBY_ENCODING |
There was a problem hiding this comment.
RUBY_ENCODING always be true since we are now using 3.x?
There was a problem hiding this comment.
I had assumed there was some legacy behavior being protected, but yes I see the 3.x requirement now that I look.. cleaned this up and also removed it from the inflation path, too
Signed-off-by: Spenser Williams <spenser@w2cs.com>
Signed-off-by: Spenser Williams <spenser@w2cs.com>
Signed-off-by: Spenser Williams <spenser@w2cs.com>
Signed-off-by: Spenser Williams <spenser@w2cs.com>
e4655e1 to
c8e6ea4
Compare
|
Updated changelog |
3 participants
Description
As described in the client initialization doc comments:
compressionflag is passed to the clientcompressionoptionIssues Resolved
Closes #337
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.