Project

General

Profile

Actions

Bug #7924

closed

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

Added by hsbt (Hiroshi SHIBATA) about 11 years ago. Updated about 11 years ago.

Status:
Closed
Target version:
ruby -v:
ruby 2.0.0dev (2013-02-24 trunk 39439) [x86_64-darwin12.2.1]
Backport:
[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 するケースの考慮漏れのような気がします。


Files

net.http.bug7924.patch (1.07 KB) net.http.bug7924.patch drbrain (Eric Hodel), 02/24/2013 08:05 AM

Related issues 1 (0 open1 closed)

Related to Backport200 - Backport #7831: Net::HTTP does not allow users to handle Content-Encoding in responses like 1.xCloseddrbrain (Eric Hodel)02/12/2013Actions

Updated by kazuhiko (Kazuhiko Shiozaki) about 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) about 11 years ago

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

Updated by drbrain (Eric Hodel) about 11 years ago

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

Updated by hsbt (Hiroshi SHIBATA) about 11 years ago

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

Updated by drbrain (Eric Hodel) about 11 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.

Updated by drbrain (Eric Hodel) about 11 years ago

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

assigned to Naruse-san for approval

Updated by hsbt (Hiroshi SHIBATA) about 11 years ago

this patch seems good.

Updated by nagachika (Tomoyuki Chikanaga) about 11 years ago

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

Updated by kosaki (Motohiro KOSAKI) about 11 years ago

Please please make a test. for preventing regression.

Updated by drbrain (Eric Hodel) about 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) about 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) about 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

Actions #13

Updated by drbrain (Eric Hodel) about 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) about 11 years ago

Also, r39464 for ruby_2_0_0 branch.

Updated by naruse (Yui NARUSE) about 11 years ago

I see.
Thank you all and Happy Ruby 2.0!

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0