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) about 9 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) about 9 years ago
問題になった r32932 ruby_2_0_0 にすでにマージされているので、このまま2.0.0を出すとけっこう影響の大きいバグだと思います。
Updated by drbrain (Eric Hodel) about 9 years ago
I found the bug, I will post a patch with a test momentarily.
Updated by hsbt (Hiroshi SHIBATA) about 9 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) about 9 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) about 9 years ago
- Assignee changed from drbrain (Eric Hodel) to naruse (Yui NARUSE)
assigned to Naruse-san for approval
Updated by hsbt (Hiroshi SHIBATA) about 9 years ago
this patch seems good.
Updated by nagachika (Tomoyuki Chikanaga) about 9 years ago
make check all green with the patch in my environment [x86_64-darwin12.2.0].
Updated by kosaki (Motohiro KOSAKI) about 9 years ago
Please please make a test. for preventing regression.
Updated by drbrain (Eric Hodel) about 9 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) about 9 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) about 9 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) about 9 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) about 9 years ago
Also, r39464 for ruby_2_0_0 branch.
Updated by naruse (Yui NARUSE) about 9 years ago
I see.
Thank you all and Happy Ruby 2.0!