remix icon indicating copy to clipboard operation
remix copied to clipboard

Uploads don't handle quoted multipart boundaries

Open colinramsay opened this issue 3 years ago • 7 comments

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.");
   }

colinramsay avatar Dec 07 '22 09:12 colinramsay

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

machour avatar Dec 07 '22 10:12 machour

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

colinramsay avatar Apr 30 '23 16:04 colinramsay

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

abustany avatar Jun 26 '24 12:06 abustany

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.

colinramsay avatar Jun 26 '24 12:06 colinramsay

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

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

kiliman avatar Jun 26 '24 14:06 kiliman

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

kiliman avatar Jun 26 '24 14:06 kiliman

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.

colinramsay avatar Jun 26 '24 14:06 colinramsay

Thank you for opening this issue, and our apologies we haven't gotten around to it yet!

With the release of React Router v7 we are sun-setting continued development/maintenance on Remix v2. If you have not already upgraded to React Router v7, we recommend you do so. We've tried to make the upgrade process as smooth as possible with our Future Flags. We are now in the process of cleaning up outdated issues and pull requests to improve the overall hygiene of our repositories.

We plan to continue to address 2 types of issues in Remix v2:

  • Bugs that pose security concerns
  • Bugs that prevent upgrading to React Router v7

If you believe this issue meets one of those criteria, please respond or create a new issue.

For all other issues, ongoing maintenance will be happening in React Router v7, so please reopen this issue in that repo with a new minimal reproduction against v7.

If you have any questions you can always reach out on Discord. Thanks again for providing feedback and helping us make our framework even better!

brookslybrand avatar Jan 22 '25 16:01 brookslybrand