Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support gzip with encoding on node pre-v0.9.4 #960

Merged
merged 1 commit into from Jul 9, 2014

Conversation

Projects
None yet
3 participants
@kevinoid
Copy link
Contributor

commented Jul 8, 2014

This pull request addresses an oversight from #951 that before 0.9.4 zlib streams did not support the setEncoding method. This means that on these versions of node, using the encoding and gzip options together would cause an error and result in test failures on these versions.

This pull request addresses the issue by chaining the zlib stream with a stringstream when both the encoding and gzip options are specified and setEncoding is not available on the zlib stream.

@LoicMahieu

This comment has been minimized.

Copy link
Member

commented Jul 8, 2014

I don't understand why travis doesn't run. Maybe rebase and push --force.

@kevinoid

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2014

Interestingly, it ran on the branch in my repository (https://travis-ci.org/kevinoid/request/builds/29416222), just not on the PR in this repository. But I'll rebase it and see if that'll kick it off.

Support gzip with encoding on node pre-v0.9.4
setEncoding was added to zlib streams in nodejs/node-v0.x-archive@9b5abe5 (v0.9.4 and
later).  In order to support encoding and gzip together on earlier
releases, pass the data through an additional Stream which decodes the
data using the stringstream module.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
@kevinoid

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2014

Ah. It needed a rebase anyway (since gzip was disabled for pre-v0.9.4 by commit c1d951e subsequent to when the previous pull request was merged). Looks like it's working now.

mikeal added a commit that referenced this pull request Jul 9, 2014

Merge pull request #960 from kevinoid/gzip-encoding-pre-v0.9.4
Support gzip with encoding on node pre-v0.9.4

@mikeal mikeal merged commit 604d157 into request:master Jul 9, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

nylen pushed a commit to nylen/request that referenced this pull request Oct 17, 2014

Merge pull request request#960 from kevinoid/gzip-encoding-pre-v0.9.4
Support gzip with encoding on node pre-v0.9.4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.