Project

General

Profile

Bug #7924

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

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

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

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 about 3 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 3 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 3 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 [ruby-dev:47076] Updated by Kazuhiko Shiozaki about 3 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 [ruby-dev:47077] Updated by Kazuhiko Shiozaki about 3 years ago

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

#3 [ruby-dev:47078] Updated by Eric Hodel about 3 years ago

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

#4 [ruby-dev:47079] Updated by Hiroshi SHIBATA about 3 years ago

  • Assignee changed from Yui NARUSE to Eric Hodel

#5 [ruby-dev:47080] Updated by Eric Hodel about 3 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 [ruby-dev:47081] Updated by Eric Hodel about 3 years ago

  • Assignee changed from Eric Hodel to Yui NARUSE

assigned to Naruse-san for approval

#7 [ruby-dev:47083] Updated by Hiroshi SHIBATA about 3 years ago

this patch seems good.

#8 [ruby-dev:47084] Updated by Tomoyuki Chikanaga about 3 years ago

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

#9 [ruby-dev:47085] Updated by Motohiro KOSAKI about 3 years ago

Please please make a test. for preventing regression.

#10 [ruby-dev:47086] Updated by Eric Hodel about 3 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 [ruby-dev:47087] Updated by Motohiro KOSAKI about 3 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 [ruby-dev:47089] Updated by Yusuke Endoh about 3 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 3 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 [ruby-dev:47090] Updated by Eric Hodel about 3 years ago

Also, r39464 for ruby_2_0_0 branch.

#15 [ruby-dev:47095] Updated by Yui NARUSE about 3 years ago

I see.
Thank you all and Happy Ruby 2.0!

Also available in: Atom PDF