perf: Optimized partial boundary detection on chunk borders

When parsing segment body chunks, we have to deal with partial boundaries at chunk
borders. Currently we always keep `len(boundary)-1` bytes in the buffer 'just in case'
and merge it with the next chunk, which is quite expensive. We can avoid this overhead
in most cases by checking for an actual partial match.

Benchmarks show a 10-37% throughput increase for typical file uploads and slight
speedups in most of the other tested scenarios.

feat: Boundary delimiters inside segment bodies are now detected and reported as
fatal errors in most cases (see RFC-7578 4.1).

Author: defnull

CHANGELOG.rst

@@ -19,13 +19,17 @@ Release 2.0 (not released yet)
   :cls:`bytearray` for part body chunks. Update your `isinstance` checks
   if you have any. 
 * change: Enforce `part_limit` (128 by default) for url-encoded data in
-  `parse_form_data()`. This is consistent with the handling of multipart
-  and an important safeguard against denial of service.
+  `parse_form_data()`. This is consistent with multipart handling and an
+  important safeguard against denial of service attacks.
 * change: New strict-mode check to reject extremely large boundaries.
 * change: Raise more helpful :exc:`ParserStateError` instead of implicit
   :exc:`AssertionError` or :exc:`TypeError` when the parser is used
   incorrectly.
+* change: The boundary delimiter must not appear inside a segment body
+  (RFC-7578 4.1). This triggers a fatal error now.
 * feat: Hardened (and faster) header validation.
+* perf: Up to 37% faster parsing for typical file uploads and large
+  text fields.
 * build: Change default branch to `main`.
 
 

multipart.py

@@ -476,8 +476,11 @@ def parse(self, chunk: t_ByteString) -> Generator[t_ParserEvent, None, None]:
             bufferlen = len(buffer)
             offset = 0
 
-            while True:
+            while offset < bufferlen:
                 if self._state is _PREAMBLE:
+                    if bufferlen < d_len:
+                        break  # Not enough data to find the initial delimiter
+
                     # Scan for first delimiter (CRLF prefix is optional here)
                     index = buffer.find(delimiter[2:], offset)
 
@@ -502,16 +505,21 @@ def parse(self, chunk: t_ByteString) -> Generator[t_ParserEvent, None, None]:
                             break  # parsing complete
                         elif tail[0:1] == b"\n":  # Broken client or legacy test case
                             raise ParserError("Invalid line break after first boundary")
-                        elif len(tail) == 2:
+                        elif next_start <= bufferlen:
                             raise ParserError("Unexpected byte after first boundary")
+                        else:  # 2-byte tail not in buffer
+                            offset = max(0, index - 2)
+                            break  # wait for more data
 
                     elif self.strict and bufferlen >= d_len:
                         # No boundary in first chunk -> Fail fast in strict mode
                         # and do not waste time consuming a legacy preamble.
                         raise StrictParserError("Boundary not found in first chunk")
 
-                    # Delimiter not found, skip data until we find one
-                    offset = max(0, bufferlen - (d_len + 1))
+                    # Boundary not found. Skip the preamble, but keep any bytes that may
+                    # belong to a partial boundary at the end of the buffer.
+                    index = buffer.rfind(b"\r", bufferlen - (d_len - 1))
+                    offset = bufferlen if index == -1 else index
                     break  # wait for more data
 
                 elif self._state is _HEADER:
@@ -538,34 +546,52 @@ def parse(self, chunk: t_ByteString) -> Generator[t_ParserEvent, None, None]:
                         break  # wait for more data
 
                 elif self._state is _BODY:
-                    # Scan for delimiter: CRLF + boundary + (CRLF or '--')
+                    # Scan for next boundary: CRLF + boundary
                     index = buffer.find(delimiter, offset)
                     if index > -1:
+                        # Emit everything up to the boundary
+                        if index > offset:
+                            yield self._on_segment_payload(buffer[offset:index])
+                            offset = index
+
                         next_start = index + d_len + 2
                         tail = buffer[next_start - 2 : next_start]
 
-                        if tail == b"\r\n" or tail == b"--":
-                            if index > offset:
-                                yield self._on_segment_payload(buffer[offset:index])
-
-                            offset = next_start
+                        if tail == b"\r\n":
+                            # Normal boundary: CRLF + boundary + CRLF
+                            self._on_segment_complete()
+                            yield None  # end of segment
+                            offset += d_len + 2
+                            self._on_segment_start()
+                            self._state = _HEADER
+                            continue
+                        elif tail == b"--":
+                            # Final boundary: CRLF + boundary + '--'
                             self._on_segment_complete()
                             yield None  # end of segment
+                            offset += d_len + 2
+                            self._state = _COMPLETE
+                            break
+                        elif next_start > bufferlen:  # 2-byte tail not in buffer
+                            break  # wait for more data
+                        else:
+                            raise ParserError("Unexpected bytes after boundary")
+
+                    # Boundary not found. Emit as much data as we can, but keep any bytes
+                    # that may belong to a partial boundary at the end of the buffer.
+                    index = buffer.rfind(b"\r", max(offset, bufferlen - (d_len - 1)))
+                    if index == -1 or not delimiter.startswith(buffer[index:]):
+                        # No partail boundary found, emit everything
+                        # This is a huge deal because it avoids buffer stitching next round
+                        yield self._on_segment_payload(
+                            buffer[offset:] if offset else buffer
+                        )
+                        offset = bufferlen
+                    elif index > offset:
+                        # Potential partial boundary found. Emit data up to that point
+                        yield self._on_segment_payload(buffer[offset:index])
+                        offset = index
 
-                            if tail == b"--":  # Last delimiter
-                                self._state = _COMPLETE
-                                break
-                            else:  # Normal delimiter
-                                self._on_segment_start()
-                                self._state = _HEADER
-                                continue
-
-                    # Only consume bytes that cannot be part of a partial delimiter at
-                    # the end of the buffer.
-                    flush_until = bufferlen - (d_len + 1)
-                    if flush_until > offset:
-                        yield self._on_segment_payload(buffer[offset:flush_until])
-                        offset = flush_until
                     break  # wait for more data
 
                 else:  # pragma: no cover

test/test_push_parser.py

@@ -513,6 +513,35 @@ def test_reject_boundary_in_preamble(self):
                 "--boundary--",
             )
 
+    def test_reject_boundary_in_payload(self):
+        """The boundary delimiter (e.g. "\r\n--boundary") must not appear in segment
+        bodies."""
+
+        def assert_bad_content(*content):
+            self.reset()
+            with self.assertParseError(
+                "Unexpected bytes after boundary"
+            ):
+                print(
+                    self.parse(
+                        "--boundary\r\n",
+                        'Content-Disposition: form-data; name="bad"\r\n',
+                        "\r\n",
+                        *content,
+                        "\r\n--boundary--",
+                        "",  # trigger close
+                    )
+                )
+
+        assert_bad_content("\r\n--boundary...")
+        assert_bad_content("\r\n--boundary\r")
+        assert_bad_content("\r\n--boundary-")
+
+        # This fake-boundary merges with the next real boundary into a
+        # valid boundary, which is still detected as an error, but with
+        # a different error message.
+        # assert_bad_content("\r\n--boundary")
+
     def test_accept_crln_before_start_boundary(self):
         """While uncommon, a single \\r\\n before and after the first and last
         boundary should be accepted even in strict mode."""