Project

General

Profile

Actions

Feature #3622

closed

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

Added by drbrain (Eric Hodel) over 13 years ago. Updated almost 13 years ago.

Status:
Closed
Target version:
[ruby-core:31519]

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


Files

net.http.continue.patch (3.39 KB) net.http.continue.patch Proposed Net::HTTP that waits for a continue response drbrain (Eric Hodel), 07/28/2010 03:08 PM
net.http.continue.patch (3.42 KB) net.http.continue.patch net.http.continue.2.patch drbrain (Eric Hodel), 07/29/2010 05:17 AM
expect-continue.diff (3.35 KB) expect-continue.diff nahi (Hiroshi Nakamura), 09/09/2010 05:24 PM
expect-continue.diff (5.88 KB) expect-continue.diff nahi (Hiroshi Nakamura), 09/10/2010 07:40 PM
expect-continue.3.diff (6.28 KB) expect-continue.3.diff drbrain (Eric Hodel), 09/11/2010 05:26 AM
Actions #1

Updated by drbrain (Eric Hodel) over 13 years ago

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

Actions #2

Updated by usa (Usaku NAKAMURA) over 13 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

Actions #3

Updated by naruse (Yui NARUSE) over 13 years ago

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

Actions #4

Updated by shyouhei (Shyouhei Urabe) over 13 years ago

  • Status changed from Open to Feedback

=begin

=end

Actions #5

Updated by drbrain (Eric Hodel) over 13 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

Actions #6

Updated by nahi (Hiroshi Nakamura) over 13 years ago

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

=begin

=end

Actions #7

Updated by nahi (Hiroshi Nakamura) over 13 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

Actions #8

Updated by nahi (Hiroshi Nakamura) over 13 years ago

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

RDoc expected. Anyone?
=end

Actions #9

Updated by drbrain (Eric Hodel) over 13 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

Actions #10

Updated by shyouhei (Shyouhei Urabe) over 13 years ago

  • Status changed from Open to Assigned

=begin

=end

Actions #11

Updated by naruse (Yui NARUSE) over 13 years ago

=begin
What's going on?
=end

Updated by nahi (Hiroshi Nakamura) almost 13 years ago

  • Due date set to 05/31/2011

Updated by nahi (Hiroshi Nakamura) almost 13 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

Updated by nahi (Hiroshi Nakamura) almost 13 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.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0