remix-run / remix

Build Better Websites. Create modern, resilient user experiences with web fundamentals.
https://remix.run
MIT License
28.41k stars 2.39k forks source link

Uploads don't handle quoted multipart boundaries #4793

Open colinramsay opened 1 year ago

colinramsay commented 1 year ago

What version of Remix are you using?

1.8

Steps to Reproduce

Send a multipart request with a quoted boundary in the content-type header:

Content-Type: multipart/form-data; boundary="2e798d44-9b0d-4284-a9d0-faaf252f82b3"

Expected Behavior

The uploaded file(s) should be detected.

Actual Behavior

Files are ignored, because web3-storage/multipart-parser expects a boundary without quotes. The following patch fixes the issue but unfortunately I can't see how to easily add a test for this.

diff --git a/node_modules/@remix-run/server-runtime/dist/formData.js b/node_modules/@remix-run/server-runtime/dist/formData.js
index f7a700e..0e756b9 100644
--- a/node_modules/@remix-run/server-runtime/dist/formData.js
+++ b/node_modules/@remix-run/server-runtime/dist/formData.js
@@ -38,6 +38,11 @@ async function parseMultipartFormData(request, uploadHandler) {
   let contentType = request.headers.get("Content-Type") || "";
   let [type, boundary] = contentType.split(/\s*;\s*boundary=/);

+  // The spec allows a boundary to be surrounded by quotes,
+  // but `multipart-parser` needs a boundary without them.
+  // https://datatracker.ietf.org/doc/html/rfc7578#section-4-1
+  boundary = boundary.replaceAll('"', '')
+
   if (!request.body || !boundary || type !== "multipart/form-data") {
     throw new TypeError("Could not parse content as FormData.");
   }
machour commented 1 year ago

This is another case of #4443 with a different character, thank you for reporting it.

colinramsay commented 1 year ago

As far as I'm aware this is still an issue.

abustany commented 1 week ago

https://github.com/web3-storage/multipart-parser has meanwhile been archived :grimacing:

colinramsay commented 1 week ago

Yeah it's crazy to me that the whole upload segment of Remix is still marked as unstable and relies on this broken and (now) archived package.

kiliman commented 1 week ago

You may want to consider just using formidable. It looks popular and well-maintained.

https://github.com/node-formidable/formidable

kiliman commented 1 week ago

There is really nothing Remix-specific about handling multipart form data. You can use any third-party library to process the standard Request object.

colinramsay commented 1 week ago

I appreciate that, but it doesn't negate the fact that the current docs reference unstable_parseMultipartFormData which in turn uses multipart-parser. I'd be happy for Remix to just remove its own file upload stuff but the docs need to reflect that and probably make some recommendations.