jimmywarting / FormData

HTML5 `FormData` polyfill for Browsers and nodejs
MIT License
360 stars 102 forks source link

encode headers more correctly #120

Closed jimmywarting closed 3 years ago

jimmywarting commented 3 years ago

@andreubotella I hope you could give me some feedback on how to improve my formdata-to-blob transformer

https://github.com/jimmywarting/FormData/blob/1c6fa5244a0d9111c4276a4b08ffeaca34dd25ed/formdata-to-blob.js#L8-L28

I know i'm doing something wrong with encoding the headers (not escaping the filename & field names correctly) and i want to be more spec compliant (without the need of using ReadableStreams)

It's very basic, fast and synchronous as well, I'm not sure what benefit it would have over a ReadableStream since you can just as easily get a stream from blob.stream() later I have use this method to polyfill formdata for a very long time by converting it to a blob and sending it instead (dating back to the year of IE9 that didn't have streams or any good way of sending binary without the support for blob) it have worked for many other ppl over the years that have used my polyfill

I see it as more beneficial then having a ReadableStream encoder, you get to know what the total size is, what type (boundary) it has, works in more older browser. It's synchronous, don't block much, don't have to read the content of a file or blob - could you enlighten me why a ReadableStream would be better?

it's not like if we can append a RedableStream into a field and do fd.append('file', new ReadableStream)? and upload a form-data without a known content-length?

Converting a FormData to Blob where you append a large file shouldn't increase the memory much since it should just only be a references point in another blob

andreubotella commented 3 years ago

Hi. If the Blob implementation in question stores parts to be resolved later, I'm not sure there's any difference in performance between using Blob or a ReadableStream. The only reason I'm creating a readable stream in https://andreubotella.github.io/multipart-form-data is because this is meant to be included somewhere in the WHATWG specs, and the fetch spec will need a stream. Also, the File API spec still doesn't define very well how Blobs turn into streams.

But you could use the multipart/form-data chunk serializer algorithm directly, by pushing the chunks into parts.

The main bug in your code that I see is that you include `\r\n--${boundary}--` at the end of each entry. `--${boundary}--` is the closing boundary delimiter, it should only be added to parts after you're done iterating through the entries (step 5 in the chunk serializer algorithm). At the end of each entry you should only have `\r\n` (steps 4.4.4 and 4.5.6).

Also, rather than just including name and value.name into the string, you should escape them with the escape a multipart-form-data name algorithm. You should also CRLF-normalize value before you include it.

jimmywarting commented 3 years ago

Thank you for your input