Re: [Mongrel] Want feedback on Mongrel patch for handling partial PUT requests

[cremes . devlist]

On Jan 23, 2007, at 12:09 AM, Zed A. Shaw wrote:

On Mon, 22 Jan 2007 19:27:35 -0600
[EMAIL PROTECTED] wrote:

By default, Mongrel will delete the HTTP request body and short
circuit calling any handlers if a request is interrupted or
incomplete. Unfortunately, this breaks any attempt to correctly
handle a partial PUT. (BTW, PUT is *way* more efficient for uploads
compared to POST which requires a MIME parsing step.) So, about a
month ago I wrote up some patches to Mongrel 0.3.18 that allowed
Mongrel to properly handle partial PUT requests. I sent the patch &
tests to Zed for comment but never really heard back.

I'll take a look this weekend, but you really should have tests to  
go with this just to make sure I'm trying it the way you are trying  
it.

Here are the patches against Mongrel 1.0 plus a test file.
# Copyright (c) 2005 Zed A. Shaw 
# You can redistribute it and/or modify it under the same terms as Ruby.
#
# Additional work donated by contributors.  See http://mongrel.rubyforge.org/attributions.html 
# for more information.

require 'test/unit'
require 'net/http'
require 'mongrel'
require 'timeout'
require 'fileutils'

require File.dirname(__FILE__) + "/testhelp.rb"

class TestHandler < Mongrel::HttpHandler
  attr_reader :ran_test

  def process(request, response)
    @ran_test = true
    response.socket.write("HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\n\r\nhello!\n")
  end
end

class UploadHandler < Mongrel::HttpHandler
  attr_accessor :default_content_type
  attr_reader :path
  attr_accessor :file_size

  ONLY_HEAD_PUT="Only HEAD and PUT allowed.".freeze

  # You give it the path to the directory root
  def initialize(path)
    @path = File.expand_path(path)
    # this only gets sent in response to a HEAD request for which only
    # the Content-Length component really matters. Leave this as a nice
    # default
    @default_content_type = "text/plain; charset=ISO-8859-1".freeze
  end

  def accept_partial_requests?
    true
  end

  def partial_put?(params)
    # returns true if the request_method is PUT and the header
    # contains a Content-Range field
    params[Const::REQUEST_METHOD] == Const::PUT && params.has_key?('HTTP_CONTENT_RANGE')
  end

  # Checks if the given path can be served and returns the full path (or nil if not).
  def can_write(path_info)
    req_path = File.expand_path(File.join(@path,HttpRequest.unescape(path_info)), @path)
    req_dir = File.dirname(req_path)

    if req_dir.index(@path) == 0 and File.exist? req_dir
      # it's a directory and we can write to it
      return req_path if File.directory?(req_dir) && File.writable?(req_dir)
    else
      # does not exist or isn't in the right spot
      return nil
    end
  end


  # Sends the size of a file back to the requester
  def send_head(req_path, request, response)
    stat = File.stat(req_path)

    # first we setup the headers and status then we do a very fast send on the socket directly
    response.status = 200
    header = response.header

    # set the mime type from our map based on the ending
    header[Const::CONTENT_TYPE] = @default_content_type

    # send a status with only content length
    response.send_status(stat.size)
    response.send_header
    self.file_size = stat.size # used so the test can peek at our internal state
  end
  
  def receive_file(req_path, request, response)
#    STDERR.puts("#receive_file req_path = #{req_path}")
#    STDERR.puts("#receive_file @path = [EMAIL PROTECTED]")
    case request.body
    when Tempfile
      # do a cheap mv using the TempFile handed off in the request object
      FileUtils.mv request.body.path, req_path
    when String
      # otherwise do a more expensive read-then-write to transfer
      # the data from the request.body into the file
      File.open(req_path, "wb+") { |f| f.write(request.body) }
    when StringIO
      File.open(req_path, "wb+") { |f| f.write(request.body.read) }
    end
  end
  
  def receive_partial_file(req_path, request)
    # glue the two pieces back together
    bytes_written = nil
    File.open(req_path, "ab") do |f|
      case request.body
      when StringIO, Tempfile
        bytes_written = f.write(request.body.read)
      when String
        bytes_written = f.write(request.body)
      end
    end
    #    STDERR.puts("#receive_partial_file wrote #{bytes_written} bytes.")
  end

  def process(request, response)
    # Process the request to return a HEAD, receive a new PUT, or
    # receive a partial-body PUT
    req_method = request.params[Const::REQUEST_METHOD] || Const::HEAD
    req_path = can_write request.params[Const::PATH_INFO]

    begin
      if req_method == Const::HEAD
        send_head(req_path, request, response)
      elsif partial_put?(request.params)
        receive_partial_file(req_path, request)
        STDERR.puts "received partial file"
      elsif req_method == Const::PUT
        receive_file(req_path, request, response)
        response.start(200) {|head,out| out << "File created." } unless response.nil?
        STDERR.puts "received complete file"
      end
    rescue => details
      STDERR.puts "Error receiving file #{req_path}: #{details}"
    end
  end
end


class UploadWebServerTest < Test::Unit::TestCase

  def setup
    # make a directory in /tmp and put a large file into it for testing
    @src_path = '/tmp/mongrel_upload_test'
    src_file = 'source_file'
    FileUtils.mkdir_p(@src_path)
    @path_to_src_file = File.join(@src_path, src_file)
    File.open(@path_to_src_file, "w+") do |f|
      20.times {|a| f.write(IO.read(File.expand_path(__FILE__))) }
    end
    @path_to_new_file = File.join(@src_path, 'upload.tst')
    @src_file_size = File.size(@path_to_src_file)
    
    @request = "GET / HTTP/1.1\r\nHost: www.zedshaw.com\r\nContent-Type: text/plain\r\n\r\n"    

    @server = HttpServer.new("127.0.0.1", 9998,num_processors=1)

    @uploader = UploadHandler.new(@src_path)
    @server.register("/uploads", @uploader)
    @tester = TestHandler.new
    @server.register("/test", @tester)
    redirect_test_io do
      @server.run 
    end
  end

  def teardown
    @server.stop
    FileUtils.rm_r(@src_path)
  end

  def do_test(st, chunk, close_after=nil)
    s = TCPSocket.new("127.0.0.1", 9998);
    req = StringIO.new(st)
    nout = 0

    while data = req.read(chunk)
      nout += s.write(data)
      s.flush
      sleep 0.2
      if close_after and nout > close_after
        s.close_write
        sleep 1
      end
    end
    s.write(" ") if RUBY_PLATFORM =~ /mingw|mswin|cygwin/
    s.close
  end
  
  def test_complete_put
    request = "PUT /uploads/upload.tst HTTP/1.1\r\nHost: www.zedshaw.com\r\nContent-Length: [EMAIL PROTECTED]: 100-continue\r\n\r\n"
    attached_file = IO.read(@path_to_src_file)
    
    redirect_test_io do
      do_test(request + attached_file, @src_file_size/2)
    end
    
    assert File.exists?(@path_to_new_file)
  end
  
  def test_file_remains_after_partial_put
    request = "PUT /uploads/upload.tst HTTP/1.1\r\nHost: www.zedshaw.com\r\nContent-Length: [EMAIL PROTECTED]: 100-continue\r\n\r\n"
    attached_file = IO.read(@path_to_src_file)

    assert_raises IOError do
      redirect_test_io  do 
        do_test(request + attached_file, @src_file_size/2, (@src_file_size-1)/2)
      end
    end

    assert File.exists?(@path_to_new_file)    
  end
  
  def test_get_head_on_partial_put
    setup_partial_file
    response = get_head

    assert_equal(File.size(@path_to_new_file), response['content-length'].to_i)    
  end
  
  def test_resume_put
    setup_partial_file
    response = get_head
    offset = response['content-length'].to_i

    continue_at = "[EMAIL PROTECTED]/[EMAIL PROTECTED]"
    new_content_length = "[EMAIL PROTECTED] - offset}"
    new_request = "PUT /uploads/upload.tst HTTP/1.1\r\nHost: www.zedshaw.com\r\nContent-Range: #{continue_at}\r\nContent-Length: #{new_content_length}\r\nExpected: 100-continue\r\n\r\n"

    file_remainder = IO.read(@path_to_src_file, offset)
    redirect_test_io do
      do_test(new_request + file_remainder, @src_file_size/2)
    end

    assert_equal(@src_file_size, File.size(@path_to_new_file))
  end
  
  private
  
  def setup_partial_file
    File.open(@path_to_new_file, "w+") do |f|
      f.write(IO.read(@path_to_src_file, @src_file_size/2))
    end
  end

  def get_head
    response = nil
    Net::HTTP.start('127.0.0.1', 9998) do |http|
      response = http.head('/uploads/upload.tst')
    end
    response  
  end
end



handlers.diff
Description: Binary data


mongrel.diff
Description: Binary data

Also, seeing how it can improve or change the upload progress  
problem (the fact that it sucks) would be a good first step.

That problem was a bit out of scope for me so I don't think this  
helps much. The common assumption is that people are using POST for  
uploads which this patch doesn't address at all.

Otherwise, I'll look and get back to you.  Sorry I didn't get in  
touch with you earlier.  Just been busy.

No problem; we're all busy!

cr

_______________________________________________
Mongrel-users mailing list
Mongrel-users@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-users