[ruby-dev:39284] Re: [Bug #2067] bodyが大きいエラーページをopen-uriで取得するとfdがリークしている

なかだです。

At Thu, 10 Sep 2009 13:17:04 +0900,
Yukihiro Matsumoto wrote in [ruby-dev:39283]:
> |begin
> |  open(url) do |f|
> |    ...
> |  end
> |rescue OpenURI::HTTPError => e
> |  e.io.close
> |end
> |とすれば良いのですね。
> 
> いや、そんなことしなければいけないのはやっぱりおかしいと思い
> ますよ。なんでかな。

エラーページの内容も例外の情報の一部になっているので、Tempfileを
使っている場合には当然そうなるでしょう。

エラーページの内容は捨ててしまうか、エラーのときにはサイズに関わ
らずStringIOにするか、Tempfileなので気にしないことにするか。


Index: lib/open-uri.rb
===================================================================
--- lib/open-uri.rb	(revision 24824)
+++ lib/open-uri.rb	(working copy)
@@ -211,5 +211,6 @@ module OpenURI
         end
         if !options.fetch(:redirect, true)
-          raise HTTPRedirect.new(buf.io.status.join(' '), buf.io, redirect)
+          buf.io.close
+          raise HTTPRedirect.new(buf.io.status.join(' '), redirect)
         end
         unless OpenURI.redirectable?(uri, redirect)
@@ -340,16 +341,18 @@ module OpenURI
         loc_uri = URI.parse(resp['location'])
       rescue URI::InvalidURIError
-        raise OpenURI::HTTPError.new(io.status.join(' ') + ' (Invalid Location URI)', io)
+        io.close
+        raise OpenURI::HTTPError.new(io.status.join(' ') + ' (Invalid Location URI)')
       end
       throw :open_uri_redirect, loc_uri
     else
-      raise OpenURI::HTTPError.new(io.status.join(' '), io)
+      io.close
+      raise OpenURI::HTTPError.new(io.status.join(' '))
     end
   end
 
   class HTTPError < StandardError
-    def initialize(message, io)
+    def initialize(message)
       super(message)
-      @io = io
+      @io = nil
     end
     attr_reader :io
@@ -357,6 +360,6 @@ module OpenURI
 
   class HTTPRedirect < HTTPError
-    def initialize(message, io, uri)
-      super(message, io)
+    def initialize(message, uri)
+      super(message)
       @uri = uri
     end

Index: lib/open-uri.rb
===================================================================
--- lib/open-uri.rb	(revision 24824)
+++ lib/open-uri.rb	(working copy)
@@ -150,5 +150,5 @@ module OpenURI
         yield io
       ensure
-        io.close
+        io.close unless io.closed?
       end
     else
@@ -200,5 +200,5 @@ module OpenURI
     while true
       redirect = catch(:open_uri_redirect) {
-        buf = Buffer.new
+        buf = Buffer.new(options)
         uri.buffer_open(buf, find_proxy.call(uri), options)
         nil
@@ -312,5 +312,8 @@ module OpenURI
       http.request(req) {|response|
         resp = response
-        if options[:content_length_proc] && Net::HTTPSuccess === resp
+        unless success = Net::HTTPSuccess === resp
+          buf.unlimit
+        end
+        if options[:content_length_proc] && success
           if resp.key?('Content-Length')
             options[:content_length_proc].call(resp['Content-Length'].to_i)
@@ -321,5 +324,5 @@ module OpenURI
         resp.read_body {|str|
           buf << str
-          if options[:progress_proc] && Net::HTTPSuccess === resp
+          if options[:progress_proc] && success
             options[:progress_proc].call(buf.size)
           end
@@ -365,7 +368,10 @@ module OpenURI
 
   class Buffer # :nodoc:
-    def initialize
+    def initialize(options)
       @io = StringIO.new
       @size = 0
+      if @stringmax = options.fetch(:max_stringio_size, StringMax)
+        raise ArgumentError, "negative max_stringio_size" if @stringmax < 0
+      end
     end
     attr_reader :size
@@ -375,5 +381,5 @@ module OpenURI
       @io << str
       @size += str.length
-      if StringIO === @io && StringMax < @size
+      if @stringmax && @stringmax < @size
         require 'tempfile'
         io = Tempfile.new('open-uri')
@@ -382,7 +388,12 @@ module OpenURI
         io << @io.string
         @io = io
+        @stringmax = nil
       end
     end
 
+    def unlimit
+      @stringmax = nil
+    end
+
     def io
       Meta.init @io unless Meta === @io


-- 
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦