Let on_headers_complete veto an upgrade and keep the body

httptools/parser/parser.pyx

@@ -128,7 +128,7 @@ cdef class HttpParser:
         self._maybe_call_on_header()
 
         if self._proto_on_headers_complete is not None:
-            self._proto_on_headers_complete()
+            return self._proto_on_headers_complete()
 
     cdef _on_chunk_header(self):
         if (self._current_header_value is not None or
@@ -352,13 +352,22 @@ cdef int cb_on_header_value(cparser.llhttp_t* parser,
 
 cdef int cb_on_headers_complete(cparser.llhttp_t* parser) except -1:
     cdef HttpParser pyparser = <HttpParser>parser.data
+    cdef object result
     try:
-        pyparser._on_headers_complete()
+        result = pyparser._on_headers_complete()
     except BaseException as ex:
         pyparser._last_error = ex
         return -1
     else:
-        if pyparser._cparser.upgrade:
+        # A protocol can veto an upgrade it doesn't want to honor (e.g. an
+        # `Upgrade: h2c` it plans to ignore) by returning False from its own
+        # `on_headers_complete`. Without this, llhttp always pauses parsing
+        # right here whenever the client asked for any upgrade, even one the
+        # application never intends to act on, leaving the rest of the
+        # message (its body, on a request that still carries one) stuck
+        # behind an HttpParserUpgrade the caller has no clean way to resume
+        # from, since the leftover bytes aren't a fresh message of their own.
+        if pyparser._cparser.upgrade and result is not False:
             return 1
         else:
             return 0

tests/test_parser.py

@@ -439,6 +439,79 @@ def on_message_complete(self):
         else:
             self.fail('HttpParserUpgrade was not raised')
 
+    def test_parser_request_upgrade_veto_keeps_body(self):
+        # A request can carry an Upgrade the receiving application has no
+        # intention of honoring (e.g. `Upgrade: h2c`, which plenty of clients
+        # send speculatively). RFC 7230 6.7 lets a server just ignore that
+        # and process the request as an ordinary HTTP/1.1 message, but until
+        # now on_headers_complete had no way to say so: llhttp always paused
+        # right there, and any body the request had (declared via
+        # Content-Length or Transfer-Encoding) was never delivered.
+        # Returning False from on_headers_complete opts out of the pause and
+        # lets the rest of the message parse normally.
+        request = (
+            b'POST / HTTP/1.1\r\n'
+            b'Host: example.com\r\n'
+            b'Upgrade: h2c\r\n'
+            b'Connection: Upgrade, HTTP2-Settings\r\n'
+            b'Transfer-Encoding: chunked\r\n'
+            b'\r\n'
+            b'3\r\nabc\r\n0\r\n\r\n'
+        )
+
+        class Protocol:
+            def __init__(self):
+                self.upgrade = None
+                self.body = b''
+                self.message_complete = False
+
+            def on_header(self, name, value):
+                if name.lower() == b'upgrade':
+                    self.upgrade = value
+
+            def on_body(self, data):
+                self.body += data
+
+            def on_message_complete(self):
+                self.message_complete = True
+
+            def on_headers_complete(self):
+                if self.upgrade is not None and self.upgrade.lower() == b'h2c':
+                    return False
+
+        protocol = Protocol()
+        parser = httptools.HttpRequestParser(protocol)
+        try:
+            parser.feed_data(request)
+        except httptools.HttpParserUpgrade:
+            # Still raised once the (now fully-parsed) message ends, so the
+            # application can act on the upgrade request afterwards if it
+            # wants to - vetoing the pause doesn't erase that it happened.
+            pass
+        else:
+            self.fail('HttpParserUpgrade was not raised')
+
+        self.assertTrue(protocol.message_complete)
+        self.assertEqual(protocol.body, b'abc')
+
+    def test_parser_request_upgrade_not_vetoed_still_pauses(self):
+        # An upgrade the application doesn't veto (the common case, e.g. a
+        # real WebSocket handshake) must keep behaving exactly as before:
+        # pause immediately at headers-complete, without waiting on a body.
+        m = mock.Mock()
+        m.on_headers_complete.return_value = None
+        p = httptools.HttpRequestParser(m)
+
+        try:
+            p.feed_data(UPGRADE_REQUEST1)
+        except httptools.HttpParserUpgrade as ex:
+            offset = ex.args[0]
+        else:
+            self.fail('HttpParserUpgrade was not raised')
+
+        self.assertEqual(UPGRADE_REQUEST1[offset:], b'Hot diggity dogg')
+        self.assertFalse(m.on_body.called)
+
     def test_parser_request_error_in_on_header(self):
         class Error(Exception):
             pass