Bug #7924
closedr39232 以降 net/http で正しく reponse を取得出来ないケースがある
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 するケースの考慮漏れのような気がします。
Files
Updated by kazuhiko (Kazuhiko Shiozaki) over 11 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の無指定時に〜」を除けばなおるはず。
Updated by kazuhiko (Kazuhiko Shiozaki) over 11 years ago
問題になった r32932 ruby_2_0_0 にすでにマージされているので、このまま2.0.0を出すとけっこう影響の大きいバグだと思います。
Updated by drbrain (Eric Hodel) over 11 years ago
I found the bug, I will post a patch with a test momentarily.
Updated by hsbt (Hiroshi SHIBATA) over 11 years ago
- Assignee changed from naruse (Yui NARUSE) to drbrain (Eric Hodel)
translation of this issue: http://bugs.ruby-lang.org/issues/7831#note-10
Updated by drbrain (Eric Hodel) over 11 years ago
- File net.http.bug7924.patch net.http.bug7924.patch added
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.
Updated by drbrain (Eric Hodel) over 11 years ago
- Assignee changed from drbrain (Eric Hodel) to naruse (Yui NARUSE)
assigned to Naruse-san for approval
Updated by hsbt (Hiroshi SHIBATA) over 11 years ago
this patch seems good.
Updated by nagachika (Tomoyuki Chikanaga) over 11 years ago
make check all green with the patch in my environment [x86_64-darwin12.2.0].
Updated by kosaki (Motohiro KOSAKI) over 11 years ago
Please please make a test. for preventing regression.
Updated by drbrain (Eric Hodel) over 11 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?
Updated by kosaki (Motohiro KOSAKI) over 11 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.
Updated by mame (Yusuke Endoh) over 11 years ago
- Status changed from Open to Assigned
- Assignee changed from naruse (Yui NARUSE) to drbrain (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
Updated by drbrain (Eric Hodel) over 11 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.
Updated by drbrain (Eric Hodel) over 11 years ago
Also, r39464 for ruby_2_0_0 branch.
Updated by naruse (Yui NARUSE) over 11 years ago
I see.
Thank you all and Happy Ruby 2.0!