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
Mongrel-users@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-users
