Repository: falconry/falcon

Issue:
Multipart form parser should not require CRLF after the closing `--`
It seems that appending `CRLF` after the closing `--`  is not strictly required by [RFC 2046](https://www.rfc-editor.org/rfc/rfc2046) if the client does not include any trailing epilogue, although it is a common convention that nearly all clients follow.

However, as witnessed by my colleague, the Node [Undici](https://undici.nodejs.org/) client, a rather new kid on the block, opts not to append it.

Discussion:
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/falconry/falcon/pull/2366?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 98.27%. Comparing base [(`06fdf28`)](https://app.codecov.io/gh/falconry/falcon/commit/06fdf2885d70149803dbc47324ae8c951b7e58b3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry) to head [(`8752ba5`)](https://app.codecov.io/gh/falconry/falcon/commit/8752ba51ad3a98eb377853c0a5983ba90a595a67?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry).

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##            master    #2366      +/-   ##
===========================================
- Coverage   100.00%   98.27%   -1.73%     
===========================================
  Files           64       64              
  Lines         7727     7725       -2     
  Branches      1073     1071       -2     
===========================================
- Hits          7727     7592     -135     
- Misses           0      115     +115     
- Partials         0       18      +18     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/falconry/falcon/pull/2366?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry).


Patch:
diff --git a/docs/changes/4.0.0.rst b/docs/changes/4.0.0.rst
index 4a54346..f706049 100644
--- a/docs/changes/4.0.0.rst
+++ b/docs/changes/4.0.0.rst
@@ -372,6 +372,13 @@ Fixed
   :class:`~falcon.media.multipart.MultipartParseOptions`. To that end, a proper
   :meth:`~falcon.media.Handlers.copy` method has been implemented for the media
   :class:`~falcon.media.Handlers` class. (`#2293 <https://github.com/falconry/falcon/issues/2293>`__)
+- Falcon's multipart form parser no longer requires a CRLF (:python:`'\\r\\n'`)
+  after the closing ``--`` delimiter. Although it is a common convention
+  (followed by the absolute majority of HTTP clients and web browsers) to
+  include a trailing CRLF, the popular Undici client
+  (used as Node's default ``fetch`` implementation) omits it at the time of
+  this writing. (The next version of Undici will adhere to the convention.)
+  (`#2364 <https://github.com/falconry/falcon/issues/2364>`__)
 
 
 Misc
diff --git a/falcon/asgi/multipart.py b/falcon/asgi/multipart.py
index 85d98e3..e4b3fdc 100644
--- a/falcon/asgi/multipart.py
+++ b/falcon/asgi/multipart.py
@@ -220,13 +220,20 @@ class MultipartForm:
                     delimiter = _CRLF + delimiter
                     prologue = False
 
-                separator = await stream.read_until(_CRLF, 2, consume_delimiter=True)
-                if separator == b'--':
-                    # NOTE(vytas): boundary delimiter + '--\r\n' signals the
-                    # end of a multipart form.
+                # NOTE(vytas): Interpretations of RFC 2046, Appendix A, vary
+                #   as to whether the closing `--` must be followed by CRLF.
+                #   While the absolute majority of HTTP clients and browsers
+                #   do append it as a common convention, it seems that this is
+                #   not mandated by the RFC, so we do not require it either.
+                # NOTE(vytas): Certain versions of the Undici client
+                #   (Node's fetch implementation) do not follow the convention.
+                if await stream.peek(2) == b'--':
+                    # NOTE(vytas): boundary delimiter + '--' signals the end of
+                    #   a multipart form.
+                    await stream.read(2)
                     break
-                elif separator:
-                    raise MultipartParseError(description='unexpected form structure')
+
+                await stream.read_until(_CRLF, 0, consume_delimiter=True)
 
             except DelimiterError as err:
                 raise MultipartParseError(
diff --git a/falcon/media/multipart.py b/falcon/media/multipart.py
index 4cf1b24..99cd878 100644
--- a/falcon/media/multipart.py
+++ b/falcon/media/multipart.py
@@ -383,13 +383,20 @@ class MultipartForm:
                     delimiter = _CRLF + delimiter
                     prologue = False
 
-                separator = stream.read_until(_CRLF, 2, consume_delimiter=True)
-                if separator == b'--':
-                    # NOTE(vytas): boundary delimiter + '--\r\n' signals the
-                    # end of a multipart form.
+                # NOTE(vytas): Interpretations of RFC 2046, Appendix A, vary
+                #   as to whether the closing `--` must be followed by CRLF.
+                #   While the absolute majority of HTTP clients and browsers
+                #   do append it as a common convention, it seems that this is
+                #   not mandated by the RFC, so we do not require it either.
+                # NOTE(vytas): Certain versions of the Undici client
+                #   (Node's fetch implementation) do not follow the convention.
+                if stream.peek(2) == b'--':
+                    # NOTE(vytas): boundary delimiter + '--' signals the end of
+                    #   a multipart form.
+                    stream.read(2)
                     break
-                elif separator:
-                    raise MultipartParseError(description='unexpected form structure')
+
+                stream.read_until(_CRLF, 0, consume_delimiter=True)
 
             except errors.DelimiterError as err:
                 raise MultipartParseError(


Test patch:
diff --git a/tests/test_media_multipart.py b/tests/test_media_multipart.py
index cd269b1..b2ad8db 100644
--- a/tests/test_media_multipart.py
+++ b/tests/test_media_multipart.py
@@ -487,6 +487,25 @@ def test_upload_multipart(client):
     ]
 
 
+@pytest.mark.parametrize('epilogue', ['', '--', '\n', '\n\n', ' <-- no CRLF', '💥'])
+def test_epilogue(client, epilogue):
+    # NOTE(vytas): According to RFC 2046, actually including an epilogue might
+    #   require a trailing CRLF first, but we do not mandate it either.
+    form_body = EXAMPLE1[:-2] + epilogue.encode()
+
+    resp = client.simulate_post(
+        '/submit',
+        headers={
+            'Content-Type': 'multipart/form-data; '
+            'boundary=5b11af82ab65407ba8cdccf37d2a9c4f',
+        },
+        body=form_body,
+    )
+
+    assert resp.status_code == 200
+    assert [part['name'] for part in resp.json] == ['hello', 'document', 'file1']
+
+
 @pytest.mark.parametrize('truncated_by', [1, 2, 3, 4])
 def test_truncated_form(client, truncated_by):
     resp = client.simulate_post(
@@ -495,7 +514,8 @@ def test_truncated_form(client, truncated_by):
             'Content-Type': 'multipart/form-data; '
             'boundary=5b11af82ab65407ba8cdccf37d2a9c4f',
         },
-        body=EXAMPLE1[:-truncated_by],
+        # NOTE(vytas): The trailing \r\n is not mandatory, hence +2.
+        body=EXAMPLE1[: -(truncated_by + 2)],
     )
 
     assert resp.status_code == 400
@@ -505,14 +525,14 @@ def test_truncated_form(client, truncated_by):
     }
 
 
-def test_unexected_form_structure(client):
+def test_unexpected_form_structure(client):
     resp1 = client.simulate_post(
         '/submit',
         headers={
             'Content-Type': 'multipart/form-data; '
             'boundary=5b11af82ab65407ba8cdccf37d2a9c4f',
         },
-        body=EXAMPLE1[:-2] + b'--\r\n',
+        body=EXAMPLE1[:-4] + b'__\r\n',
     )
 
     assert resp1.status_code == 400
@@ -577,7 +597,8 @@ def test_too_many_body_parts(custom_client, max_body_part_count):
 
 
 @pytest.mark.skipif(not msgpack, reason='msgpack not installed')
-def test_random_form(client):
+@pytest.mark.parametrize('close_delimiter', ['--', '--\r\n'])
+def test_random_form(client, close_delimiter):
     part_data = [os.urandom(random.randint(0, 2**18)) for _ in range(64)]
     form_data = (
         b''.join(
@@ -588,7 +609,7 @@ def test_random_form(client):
             + b'\r\n'
             for i in range(64)
         )
-        + '--{}--\r\n'.format(HASH_BOUNDARY).encode()
+        + '--{}{}'.format(HASH_BOUNDARY, close_delimiter).encode()
     )
 
     handler = media.MultipartFormHandler()