Repository: pallets/werkzeug (python)

Issue:
Parsing of some post form data fails on empty fields
Some post form data containing empty field values fail to be parsed correctly, even though they are parsed successfully with other frameworks like python cgi or multiport. The problem is with such data (multiline for legibility):

```
--foo\r\n
Content-Type: text/plain; charset="UTF-8"\r\n
Content-Disposition: form-data; name=empty\r\n
\r\n
--foo\r\n
Content-Type: text/plain; charset="UTF-8"\r\n
Content-Disposition: form-data; name=text\r\n
\r\n
Some Text\r\n
--foo--
```

The field "empty" is indeed left empty. It's unclear from the RFC if it should be followed by 1 or 2 CRLF before the next field. Browsers usually use 2 CRLF, but the Google App Engine uses only 1 for data posted internally. The code below shows the difference between cgi, multipart and werkzeug in this case.

```
from io import BytesIO
from werkzeug.formparser import parse_form_data
import cgi
import multipart

data = b'--foo\r\nContent-Type: text/plain; charset="UTF-8"\r\nContent-Disposition: form-data; name=empty\r\n\r\n--foo\r\nContent-Type: text/plain; charset="UTF-8"\r\nContent-Disposition: form-data; name=text\r\n\r\nSome Text\r\n--foo--'

environ = {'CONTENT_LENGTH': str(len(data)), 
        'CONTENT_TYPE': 'multipart/form-data; boundary="foo"',
        'REQUEST_METHOD': 'POST',
        'boundary': b'foo'}

print('### werkzeug')
environ['wsgi.input'] = BytesIO(data)
stream, form, files = parse_form_data(environ)
for k, v in form.items():
    print({'field': k, 'value': v})
print('###')
    
print('### cgi: ')
stream = BytesIO(data)
print(cgi.parse_multipart(stream, environ))
print('###')

print('### multipart')
environ['wsgi.input'] = BytesIO(data)
(form, files) = multipart.parse_form_data(environ)
for k, v in form.items():
    print({'field': k, 'value': v})
print('###')
```

Output:
```
### werkzeug
{'field': 'empty', 'value': '--foo\r\nContent-Type: text/plain; charset="UTF-8"\r\nContent-Disposition: form-data; name=text\r\n\r\nSome Text'}
###
### cgi: 
{'text': ['Some Text'], 'empty': ['']}
###
### multipart
{'field': 'empty', 'value': ''}
{'field': 'text', 'value': 'Some Text'}
###
```

As you can see, werkzeug thinks that the value of the empty field is the boundary and what follows as it apparently expects 2 CRLF. Other frameworks correctly return both values, empty and not empty.

Environment:

- Python version: 3.11
- Werkzeug version: 2.2.3


Pull request:
Correctly implement multipart parsing for empty fields
According to RFC2046

    body-part := MIME-part-headers [CRLF *OCTET]

Meaning there should only be a blank line if there is are body octets, hence if the field is empty it should not be present. The previous implementation assumed the blacnk line would always be present.

This fixes both the decoder and encoder to ensure that empty fields are correctly decoded and encoded.

Closes #2632

Checklist:

- [x] Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
- [x] Add or update relevant docs, in the docs folder and in code.
- [x] Add an entry in `CHANGES.rst` summarizing the change and linking to the issue.
- [x] Add `.. versionchanged::` entries in any relevant code docs.
- [x] Run `pre-commit` hooks and fix any issues.
- [x] Run `pytest` and `tox`, no tests failed.


Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 87dfed5a..30fe8ae2 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -99,6 +99,7 @@ Unreleased
 -   ``secure_filename`` looks for more Windows reserved file names. :pr:`2622`
 -   Update type annotation for ``best_match`` to make ``default`` parameter clearer.
     :issue:`2625`
+-   Multipart parser handles empty fields correctly. :issue:`2632`
 
 
 Version 2.2.3
diff --git a/src/werkzeug/sansio/multipart.py b/src/werkzeug/sansio/multipart.py
index 2684e5dd..845a0a4e 100644
--- a/src/werkzeug/sansio/multipart.py
+++ b/src/werkzeug/sansio/multipart.py
@@ -4,6 +4,7 @@ from enum import auto
 from enum import Enum
 from typing import cast
 from typing import List
+from typing import Match
 from typing import Optional
 from typing import Tuple
 
@@ -58,6 +59,7 @@ class State(Enum):
     PREAMBLE = auto()
     PART = auto()
     DATA = auto()
+    DATA_START = auto()
     EPILOGUE = auto()
     COMPLETE = auto()
 
@@ -172,7 +174,11 @@ class MultipartDecoder:
             match = BLANK_LINE_RE.search(self.buffer, self._search_position)
             if match is not None:
                 headers = self._parse_headers(self.buffer[: match.start()])
-                del self.buffer[: match.end()]
+                # The final header ends with a single CRLF, however a
+                # blank line indicates the start of the
+                # body. Therefore the end is after the first CRLF.
+                headers_end = (match.start() + match.end()) // 2
+                del self.buffer[:headers_end]
 
                 if "content-disposition" not in headers:
                     raise ValueError("Missing Content-Disposition header")
@@ -193,7 +199,7 @@ class MultipartDecoder:
                         headers=headers,
                         name=name,
                     )
-                self.state = State.DATA
+                self.state = State.DATA_START
                 self._search_position = 0
                 self._parts_decoded += 1
 
@@ -205,28 +211,15 @@ class MultipartDecoder:
                 # safe buffer for part of the search target.
                 self._search_position = max(0, len(self.buffer) - SEARCH_EXTRA_LENGTH)
 
-        elif self.state == State.DATA:
-            if self.buffer.find(b"--" + self.boundary) == -1:
-                # No complete boundary in the buffer, but there may be
-                # a partial boundary at the end. As the boundary
-                # starts with either a nl or cr find the earliest and
-                # return up to that as data.
-                data_length = del_index = self.last_newline()
-                more_data = True
-            else:
-                match = self.boundary_re.search(self.buffer)
-                if match is not None:
-                    if match.group(1).startswith(b"--"):
-                        self.state = State.EPILOGUE
-                    else:
-                        self.state = State.PART
-                    data_length = match.start()
-                    del_index = match.end()
-                else:
-                    data_length = del_index = self.last_newline()
-                more_data = match is None
+        elif self.state == State.DATA_START:
+            data, del_index, more_data = self._parse_data(self.buffer, start=True)
+            del self.buffer[:del_index]
+            event = Data(data=data, more_data=more_data)
+            if more_data:
+                self.state = State.DATA
 
-            data = bytes(self.buffer[:data_length])
+        elif self.state == State.DATA:
+            data, del_index, more_data = self._parse_data(self.buffer, start=False)
             del self.buffer[:del_index]
             if data or not more_data:
                 event = Data(data=data, more_data=more_data)
@@ -252,6 +245,28 @@ class MultipartDecoder:
                 headers.append((name.strip(), value.strip()))
         return Headers(headers)
 
+    def _parse_data(self, data: bytes, *, start: bool) -> Tuple[bytes, int, bool]:
+        # Body parts must start with CRLF (or CR or LF)
+        if start:
+            match = LINE_BREAK_RE.match(data)
+            data_start = cast(Match[bytes], match).end()
+        else:
+            data_start = 0
+
+        match = self.boundary_re.search(data)
+        if match is not None:
+            if match.group(1).startswith(b"--"):
+                self.state = State.EPILOGUE
+            else:
+                self.state = State.PART
+            data_end = match.start()
+            del_index = match.end()
+        else:
+            data_end = del_index = self.last_newline()
+        more_data = match is None
+
+        return bytes(data[data_start:data_end]), del_index, more_data
+
 
 class MultipartEncoder:
     def __init__(self, boundary: bytes) -> None:
@@ -267,7 +282,6 @@ class MultipartEncoder:
             State.PART,
             State.DATA,
         }:
-            self.state = State.DATA
             data = b"\r\n--" + self.boundary + b"\r\n"
             data += b'Content-Disposition: form-data; name="%s"' % _to_bytes(event.name)
             if isinstance(event, File):
@@ -276,8 +290,14 @@ class MultipartEncoder:
             for name, value in cast(Field, event).headers:
                 if name.lower() != "content-disposition":
                     data += _to_bytes(f"{name}: {value}\r\n")
-            data += b"\r\n"
+            self.state = State.DATA_START
             return data
+        elif isinstance(event, Data) and self.state == State.DATA_START:
+            self.state = State.DATA
+            if len(event.data) > 0:
+                return b"\r\n" + event.data
+            else:
+                return event.data
         elif isinstance(event, Data) and self.state == State.DATA:
             return event.data
         elif isinstance(event, Epilogue):


Test patch:
diff --git a/tests/sansio/test_multipart.py b/tests/sansio/test_multipart.py
index f9c48b47..1d836d64 100644
--- a/tests/sansio/test_multipart.py
+++ b/tests/sansio/test_multipart.py
@@ -30,7 +30,7 @@ asdasd
     decoder.receive_data(data)
     decoder.receive_data(None)
     events = [decoder.next_event()]
-    while not isinstance(events[-1], Epilogue) and len(events) < 6:
+    while not isinstance(events[-1], Epilogue):
         events.append(decoder.next_event())
     assert events == [
         Preamble(data=b""),
@@ -78,3 +78,58 @@ def test_chunked_boundaries() -> None:
     assert not event.more_data
     decoder.receive_data(None)
     assert isinstance(decoder.next_event(), Epilogue)
+
+
+def test_empty_field() -> None:
+    boundary = b"foo"
+    decoder = MultipartDecoder(boundary)
+    data = """
+--foo
+Content-Disposition: form-data; name="text"
+Content-Type: text/plain; charset="UTF-8"
+
+Some Text
+--foo
+Content-Disposition: form-data; name="empty"
+Content-Type: text/plain; charset="UTF-8"
+
+--foo--
+    """.replace(
+        "\n", "\r\n"
+    ).encode(
+        "utf-8"
+    )
+    decoder.receive_data(data)
+    decoder.receive_data(None)
+    events = [decoder.next_event()]
+    while not isinstance(events[-1], Epilogue):
+        events.append(decoder.next_event())
+    assert events == [
+        Preamble(data=b""),
+        Field(
+            name="text",
+            headers=Headers(
+                [
+                    ("Content-Disposition", 'form-data; name="text"'),
+                    ("Content-Type", 'text/plain; charset="UTF-8"'),
+                ]
+            ),
+        ),
+        Data(data=b"Some Text", more_data=False),
+        Field(
+            name="empty",
+            headers=Headers(
+                [
+                    ("Content-Disposition", 'form-data; name="empty"'),
+                    ("Content-Type", 'text/plain; charset="UTF-8"'),
+                ]
+            ),
+        ),
+        Data(data=b"", more_data=False),
+        Epilogue(data=b"    "),
+    ]
+    encoder = MultipartEncoder(boundary)
+    result = b""
+    for event in events:
+        result += encoder.send_event(event)
+    assert data == result