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

Editorial: Have the multipart/form-data encoding algorithm use "encode" #6141

Merged
merged 2 commits into from
Nov 11, 2020

Conversation

andreubotella
Copy link
Member

@andreubotella andreubotella commented Nov 10, 2020

After whatwg/url#558, the form submission algorithm is the only place in the web platform that uses "encode". However, although for application/x-www-form-urlencoded and text/plain "encode" is clearly linked, multipart/form-data instead describes an equivalent algorithm. This change fixes that.


/form-control-infrastructure.html ( diff )

After whatwg/url#558, the form submission algorithm is the only place in
the web platform that uses "encode". However, although for
application/x-www-form-urlencoded and text/plain "encode" is clearly
linked, multipart/form-data instead describes an equivalent algorithm.
This change fixes that.
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this seems like a big improvement. I have a couple of minor nits.

source Show resolved Hide resolved
source Show resolved Hide resolved
<var>encoding</var>, converted to a byte sequence. In the case of file names, however, the
precise name may be approximated if necessary (e.g. newlines could be removed from file names,
quotes could be changed to "%22", and characters not expressible in <var>encoding</var> could
be replaced by other characters before encoding).</p></li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess as a follow-up we should test if this is what user agents do.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thoughts exactly.

source Show resolved Hide resolved
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again, will let @domenic do the final check.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, although it took me a long time to understand and trace through the specs to learn that "encode" (which I would have thought to be a generic thing similar to TextEncoder's encode()) actually does the HTML-entity thing.

@annevk annevk merged commit ea2b4ab into whatwg:master Nov 11, 2020
@andreubotella andreubotella deleted the multipart branch November 11, 2020 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants