Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support multipart/form-data encoding #1810

Closed
1 task done
octet-stream opened this issue Jul 27, 2021 · 14 comments
Closed
1 task done

Support multipart/form-data encoding #1810

octet-stream opened this issue Jul 27, 2021 · 14 comments

Comments

@octet-stream
Copy link
Contributor

octet-stream commented Jul 27, 2021

What problem are you trying to solve?

General support for spec-compatible FormData implementations.

Describe the feature

Current form-data encoding in Got relies on form-data package implementation which is not spec-compatible and lacks of most of FormData interface methods. I suggest to add general support for this type of request body. How this can be done:

  • Detect spec-compatible FormData objects in request body: this will allow to use either formdata-node, or formdata-polyfill, or any other implementation as long as it follows the spec;
  • Implement in-house form-data encoder or just use form-data-encoder package instead;

References:

If you don't mind to use form-data-encoder to handle this encoding, I can file a PR to bring its support.

Checklist

  • I have read the documentation and made sure this feature doesn't already exist.
@szmarczak
Copy link
Collaborator

Got v12 supports async iterators so https://www.npmjs.com/package/formdata-node should work out of the box.

@szmarczak
Copy link
Collaborator

https://github.com/jimmywarting/FormData is a no as of now - Got doesn't support Blobs

@octet-stream
Copy link
Contributor Author

Got v12 supports async iterators so npmjs.com/package/formdata-node should work out of the box.

Builtin encoding will be removed from formdata-node in next major release, because it must be handled by HTTP client. This suggestion is not about support for some specific package, but rather for multipart/form-data encoding in general.

@szmarczak
Copy link
Collaborator

szmarczak commented Jul 27, 2021

Please consider removing nanoid. It weighs 52KB. Math.random() should be enough.

@octet-stream
Copy link
Contributor Author

Yeah, I will do something about this.

@octet-stream
Copy link
Contributor Author

octet-stream commented Jul 27, 2021

Decided to just use crypto.randomBytes for boundary string, should be more than enough. Kind of don't want to use just Math.random() for this, but it would've be fine too.

Available in form-data-encoder 1.1.0, now it weights 35.8 KB. I will reduce the size once again when I drop CJS support.

@szmarczak

This comment has been minimized.

@octet-stream
Copy link
Contributor Author

octet-stream commented Aug 1, 2021

What's wrong with Math.random()?

It seem to be fine as well, found few other implementations which are using Math.random() for boundaries (one of them is part of Deno).

form-data-encoder integration seem to work fine, the tests are passing and I can file a PR for further discussion if you want.

@szmarczak
Copy link
Collaborator

crypto is fine I guess :) I forgot Node.js has a dedicated API for generating random numbers.

@jimmywarting
Copy link

Firefox only use numbers for it's boundary. They don't have to be super random. Math.random is fine and more cross Deno/Browser/nodejs friendlier as it dose not depend on any core nodejs stuff

@octet-stream
Copy link
Contributor Author

octet-stream commented Aug 5, 2021

Yes, I already switched to Math.random()

@tutods
Copy link

tutods commented Oct 15, 2022

Is it possible to use the got with multipart/formdata?

@robbash
Copy link

robbash commented Jun 7, 2023

Is it possible to use the got with multipart/formdata?

Yes. It took me quite a while to find how to upload a file using formdata, so leaving the link here in hope it helps others in the future: #1877 (reply in thread)

@octet-stream
Copy link
Contributor Author

Starting with Got v12, multipart/form-data supported natively. Just use any spec-compliant implementation (including Node.js' own implementation). This PR brought this feature: #1835

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants