Fix gzip with chunked - write full response to a buffer before decompressing it #163

Closed
wants to merge 4 commits into
from

Projects

None yet

6 participants

@glebtv
glebtv commented May 6, 2013

Fixes #162

@buildhive

Hiroshi Nakamura » httpclient #72 SUCCESS
This pull request looks good
(what's this?)

@buildhive

Hiroshi Nakamura » httpclient #73 SUCCESS
This pull request looks good
(what's this?)

@webos-goodies

I believe we must keep last two bytes of each block. Trailing CRLFs are already truncated by read_body_chunked.

Additionally, "if @gzipped" should be "if @gzipped and @transparent_gzip_decompression" for transparent_gzip_decompression=false case.

@buildhive

Hiroshi Nakamura » httpclient #74 SUCCESS
This pull request looks good
(what's this?)

@glebtv
glebtv commented May 16, 2013

Yes, i forgot @transparent_gzip_decompression
As for last two bytes - it didn't work properly for me without this, there were CRLF.

@webos-goodies

These urls can be loaded properly?

http://theappleblog.com/feed/
http://googlejapan.blogspot.com/atom.xml
http://googleplusplatform.blogspot.com/feeds/posts/default

I got this error while loading them.

incorrect length check
/lib/ruby/gems/1.9.1/bundler/gems/httpclient-cf5379be35ca/lib/httpclient/session.rb:712:in `inflate'
/lib/ruby/gems/1.9.1/bundler/gems/httpclient-cf5379be35ca/lib/httpclient/session.rb:712:in `get_body'
/lib/ruby/gems/1.9.1/bundler/gems/httpclient-cf5379be35ca/lib/httpclient.rb:1088:in `do_get_block'
/lib/ruby/gems/1.9.1/bundler/gems/httpclient-cf5379be35ca/lib/httpclient.rb:887:in `block in do_request'

I applied your patch (a194260) to nahi's master branch and used HTTPClient#get with :follow_redirect => false.

@glebtv
glebtv commented May 16, 2013

First one doesn't work, two other work for me

* Update:* sorry, used wrong version - All 3 work for me with 3334637

2.0.0p0 :002 > t = HTTPClient.new
2.0.0p0 :003 > t.transparent_gzip_decompression = true
2.0.0p0 :004 > t.transparent_gzip_decompression = true
 => true 
2.0.0p0 :005 > t.get_content('http://theappleblog.com/feed/').length
could be a relative URI in location header which is not recommended
'The field value consists of a single absolute URI' in HTTP spec
 => 60193 
2.0.0p0 :006 > t.get_content('http://gigaom.com/channel/apple/feed/').length
 => 60193 
2.0.0p0 :007 > t.get_content('http://googlejapan.blogspot.com/atom.xml').length
 => 128935 
2.0.0p0 :008 > t.get_content('http://googleplusplatform.blogspot.com/feeds/posts/default').length
@buildhive

Hiroshi Nakamura » httpclient #75 SUCCESS
This pull request looks good
(what's this?)

@bmaland
bmaland commented Jun 9, 2013

This patch fixes my issue with incorrect data check (Zlib::DataError). So far I haven't had any problems using it against our internal API

@funglaub

This fixes the issue for me, too. Any updates on this pull request?

@nahi
Owner
nahi commented May 25, 2014

Sorry for long silence. Can someone try HEAD or 2.3.4.1 for the given urls? All seems to work for me with ruby-HEAD, ruby 2.1 and ruby 2.0.

I tried the client at #162 against http://www.yandex.ru but it works for me. http://theappleblog.com/feed/ works, too. I'm suspecting another patch https://github.com/nahi/httpclient/pull/167/files fixed this issue, too.

@funglaub

@nahi 2.3.4.1 seems to work just fine. Thanks for the feedback!

@nahi
Owner
nahi commented Jun 7, 2014

@funglaub Thank you, too!

@nahi nahi closed this Jun 7, 2014
@glebtv glebtv deleted the glebtv:gzip-with-chunked branch Aug 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment