Bug #7924

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

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

[ruby-dev:47075]
Status:Closed
Priority:Normal
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 in responses like 1.x Closed 02/12/2013

Associated revisions

Revision 39463
Added by Eric Hodel over 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 over 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 over 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 over 2 years ago

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

#3 Updated by Eric Hodel over 2 years ago

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

#4 Updated by Hiroshi SHIBATA over 2 years ago

  • Assignee changed from Yui NARUSE to Eric Hodel

#5 Updated by Eric Hodel over 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 over 2 years ago

  • Assignee changed from Eric Hodel to Yui NARUSE

assigned to Naruse-san for approval

#7 Updated by Hiroshi SHIBATA over 2 years ago

this patch seems good.

#8 Updated by Tomoyuki Chikanaga over 2 years ago

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

#9 Updated by Motohiro KOSAKI over 2 years ago

Please please make a test. for preventing regression.

#10 Updated by Eric Hodel over 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 over 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 over 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 over 2 years ago

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

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 over 2 years ago

Also, r39464 for ruby_2_0_0 branch.

#15 Updated by Yui NARUSE over 2 years ago

I see.
Thank you all and Happy Ruby 2.0!

Also available in: Atom PDF