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

• Status: Closed
• Priority: Normal
• Assignee: drbrain (Eric Hodel)
• Target version: 2.0.0
• ruby -v: ruby 2.0.0dev (2013-02-24 trunk 39439) [x86_64-darwin12.2.1]

[hsbt (Hiroshi SHIBATA)]

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 するケースの考慮漏れのような気がします。

[kazuhiko (Kazuhiko Shiozaki)]

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の無指定時に〜」を除けばなおるはず。

[kazuhiko (Kazuhiko Shiozaki)]

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

[drbrain (Eric Hodel)]

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

[hsbt (Hiroshi SHIBATA)]

• Assignee changed from naruse (Yui NARUSE) to drbrain (Eric Hodel)

translation of this issue: http://bugs.ruby-lang.org/issues/7831#note-10

[drbrain (Eric Hodel)]

• 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.

[drbrain (Eric Hodel)]

• Assignee changed from drbrain (Eric Hodel) to naruse (Yui NARUSE)

assigned to Naruse-san for approval

[hsbt (Hiroshi SHIBATA)]

this patch seems good.

[nagachika (Tomoyuki Chikanaga)]

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

[kosaki (Motohiro KOSAKI)]

Please please make a test. for preventing regression.

[drbrain (Eric Hodel)]

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?

[kosaki (Motohiro KOSAKI)]

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.

[mame (Yusuke Endoh)]

• 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 [e-mail]

[drbrain (Eric Hodel)]

• 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.

[drbrain (Eric Hodel)]

Also, r39464 for ruby_2_0_0 branch.

[naruse (Yui NARUSE)]

I see.
Thank you all and Happy Ruby 2.0!