I would like to play with this a bit as it changes the way the
mongrel_upload_progress does a request_aborted to cleanup. That
plugin relies on the @body being nil to work but I think that could
be worked around in your approach.
Can you please send an svn diff of this file? I don't feel like copy
pasting all that jazz.
Thanks
-Ezra
On Jan 22, 2007, at 5:27 PM, [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.
>
> It occurred to me that there might be a better way to handle things,
> so here's my second stab at writing a patch that I hope will be
> accepted into the Mongrel trunk. This patch was written against
> Mongrel 1.0. The original patch detected if the request was a PUT and
> ran various special case code. This approach makes handling partial
> or interrupted requests in a generic way and pushes the
> responsibility for accepting or rejecting them back onto the handler.
>
> If there's no chance this will be accepted into trunk, please let me
> know. I'd be happy to provide this as a "plugin" to monkey-patch
> mongrel for any people interested in this functionality.
>
> So, the patch touches three (3) classes and makes minor modifications.
>
> 1. class HttpHandler
> Adds a single method called #accept_partial_requests? which is false
> by default since most handlers do NOT want to handle a partial
> request. Those that do can simply override this method in the
> subclass. Used in HttpServer#process_client for determining if a
> handler should get called.
>
> 2. class HttpRequest
> Changes the logic in the #read_body method so it no longer deletes
> the TempFile and sets @body to nil as a "signal" that the request was
> interrupted. Adds a #attr_reader called #interrupted? for use outside
> the class along with the appropriate initialization statement (this
> is the new "signal"). Lastly, adds a method called #cleanup which
> essentially restores the original activity from #read_body and
> deletes the TempFile.
>
> 3. class HttpServer
> Some logic changes to the #process_client method which is the main
> workhorse of the class. It deletes the original statement that short
> circuits the loop ( break if request.body == nil) and instead pushes
> that check to the loop invoking all the handlers. The handler loop
> now checks each handler to verify #handler.accept_partial_requests?
> before passing a partial request to the handler. If it's a complete
> request, the original calling semantics apply. This means all
> existing handlers would continue to function as they did before. Only
> those handlers which specifically override #accept_partial_requests?
> will ever get called with a partial/malformed request.
>
> I'm a little concerned that these extra checks would be a performance
> drag in the general case. Any feedback on improvements here would be
> much appreciated.
>
> Lastly, the end of the method then calls #request.cleanup to delete
> any lingering TempFile.
>
>
> Anyway, enough talk... time for the code. It's all listed below,
> though currently *untested*. I want to get feedback from Zed and the
> group before I take the extra step of adding and testing all of this
> (though I expect it to pass all tests just like the original patch).
>
> Thanks for your attention.
>
> cr
>
> ---- code below ----
>
> module Mongrel
> class HttpHandler
> # NEW
> def accept_partial_requests?
> false
> end
> end
>
> class HttpRequest
> # NEW
> attr_reader :interrupted?
>
> def initialize
> # original code plus...
> self.interrupted? = false
> end
>
> # modify #read_body so it does NOT delete the @body if an
> exception is raised.
> def read_body(remain, total)
> begin
> # write the odd sized chunk first
> @params.http_body = read_socket(remain % Const::CHUNK_SIZE)
>
> remain -= @body.write(@params.http_body)
>
> update_request_progress(remain, total)
>
> # then stream out nothing but perfectly sized chunks
> until remain <= 0 or @socket.closed?
> # ASSUME: we are writing to a disk and these writes always
> write the requested amount
> @params.http_body = read_socket(Const::CHUNK_SIZE)
> remain -= @body.write(@params.http_body)
>
> update_request_progress(remain, total)
> end
> rescue Object
> STDERR.puts "ERROR reading http body: #$!"
> $!.backtrace.join("\n")
> @socket.close rescue Object
> self.interrupted? = true # NEW
> end
> end
>
> # NEW
> # clean up any lingering temp files
> def cleanup
> @body.delete if @body.class == Tempfile && interrupted?
> end
> end
>
>
> class HttpServer
> def process_client(client)
> begin
> # deleted for brevity
>
> while nparsed < data.length
> nparsed = parser.execute(params, data, nparsed)
>
> if parser.finished?
> # deleted for brevity
>
> if handlers
> # deleted for brevity
>
> # select handlers that want more detailed request
> notification
> notifiers = handlers.select { |h| h.request_notify }
> request = HttpRequest.new(params, client, notifiers)
>
> # break if request.body == nil ---- REMOVED
>
> # request is good so far, continue processing the
> response
> response = HttpResponse.new(client) unless
> request.interrupted? # NEW statement modifier
>
> # Process each handler in registered order until we
> run out or one finalizes the response.
> if request.interrupted?
> handlers.each do |handler|
> handler.process(request, nil) if
> handler.accept_partial_requests
> end
> else
> # ORIGINAL logic
> handlers.each do |handler|
> handler.process(request, response)
> break if response.done or client.closed?
> end
> end
>
> # NEW
> # In the case of large file uploads the user could
> close the socket, so the request is in a
> # partial state. Clean up after it by removing any
> TempFile that may exist.
> request.cleanup if request.interrupted?
>
> # And finally, if nobody closed the response off, we
> finalize it.
> unless response.done or client.closed?
> response.finished
> end
> else
> # brevity!
> end
>
> break #done
> else
> # deleted for brevity
> end
> end
> rescue
> EOFError,Errno::ECONNRESET,Errno::EPIPE,Errno::EINVAL,Errno::EBADF
> # deleted for brevity
> end
> end
> end
> end
>
> _______________________________________________
> Mongrel-users mailing list
> [email protected]
> http://rubyforge.org/mailman/listinfo/mongrel-users
>
-- Ezra Zygmuntowicz
-- Lead Rails Evangelist
-- [EMAIL PROTECTED]
-- Engine Yard, Serious Rails Hosting
-- (866) 518-YARD (9273)
_______________________________________________
Mongrel-users mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/mongrel-users
