Add test cases for newline handling in form payloads #26740
Conversation
wpt-pr-bot
requested review from
annevk,
domenic,
GPHemsley,
mikewest and
TimothyGu
There was a problem hiding this comment.
I initially thought it was okay, but this is also testing form submission and that currently requires newline normalization for file names that the expected results do not seem to reflect.
And since this is really about form submission perhaps this should be placed in the html/ top-level directory, though I'm not entirely sure as there is no other way to test the encoding aspect of urlencoding... Either way we should probably link from a README on the other side, wherever we decide to put these.
wpt-pr-bot
requested review from
foolip,
jdm,
jgraham,
tkent-google and
zqzhang
|
Although this matches implementations per whatwg/url#562 (comment), I want to wait with landing these tests until the specifications back them up. Or am I misunderstanding something still? |
|
I wrote these tests are written to follow the specs, without the newline normalization, so that all browsers fail. Since we now have settled on a way to fix the spec to match the implementations, I'll update the tests to match. |
|
Oh right, so much misunderstanding around these newlines. 😊 Are you also planning to test |
|
Should I add the tests for |
|
Sounds great! |
wpt-pr-bot
requested review from
inexorabletash and
mkruisselbrink
and removed request for
tkent-google
|
I think this is now ready for review. For any reviewers: note that the spec PR corresponding to these tests is whatwg/html#6287, and see also whatwg/html#6247 for some context. |
andreubotella
requested review from
annevk
and removed request for
TakayoshiKochi
wpt-pr-bot
requested review from
TakayoshiKochi
and removed request for
zqzhang
On `multipart/form-data` payloads, a null byte on the name, filename or string value cuts off the rest of the name, filename or value. On `text/plain` payloads, a null byte anywhere cuts off the rest of the entire payload. This is because `nsLinebreakConverter::ConvertLineBreaks` is called without giving a length parameter, which causes it to treat the input C string as null-terminated. The tests for `text/plain` are under review on WPT: web-platform-tests/wpt#26740 (https://wpt.fyi/results/html/semantics/forms/form-submission-0/text-plain.window.html?label=pr_head&max-count=1&pr=26740) Differential Revision: https://phabricator.services.mozilla.com/D102625
On `multipart/form-data` payloads, a null byte on the name, filename or string value cuts off the rest of the name, filename or value. On `text/plain` payloads, a null byte anywhere cuts off the rest of the entire payload. This is because `nsLinebreakConverter::ConvertLineBreaks` is called without giving a length parameter, which causes it to treat the input C string as null-terminated. The tests for `text/plain` are under review on WPT: web-platform-tests/wpt#26740 (https://wpt.fyi/results/html/semantics/forms/form-submission-0/text-plain.window.html?label=pr_head&max-count=1&pr=26740) Differential Revision: https://phabricator.services.mozilla.com/D102625 UltraBlame original commit: 6c7f4b2ece67c32ae549c2d53efaf10e188ae182
On `multipart/form-data` payloads, a null byte on the name, filename or string value cuts off the rest of the name, filename or value. On `text/plain` payloads, a null byte anywhere cuts off the rest of the entire payload. This is because `nsLinebreakConverter::ConvertLineBreaks` is called without giving a length parameter, which causes it to treat the input C string as null-terminated. The tests for `text/plain` are under review on WPT: web-platform-tests/wpt#26740 (https://wpt.fyi/results/html/semantics/forms/form-submission-0/text-plain.window.html?label=pr_head&max-count=1&pr=26740) Differential Revision: https://phabricator.services.mozilla.com/D102625
|
Since it seems like implementations agree on the spec change, we should work towards getting this merged. Since this PR touched many files all over the place, there are many suggested reviewers, but in addition to @annevk, I'd like to get @tkent-google's approval, since in #26838 I changed the tests for form-associated custom elements to match the spec, not yet realizing that both Chrome and Firefox normalized strings added in the |
|
@tkent-google Ping |
html/semantics/forms/form-submission-0/multipart-formdata.window.js
Outdated
Show resolved
Hide resolved
User agents normalize newlines when serializing form data to text/plain, application/x-www-form-urlencoded, and multipart/form-data. (This can be observed through FormData or the formdata event.) This additionally changes the input passed to the application/x-www-form-urlencoded and text/plain serializers to be a list of name-value pairs, where the values are always strings rather than potentially File objects. Tests: web-platform-tests/wpt#26740. Follow-up: #6624 & #6697. Closes #6247. Helps with whatwg/url#562.
(The title of this PR used to be "Add test cases for urlencoded serialization of filenames", dealing in particular with newline normalizations in the urlencoded serialization. However, the scope of the spec issues whatwg/url#562 and whatwg/html#6247 grew in scope as similar cases were found in other form enctypes.)