Feature #3622

Net::HTTP does not wait to send request body with Expect: 100-continue

Added by Eric Hodel almost 5 years ago. Updated almost 4 years ago.

[ruby-core:31519]
Status:Closed
Priority:Normal
Assignee:Hiroshi Nakamura

Description

=begin
HTTP/1.1 allows a client to determine if the server will accept a request body using the Expect header with a value of 100-continue. If the server finds the request header the client sent acceptable it will return with a 100 Continue response and the client will then send the request body.

Instead of waiting for a 100 Continue response Net::HTTP immediately sends the request body and ignores any 100 Continue responses.

The current behavior defeats the purpose of the Expect: 100-continue value and the 100 Continue response code.

If I am attempting to upload a large file like a photo and to a server that requires HTTP authentication I will have to wait until the upload is complete before I can retrieve a 401 response for incorrect authentication.

I have attached a proposed patch that adds a continue timeout. Net::HTTP will wait up to the continue timeout before sending the request body.
=end

net.http.continue.patch Magnifier - Proposed Net::HTTP that waits for a continue response (3.39 KB) Eric Hodel, 07/28/2010 03:08 PM

net.http.continue.patch Magnifier - net.http.continue.2.patch (3.42 KB) Eric Hodel, 07/29/2010 05:17 AM

expect-continue.diff Magnifier (3.35 KB) Hiroshi Nakamura, 09/09/2010 05:24 PM

expect-continue.diff Magnifier (5.88 KB) Hiroshi Nakamura, 09/10/2010 07:40 PM

expect-continue.3.diff Magnifier (6.28 KB) Eric Hodel, 09/11/2010 05:26 AM

Associated revisions

Revision 31860
Added by Hiroshi Nakamura almost 4 years ago

  • lib/net/http.rb, lib/net/protocol.rb: Allow to configure to wait
    server returning '100 continue' response befor sending HTTP request
    body. See NEWS for more detail. See #3622.
    Original patch is made by Eric Hodel drbrain@segment7.net.

  • test/net/http/test_http.rb: test it.

  • NEWS: Add new feature.

On my env (Ubuntu 11.04 64bit),
9510 tests, 2203824 assertions, 0 failures, 0 errors, 29 skips
->
9514 tests, 2203836 assertions, 0 failures, 0 errors, 29 skips

Revision 31860
Added by Hiroshi Nakamura almost 4 years ago

  • lib/net/http.rb, lib/net/protocol.rb: Allow to configure to wait
    server returning '100 continue' response befor sending HTTP request
    body. See NEWS for more detail. See #3622.
    Original patch is made by Eric Hodel drbrain@segment7.net.

  • test/net/http/test_http.rb: test it.

  • NEWS: Add new feature.

On my env (Ubuntu 11.04 64bit),
9510 tests, 2203824 assertions, 0 failures, 0 errors, 29 skips
->
9514 tests, 2203836 assertions, 0 failures, 0 errors, 29 skips

History

#1 Updated by Eric Hodel almost 5 years ago

=begin
Updated patch, fixes bug where Expect header is not provided.
=end

#2 Updated by Usaku NAKAMURA almost 5 years ago

=begin
(1) Are there any grounds in the value of 0.5 seconds of the time-out?
(2) Can you write a test for this change?

After the above-mentioned point can be confirmed, I do not oppose taking this patch.
=end

#3 Updated by Yui NARUSE almost 5 years ago

=begin
I agree with this idea.
When usa's points are completed, you can commit it.
=end

#4 Updated by Shyouhei Urabe over 4 years ago

  • Status changed from Open to Feedback

=begin

=end

#5 Updated by Eric Hodel over 4 years ago

=begin
I don't see any reason to use 0.5 for the timeout. I think a timeout of 0 by default is acceptable to maintain compatible behavior.

I don't understand how to write a test for this that would go in test/net/http yet. I will try again.
=end

#6 Updated by Hiroshi Nakamura over 4 years ago

  • Assignee set to Hiroshi Nakamura
  • Status changed from Feedback to Open

=begin

=end

#7 Updated by Hiroshi Nakamura over 4 years ago

=begin
Here's a modified version of the patch. [expect-continue.diff]
I'll write test case.

Eric: Would you please confirm that this patch works for you? Thanks in advance.
=end

#8 Updated by Hiroshi Nakamura over 4 years ago

=begin
Patch updated with tests. Tests requires current trunk HEAD.
Eric, please try this patch.

RDoc expected. Anyone?
=end

#9 Updated by Eric Hodel over 4 years ago

=begin
With your latest patch if you do not set the continue_timeout after the server is started it does not wait for "100 Continue".

h = Net::HTTP.new host, port
h.continue_timeout = 10 # does nothing

With your latest patch Net::HTTP does not send the HTTP header immediately, it waits for the continue timeout first.

Placing wait_for_continue after sending headers fixes this.

Does Net::HTTP require additional RDoc? I think the current comments for the new methods are ok.
=end

#10 Updated by Shyouhei Urabe over 4 years ago

  • Status changed from Open to Assigned

=begin

=end

#11 Updated by Yui NARUSE over 4 years ago

=begin
What's going on?
=end

#12 Updated by Hiroshi Nakamura almost 4 years ago

  • Due date set to 05/31/2011

#13 Updated by Hiroshi Nakamura almost 4 years ago

Merged the update from Eric Hodel. Thanks Eric, and I'm sorry for posting the patch which does not include full changes. Can't remember why I thought the tests I added run correctly...

I'll commit this.
https://github.com/nahi/ruby/compare/4e4649e13cd4175aab75...0fa41a6f7b86c17b0225

#14 Updated by Hiroshi Nakamura almost 4 years ago

  • Status changed from Assigned to Closed

I close this since I believe r31860 includes the original intent of the patch from Eric. Please reopen this if it doesn't work.

Also available in: Atom PDF