Fix GzipWriter output #3421

Merged
merged 2 commits into from Nov 3, 2015

Conversation

Projects
None yet
2 participants
@dmarcotte
Contributor

dmarcotte commented Oct 23, 2015

GZIPOutputStream was passing one of its internal byte buffers to IOOutputStream#write, which in turn used those bytes to back the ByteList for its output. So every subsequent time GZIPOutputStream wrote to that buffer, it overwrote the bytes behind the back of all the previous chunks of output.

Ensure that IOOutputStream is working on a copy of the bytes it gets from GZIPOutputStream to fix this issue.

Fixes #1371

Let me know if there's any questions, and thanks again!

dmarcotte added some commits Oct 23, 2015

Fix GzipWriter output
GZIPOutputStream was passing one of its internal byte buffers to
IOOutputStream#write, which in turn used those bytes to back the
ByteList for its output.  So every subsequent time GZIPOutputStream
wrote to that buffer, it overwrote the bytes behind the back of all
the previous chunks of output.

Ensure that IOOutputStream is working on a copy of the bytes
it gets from GZIPOutputStream to fix this issue.
Fix typo [ci skip]
Typo'ed variable reference should have caused problems, but got lucky
and the only place calling this `write` passes 0 for `off` so everything
just happens to always work in spite of looking so, so wrong.
@dmarcotte

This comment has been minimized.

Show comment
Hide comment
@dmarcotte

dmarcotte Oct 23, 2015

Contributor

Just pushed a small extra change on this pull: b2dc3da. Apologies for the noise.

Contributor

dmarcotte commented Oct 23, 2015

Just pushed a small extra change on this pull: b2dc3da. Apologies for the noise.

@dmarcotte

This comment has been minimized.

Show comment
Hide comment
@dmarcotte

dmarcotte Oct 26, 2015

Contributor

Just saw Travis is showing red on this pull because the 1_7 branch went red while I was working on this. See here for a green run of these changes: https://travis-ci.org/dmarcotte/jruby/builds/87085859

Contributor

dmarcotte commented Oct 26, 2015

Just saw Travis is showing red on this pull because the 1_7 branch went red while I was working on this. See here for a green run of these changes: https://travis-ci.org/dmarcotte/jruby/builds/87085859

@kares kares added this to the JRuby 1.7.23 milestone Nov 3, 2015

kares added a commit that referenced this pull request Nov 3, 2015

@kares kares merged commit a7f8715 into jruby:jruby-1_7 Nov 3, 2015

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Nov 3, 2015

Member

☝️ very najs find - much appreciated!

Member

kares commented Nov 3, 2015

☝️ very najs find - much appreciated!

@dmarcotte

This comment has been minimized.

Show comment
Hide comment
@dmarcotte

dmarcotte Nov 6, 2015

Contributor

Thanks @kares!

Contributor

dmarcotte commented Nov 6, 2015

Thanks @kares!

@dmarcotte dmarcotte deleted the dmarcotte:test-gzip-fix branch Nov 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment