2010年12月28日火曜日

rails2.3.5のマルチパート解析ではまった

概要
rails2.3.5、正確にはrack1.0.1のマルチパート解析処理に問題があり、データの欠落する可能性があります。

問題
「チェックボックスがおかしな状態になる」という問い合わせがあり調査開始しました。ほどなく"ごくまれ"にチェックボックスの入力状態が正しく反映されないケースがある事に気付きました。

切り分けの為、問題箇所を確認した所
  • パケットダンパーでデータを確認すると、socket通信上は想定値が確認できた。
  • 例外メール上の記録から、railsでのparamsには、おかしな値が入っている。
  • 上記メールから、rackより引き継いだ段階でも、おかしな値が入っている。
という事が判明しました。これで犯人は、apache か passenger か rack に絞られました。

そこで、Rack::Utils::Multipart.parse_multipartメソッドにログを仕込んで確認した所
  • content_lengthは想定どおりの値が確認できた。
  • io上入ってきた内容も想定どおり確認できた。
  • 処理ループが想定よりもかなり早い段階で抜けている。但し例外は発生していない。
と言う事がわかりました。
上手くいかない場合は、変数の動きをみていると、buf変数に一度読み込まれるだけで、後続のデータが処理されていません。

解決
buf変数に読み込んだデータを逐次処理していきますが、バウンダリ+EOL(\r\n)で丁度処理が終了した場合、buf.empty?が真となり、bufに読み込まれていないデータがあっても処理終了になっているようです。よって上記をrack1.1.xで修正されているように

break if (buf.empty? && $1 != EOL) || content_length == -1

とする必要があるようです($1 != EOLを追加)。
しかしrack1.1とrails2.3.5は相性が悪いみたいなので、モンキーパッチで対処する必要がありますね。

下記のような内容をconfig/initializers/rack_patch.rbとかで配置して下さい。

module Rack
  module Utils
    module Multipart
      def self.parse_multipart(env)
        unless env['CONTENT_TYPE'] =~
            %r|\Amultipart/.*boundary=\"?([^\";,]+)\"?|n
          nil
        else
          boundary = "--#{$1}"

          params = {}
          buf = ""
          content_length = env['CONTENT_LENGTH'].to_i
          input = env['rack.input']
          input.rewind

          boundary_size = Utils.bytesize(boundary) + EOL.size
          bufsize = 16384

          content_length -= boundary_size

          read_buffer = ''

          status = input.read(boundary_size, read_buffer)
          raise EOFError, "bad content body"  unless status == boundary + EOL

          rx = /(?:#{EOL})?#{Regexp.quote boundary}(#{EOL}|--)/n

          loop {
            head = nil
            body = ''
            filename = content_type = name = nil

            until head && buf =~ rx
              if !head && i = buf.index(EOL+EOL)
                head = buf.slice!(0, i+2) # First \r\n
                buf.slice!(0, 2)          # Second \r\n

                filename = head[/Content-Disposition:.* filename=(?:"((?:\\.|[^\"])*)"|([^;\s]*))/ni, 1]
                content_type = head[/Content-Type: (.*)#{EOL}/ni, 1]
                name = head[/Content-Disposition:.*\s+name="?([^\";]*)"?/ni, 1] || head[/Content-ID:\s*([^#{EOL}]*)/ni, 1]

                if content_type || filename
                  body = Tempfile.new("RackMultipart")
                  body.binmode  if body.respond_to?(:binmode)
                end

                next
              end

              # Save the read body part.
              if head && (boundary_size+4 < buf.size)
                body << buf.slice!(0, buf.size - (boundary_size+4))
              end

              c = input.read(bufsize < content_length ? bufsize : content_length, read_buffer)
              raise EOFError, "bad content body"  if c.nil? || c.empty?
              buf << c
              content_length -= c.size
            end

            # Save the rest.
            if i = buf.index(rx)
              body << buf.slice!(0, i)
              buf.slice!(0, boundary_size+2)

              content_length = -1  if $1 == "--"
            end

            if filename == ""
              # filename is blank which means no file has been selected
              data = nil
            elsif filename
              body.rewind

              # Take the basename of the upload's original filename.
              # This handles the full Windows paths given by Internet Explorer
              # (and perhaps other broken user agents) without affecting
              # those which give the lone filename.
              filename =~ /^(?:.*[:\\\/])?(.*)/m
              filename = $1

              data = {:filename => filename, :type => content_type,
                      :name => name, :tempfile => body, :head => head}
            elsif !filename && content_type
              body.rewind
              
              # Generic multipart cases, not coming from a form
              data = {:type => content_type,
                      :name => name, :tempfile => body, :head => head}
            else
              data = body
            end

            Utils.normalize_params(params, name, data) unless data.nil?

            # break  if buf.empty? || content_length == -1
            break  if (buf.empty? && $1 != EOL) || content_length == -1
          }

          input.rewind

          params
        end
      end
    end
  end
end

0 件のコメント:

コメントを投稿