Project

General

Profile

Bug #12890

Net::HTTP should treat unexpected 1XX responses as non-final.

Added by lukasa (Cory Benfield) about 3 years ago. Updated about 3 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-darwin16]
[ruby-core:77866]

Description

Long story short

Net::HTTP's client does not tolerate non-100 status codes from the 1XX block in a sensible manner: it treats them as final responses, rather than as provisional ones.

Expected behaviour

When Net::HTTP receives a 1XX status code that it does not recognise, it should either surface these to a user in such a way that allows the user to access the subsequent final response, or it should ignore them and only show the user the eventual final status code. This is required by RFC 7231 Section 6.2 (Informational 1xx), which reads:

A client MUST be able to parse one or more 1xx responses received prior to a final response, even if the client does not expect one. A user agent MAY ignore unexpected 1xx responses.

Actual behaviour

Net::HTTP treats the 1XX status code as final. It parses the header block as though it were a response in the 2XX or higher range. Net::HTTP rightly assumes that 1XX responses have no body, which means that it leaves the following 2XX response unconsumed in the socket buffer, which will hide critical data from the user.

Steps to reproduce

The following Python "server" can demonstrate the issue:

import socket
import time

document = b'''<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8">
    <title>title</title>
    <link rel="stylesheet" href="/other/styles.css">
    <script src="/other/action.js"></script>
  </head>
  <body>
    <h1>Hello, world!</h1>
  </body>
</html>
'''

s = socket.socket()
s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
s.bind(('localhost', 8080))
s.listen(5)

while True:
    new_socket, _ = s.accept()
    data = b''

    while not data.endswith(b'\r\n\r\n'):
        data += new_socket.recv(8192)

    new_socket.sendall(
        b'HTTP/1.1 103 Early Hints\r\n'
        b'Server: socketserver/1.0.0\r\n'
        b'Link: </other/styles.css>; rel=preload; as=style\r\n'
        b'Link: </other/action.js>; rel=preload; as=script\r\n'
        b'\r\n'
    )
    time.sleep(1)
    new_socket.sendall(
        b'HTTP/1.1 200 OK\r\n'
        b'Server: socketserver/1.0.0\r\n'
        b'Content-Type: text/html\r\n'
        b'Content-Length: %s\r\n'
        b'Link: </other/styles.css>; rel=preload; as=style\r\n'
        b'Link: </other/action.js>; rel=preload; as=script\r\n'
        b'Connection: close\r\n'
        b'\r\n' % len(document)
    )
    new_socket.sendall(document)
    new_socket.close()

If this server is run directly, the following client can be used to test it:

require 'net/http'

uri = URI('http://localhost:8080/')
res = Net::HTTP.get_response(uri)
puts "Status: #{res.code}"
puts "Body: #{res.body}"

This client will print

Status: 103
Body:

Note that the client has treated the 1XX response as final, and has left the 200 response unconsumed in the socket.


Files

patch1.diff (2.17 KB) patch1.diff phluid61 (Matthew Kerwin), 11/02/2016 10:02 PM

Associated revisions

Revision 8ec6fcb2
Added by naruse (Yui NARUSE) about 3 years ago

  • lib/net/http.rb (transport_request): other than HTTPContinue in 1xx (HTTPInformation) also needs to continue. [Bug #12890]

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

Revision 56596
Added by naruse (Yui NARUSE) about 3 years ago

  • lib/net/http.rb (transport_request): other than HTTPContinue in 1xx (HTTPInformation) also needs to continue. [Bug #12890]

Revision 56596
Added by naruse (Yui NARUSE) about 3 years ago

  • lib/net/http.rb (transport_request): other than HTTPContinue in 1xx (HTTPInformation) also needs to continue. [Bug #12890]

Revision 56596
Added by naruse (Yui NARUSE) about 3 years ago

  • lib/net/http.rb (transport_request): other than HTTPContinue in 1xx (HTTPInformation) also needs to continue. [Bug #12890]

Revision 56596
Added by naruse (Yui NARUSE) about 3 years ago

  • lib/net/http.rb (transport_request): other than HTTPContinue in 1xx (HTTPInformation) also needs to continue. [Bug #12890]

Revision edf505a7
Added by nagachika (Tomoyuki Chikanaga) about 3 years ago

merge revision(s) 56596: [Backport #12890]

    * lib/net/http.rb (transport_request): other than HTTPContinue
      in 1xx (HTTPInformation) also needs to continue. [Bug #12890]

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

Revision 56782
Added by nagachika (Tomoyuki Chikanaga) about 3 years ago

merge revision(s) 56596: [Backport #12890]

* lib/net/http.rb (transport_request): other than HTTPContinue
  in 1xx (HTTPInformation) also needs to continue. [Bug #12890]

Revision bb5114bc
Added by usa (Usaku NAKAMURA) about 3 years ago

merge revision(s) 56596: [Backport #12890]

    * lib/net/http.rb (transport_request): other than HTTPContinue
      in 1xx (HTTPInformation) also needs to continue. [Bug #12890]

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

Revision 56787
Added by usa (Usaku NAKAMURA) about 3 years ago

merge revision(s) 56596: [Backport #12890]

* lib/net/http.rb (transport_request): other than HTTPContinue
  in 1xx (HTTPInformation) also needs to continue. [Bug #12890]

History

Updated by duerst (Martin Dürst) about 3 years ago

  • Subject changed from Net::HTTP treats unexpected 1XX responses as final. to Net::HTTP should treat unexpected 1XX responses as non-final.

Updated by phluid61 (Matthew Kerwin) about 3 years ago

I believe the fix is relatively simple; the main change is to lib/net/http.rb:

--- a/lib/net/http.rb
+++ b/lib/net/http.rb
@@ -1439,7 +1439,7 @@ def transport_request(req)
           begin
             res = HTTPResponse.read_new(@socket)
             res.decode_content = req.decode_content
-          end while res.kind_of?(HTTPContinue)
+          end while res.continue?

           res.uri = req.uri

Updated by lukasa (Cory Benfield) about 3 years ago

Apologies, it was noted that it would be more helpful to provide the example server in Ruby instead:


require 'socket'

server = TCPServer.new 8080

loop do
    client = server.accept
    client.puts "HTTP/1.1 103 Early Hints\r\n" +
                "Server: socketserver/1.0.0\r\n" +
                "Link: </other/styles.css>; rel=preload; as=style\r\n" +
                "Link: </other/action.js>; rel=preload; as=script\r\n" +
                "\r\n"
    sleep 1
    client.puts "HTTP/1.1 200 OK\r\n" +
                "Server: socketserver/1.0.0\r\n" +
                "Content-Length: 0\r\n" +
                "Link: </other/styles.css>; rel=preload; as=style\r\n" +
                "Link: </other/action.js>; rel=preload; as=script\r\n" +
                "Connection: close\r\n" +
                "\r\n"
    client.close
end
#5

Updated by naruse (Yui NARUSE) about 3 years ago

  • Status changed from Open to Closed

Applied in changeset r56596.


  • lib/net/http.rb (transport_request): other than HTTPContinue in 1xx (HTTPInformation) also needs to continue. [Bug #12890]

Updated by duerst (Martin Dürst) about 3 years ago

  • Backport changed from 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN to 2.1: REQUIRED, 2.2: REQUIRED, 2.3: REQUIRED

Updated by nagachika (Tomoyuki Chikanaga) about 3 years ago

  • Backport changed from 2.1: REQUIRED, 2.2: REQUIRED, 2.3: REQUIRED to 2.1: REQUIRED, 2.2: REQUIRED, 2.3: DONE

ruby_2_3 r56782 merged revision(s) 56596.

Updated by usa (Usaku NAKAMURA) about 3 years ago

  • Backport changed from 2.1: REQUIRED, 2.2: REQUIRED, 2.3: DONE to 2.1: REQUIRED, 2.2: DONE, 2.3: DONE

ruby_2_2 r56787 merged revision(s) 56596.

Also available in: Atom PDF