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

• Status: Closed
• Assignee: nahi (Hiroshi Nakamura)
• Target version: 2.0.0

[drbrain (Eric Hodel)]

=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

[drbrain (Eric Hodel)]

• File net.http.continue.patch net.http.continue.patch added

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

[usa (Usaku NAKAMURA)]

=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

[naruse (Yui NARUSE)]

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

[shyouhei (Shyouhei Urabe)]

• Status changed from Open to Feedback

=begin

=end

[drbrain (Eric Hodel)]

=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

[nahi (Hiroshi Nakamura)]

• Status changed from Feedback to Open
• Assignee set to nahi (Hiroshi Nakamura)

=begin

=end

[nahi (Hiroshi Nakamura)]

• File expect-continue.diff expect-continue.diff added

=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

[nahi (Hiroshi Nakamura)]

• File expect-continue.diff expect-continue.diff added

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

RDoc expected. Anyone?
=end

[drbrain (Eric Hodel)]

• File expect-continue.3.diff expect-continue.3.diff added

=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

[shyouhei (Shyouhei Urabe)]

• Status changed from Open to Assigned

=begin

=end

[naruse (Yui NARUSE)]

=begin
What's going on?
=end

[nahi (Hiroshi Nakamura)]

• Due date set to 05/31/2011

[nahi (Hiroshi Nakamura)]

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

[nahi (Hiroshi Nakamura)]

• 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.

• 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 [e-mail].

• 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

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@31860 b2dd03c8-39d4-4d8f-98ff-823fe69b080e