Bug #7924

r39232 以降 net/http で正しく reponse を取得出来ないケースがある

Added by Hiroshi SHIBATA about 2 years ago. Updated about 2 years ago.

[ruby-dev:47075]
Status:Closed
Priority:Immediate
Assignee:Eric Hodel
ruby -v:ruby 2.0.0dev (2013-02-24 trunk 39439) [x86_64-darwin12.2.1] Backport:

Description

r39232 以降、tDiary の以下のようなコードが動かなくなりました。

https://github.com/tdiary/tdiary-contrib/blob/master/plugin/flickr.rb#L185

単純に net/http を使用して flickr.com から xml を取得するコードですが、r39232で加えた
変更により、本来 inflate されるべき response.body が gzip の状態のままになっています。

response を inflate するケースの考慮漏れのような気がします。

net.http.bug7924.patch Magnifier (1.07 KB) Eric Hodel, 02/24/2013 08:05 AM


Related issues

Related to Backport200 - Backport #7831: Net::HTTP does not allow users to handle Content-Encoding... Closed 02/12/2013

Associated revisions

Revision 39463
Added by Eric Hodel about 2 years ago

  • lib/net/http.rb: Removed duplicate Accept-Encoding in Net::HTTP#get. [ruby-trunk - Bug #7924]
  • test/net/http/test_http.rb: Test for the above.

Revision 39463
Added by Eric Hodel about 2 years ago

  • lib/net/http.rb: Removed duplicate Accept-Encoding in Net::HTTP#get. [ruby-trunk - Bug #7924]
  • test/net/http/test_http.rb: Test for the above.

History

#1 Updated by Kazuhiko Shiozaki about 2 years ago

accept-encodingの無指定時に、http.rbのget()でaccept-encodingをつけていて、そのためにgeneric_request.rbが「あ、指定してるんならdecode_content=falseでいいよね」としているのが原因。

accept-encodingの無指定時に"gzip;q=1.0,deflate;q=0.6,identity;q=0.3"をセットするコードが、http.rbのget()とhttp/generic_request.rbのinitialize()の両方にあって、後者は前者の可能性を考慮していない。なので、この問題はputでは起きない。

その二箇所以外にaccept-encodingをセットしているコードはなさそうなので、http.rbのget()から「accept-encodingの無指定時に〜」を除けばなおるはず。

#2 Updated by Kazuhiko Shiozaki about 2 years ago

問題になった r32932 ruby_2_0_0 にすでにマージされているので、このまま2.0.0を出すとけっこう影響の大きいバグだと思います。

#3 Updated by Eric Hodel about 2 years ago

I found the bug, I will post a patch with a test momentarily.

#4 Updated by Hiroshi SHIBATA about 2 years ago

  • Assignee changed from Yui NARUSE to Eric Hodel

#5 Updated by Eric Hodel about 2 years ago

The attached patch removes the duplicated header setting in Net::HTTP#get and adds a test.

I double checked net/http* for use of accept-encoding or HAVE_ZLIB, now it only exists in Net::HTTPGenericRequest and Net::HTTPResponse.

#6 Updated by Eric Hodel about 2 years ago

  • Assignee changed from Eric Hodel to Yui NARUSE

assigned to Naruse-san for approval

#7 Updated by Hiroshi SHIBATA about 2 years ago

this patch seems good.

#8 Updated by Tomoyuki Chikanaga about 2 years ago

make check all green with the patch in my environment [x86_64-darwin12.2.0].

#9 Updated by Motohiro KOSAKI about 2 years ago

Please please make a test. for preventing regression.

#10 Updated by Eric Hodel about 2 years ago

Kosaki-san, the patch contains a test to ensure that decode_content is enabled when using Net::HTTP#get. The behavior of decode_content is already tested in r39232.

Is it sufficient?

#11 Updated by Motohiro KOSAKI about 2 years ago

Kosaki-san, the patch contains a test to ensure that decode_content is enabled when using Net::HTTP#get. The behavior of decode_content is already tested in r39232.

Is it sufficient?

Oops, I missed that. sorry for noise.

#12 Updated by Yusuke Endoh about 2 years ago

  • Status changed from Open to Assigned
  • Assignee changed from Yui NARUSE to Eric Hodel

Looks serious. Got ack from hsbt and nagachika.
Drbrain, could you please commit it to trunk and ruby_2_0_0 in advance?
I'll ask naruse-san to do post-review, if he could wake up early enough ;-)

Yusuke Endoh mame@tsg.ne.jp

#13 Updated by Eric Hodel about 2 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r39463.
Hiroshi, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • lib/net/http.rb: Removed duplicate Accept-Encoding in Net::HTTP#get. [ruby-trunk - Bug #7924]
  • test/net/http/test_http.rb: Test for the above.

#14 Updated by Eric Hodel about 2 years ago

Also, r39464 for ruby_2_0_0 branch.

#15 Updated by Yui NARUSE about 2 years ago

I see.
Thank you all and Happy Ruby 2.0!

Also available in: Atom PDF