wsgi: Don't break HTTP framing during 100-continue handling

Expect: 100-continue is a funny beast -- the client sends it to indicate
that it's willing to wait for an early error, but

- the client has no guarantee that the server supports 100 Continue,
- the server gets no indication of how long the client's willing to wait
  for the go/no-go response, and
- even if it did, the server has no way of knowing that the response it
  *emitted* within that time was actually *received* within that time
- so the client may have started sending the body regardless of what the
  server's done.

As a result, the server only has two options when it *does not* send the
100 Continue response:

- close the socket
- read and discard the request body

Previously, we did neither of these things; as a result, a request body
could be interpreted as a new request. Now, close out the connection,
including sending a `Connection: close` header when practical.

2 files changed, 110 insertions, 10 deletions

diff --git a/eventlet/wsgi.py b/eventlet/wsgi.py
index 75205a7..7ef0254 100644
--- a/eventlet/wsgi.py
+++ b/eventlet/wsgi.py
@@ -134,8 +134,12 @@ class Input(object):
         # Reinitialize chunk_length (expect more data)
         self.chunk_length = -1
 
+    @property
+    def should_send_hundred_continue(self):
+        return self.wfile is not None and not self.is_hundred_continue_response_sent
+
     def _do_read(self, reader, length=None):
-        if self.wfile is not None and not self.is_hundred_continue_response_sent:
+        if self.should_send_hundred_continue:
             # 100 Continue response
             self.send_hundred_continue_response()
             self.is_hundred_continue_response_sent = True
@@ -152,7 +156,7 @@ class Input(object):
         return read
 
     def _chunked_read(self, rfile, length=None, use_readline=False):
-        if self.wfile is not None and not self.is_hundred_continue_response_sent:
+        if self.should_send_hundred_continue:
             # 100 Continue response
             self.send_hundred_continue_response()
             self.is_hundred_continue_response_sent = True
@@ -464,9 +468,11 @@ class HttpProtocol(BaseHTTPServer.BaseHTTPRequestHandler):
         start = time.time()
         headers_set = []
         headers_sent = []
+        # Grab the request input now; app may try to replace it in the environ
+        request_input = self.environ['eventlet.input']
         # Push the headers-sent state into the Input so it won't send a
         # 100 Continue response if we've already started a response.
-        self.environ['wsgi.input'].headers_sent = headers_sent
+        request_input.headers_sent = headers_sent
 
         wfile = self.wfile
         result = None
@@ -563,10 +569,15 @@ class HttpProtocol(BaseHTTPServer.BaseHTTPRequestHandler):
                 result = self.application(self.environ, start_response)
 
                 # Set content-length if possible
-                if headers_set and \
-                        not headers_sent and hasattr(result, '__len__') and \
-                        'Content-Length' not in [h for h, _v in headers_set[1]]:
-                    headers_set[1].append(('Content-Length', str(sum(map(len, result)))))
+                if headers_set and not headers_sent and hasattr(result, '__len__'):
+                    # We've got a complete final response
+                    if 'Content-Length' not in [h for h, _v in headers_set[1]]:
+                        headers_set[1].append(('Content-Length', str(sum(map(len, result)))))
+                    if request_input.should_send_hundred_continue:
+                        # We've got a complete final response, and never sent a 100 Continue.
+                        # There's no chance we'll need to read the body as we stream out the
+                        # response, so we can be nice and send a Connection: close header.
+                        self.close_connection = 1
 
                 towrite = []
                 towrite_size = 0
@@ -607,11 +618,22 @@ class HttpProtocol(BaseHTTPServer.BaseHTTPRequestHandler):
         finally:
             if hasattr(result, 'close'):
                 result.close()
-            request_input = self.environ['eventlet.input']
+            if request_input.should_send_hundred_continue:
+                # We just sent the final response, no 100 Continue. Client may or
+                # may not have started to send a body, and if we keep the connection
+                # open we've seen clients either
+                #   * send a body, then start a new request
+                #   * skip the body and go straight to a new request
+                # Looks like the most broadly compatible option is to close the
+                # connection and let the client retry.
+                # https://curl.se/mail/lib-2004-08/0002.html
+                # Note that we likely *won't* send a Connection: close header at this point
+                self.close_connection = 1
+
             if (request_input.chunked_input or
                     request_input.position < (request_input.content_length or 0)):
-                # Read and discard body if there was no pending 100-continue
-                if not request_input.wfile and self.close_connection == 0:
+                # Read and discard body if connection is going to be reused
+                if self.close_connection == 0:
                     try:
                         request_input.discard()
                     except ChunkReadError as e:
diff --git a/tests/wsgi_test.py b/tests/wsgi_test.py
index d833400..999d6f0 100644
--- a/tests/wsgi_test.py
+++ b/tests/wsgi_test.py
@@ -608,6 +608,57 @@ class TestHttpd(_TestBase):
         self.assertEqual('keep-alive', result2.headers_lower['connection'])
         sock.close()
 
+    def test_018b_http_10_keepalive_framing(self):
+        # verify that if an http/1.0 client sends connection: keep-alive
+        # that we don't mangle the request framing if the app doesn't read the request
+        def app(environ, start_response):
+            resp_body = {
+                '/1': b'first response',
+                '/2': b'second response',
+                '/3': b'third response',
+            }.get(environ['PATH_INFO'])
+            if resp_body is None:
+                resp_body = 'Unexpected path: ' + environ['PATH_INFO']
+                if six.PY3:
+                    resp_body = resp_body.encode('latin1')
+            # Never look at wsgi.input!
+            start_response('200 OK', [('Content-type', 'text/plain')])
+            return [resp_body]
+
+        self.site.application = app
+        sock = eventlet.connect(self.server_addr)
+        req_body = b'GET /tricksy HTTP/1.1\r\n'
+        body_len = str(len(req_body)).encode('ascii')
+
+        sock.sendall(b'PUT /1 HTTP/1.0\r\nHost: localhost\r\nConnection: keep-alive\r\n'
+                     b'Content-Length: ' + body_len + b'\r\n\r\n' + req_body)
+        result1 = read_http(sock)
+        self.assertEqual(b'first response', result1.body)
+        self.assertEqual(result1.headers_original.get('Connection'), 'keep-alive')
+
+        sock.sendall(b'PUT /2 HTTP/1.0\r\nHost: localhost\r\nConnection: keep-alive\r\n'
+                     b'Content-Length: ' + body_len + b'\r\nExpect: 100-continue\r\n\r\n')
+        # Client may have a short timeout waiting on that 100 Continue
+        # and basically immediately send its body
+        sock.sendall(req_body)
+        result2 = read_http(sock)
+        self.assertEqual(b'second response', result2.body)
+        self.assertEqual(result2.headers_original.get('Connection'), 'close')
+
+        sock.sendall(b'PUT /3 HTTP/1.0\r\nHost: localhost\r\nConnection: close\r\n\r\n')
+        with self.assertRaises(ConnectionClosed):
+            read_http(sock)
+        sock.close()
+
+        # retry
+        sock = eventlet.connect(self.server_addr)
+        sock.sendall(b'PUT /3 HTTP/1.0\r\nHost: localhost\r\nConnection: close\r\n\r\n')
+        result3 = read_http(sock)
+        self.assertEqual(b'third response', result3.body)
+        self.assertEqual(result3.headers_original.get('Connection'), 'close')
+
+        sock.close()
+
     def test_019_fieldstorage_compat(self):
         def use_fieldstorage(environ, start_response):
             cgi.FieldStorage(fp=environ['wsgi.input'], environ=environ)
@@ -756,9 +807,23 @@ class TestHttpd(_TestBase):
                  b'Expect: 100-continue\r\n\r\n')
         fd.flush()
         result = read_http(sock)
+        # No "100 Continue" -- straight to final response
         self.assertEqual(result.status, 'HTTP/1.1 417 Expectation Failed')
         self.assertEqual(result.body, b'failure')
+        self.assertEqual(result.headers_original.get('Connection'), 'close')
+        # Client may still try to send the body
+        fd.write(b'x' * 25)
+        fd.flush()
+        # But if they keep using this socket, it's going to close on them eventually
+        fd.write(b'x' * 25)
+        with self.assertRaises(socket.error) as caught:
+            fd.flush()
+        self.assertEqual(caught.exception.errno, errno.EPIPE)
+        sock.close()
 
+        sock = eventlet.connect(self.server_addr)
+        fd = sock.makefile('rwb')
+        # If we send the "100 Continue", we can pipeline requests through the one connection
         for expect_value in ('100-continue', '100-Continue'):
             fd.write(
                 'PUT / HTTP/1.1\r\nHost: localhost\r\nContent-length: 7\r\n'
@@ -767,6 +832,8 @@ class TestHttpd(_TestBase):
             header_lines = []
             while True:
                 line = fd.readline()
+                if not line:
+                    raise ConnectionClosed
                 if line == b'\r\n':
                     break
                 else:
@@ -775,11 +842,14 @@ class TestHttpd(_TestBase):
             header_lines = []
             while True:
                 line = fd.readline()
+                if not line:
+                    raise ConnectionClosed
                 if line == b'\r\n':
                     break
                 else:
                     header_lines.append(line)
             assert header_lines[0].startswith(b'HTTP/1.1 200 OK')
+            assert 'Connection: close' not in header_lines
             assert fd.read(7) == b'testing'
         fd.close()
         sock.close()
@@ -806,6 +876,14 @@ class TestHttpd(_TestBase):
         result = read_http(sock)
         self.assertEqual(result.status, 'HTTP/1.1 417 Expectation Failed')
         self.assertEqual(result.body, b'failure')
+        self.assertEqual(result.headers_original.get('Connection'), 'close')
+        # At this point, the client needs to either kill the connection or send the bytes
+        # because even though the server sent the response without reading the body,
+        # it has no way of knowing whether the client already started sending or not
+        sock.close()
+        sock = eventlet.connect(self.server_addr)
+        fd = sock.makefile('rwb')
+
         fd.write(
             b'PUT / HTTP/1.1\r\nHost: localhost\r\nContent-length: 7\r\n'
             b'Expect: 100-continue\r\n\r\ntesting')