220780 2021-01-20 15:01:41 -0800 Null bytes aren't percent-encoded on urlencoded over POST 2022-02-04 12:51:10 -0800 1 1 1 Unclassified WebKit Forms WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 andreu webkit-unassigned achristensen andreu bfulgham cdumez smoley webkit-bug-importer wenson_hsieh oldest_to_newest 1721016 0 andreu 2021-01-20 15:01:41 -0800 The application/x-www-form-urlencoded serializer per the URL spec percent-encodes every byte in the urlencoded percent-encode set, which includes all controls. But WebKit doesn't seem to be percent-encoding the null byte on urlencoded over POST. It is percent-encoding it over GET, which I assume is because the normal url query string percent-encoding is being applied on top. This bug doesn't happen with URLSearchParams, though (https://wpt.fyi/results/url/urlsearchparams-stringifier.any.html?label=pr_head&max-count=1&pr=26740). Tests: https://wpt.fyi/results/html/semantics/forms/form-submission-0/urlencoded2.window.html?label=pr_head&max-count=1&pr=26740 (ignore the "formdata event" entries, since those are meant to test entries added to the form entry list through the formdata event, which WebKit doesn't yet implement) 1721415 1 webkit-bug-importer 2021-01-21 16:50:42 -0800 <rdar://problem/73475504> 1721477 2 achristensen 2021-01-21 20:37:55 -0800 I'm quite interested in this bug. Why do the links from wpt.fyi to run it in my browser or view the source on GitHub just result in HTTP 404 errors? 1721523 3 andreu 2021-01-22 01:50:20 -0800 (In reply to Alex Christensen from comment #2) > I'm quite interested in this bug. Why do the links from wpt.fyi to run it > in my browser or view the source on GitHub just result in HTTP 404 errors? Those tests are from a PR that hasn't been merged yet because it's primarily meant to test changes in the spec that are still being discussed (and that don't affect WebKit atm, but might in the final version), and the tests for null bytes are incidental: https://github.com/web-platform-tests/wpt/pull/26740 I thought the fact that those wpt.fyi links didn't work for PRs was a known bug, but it's not listed on the https://github.com/web-platform-tests/wpt.fyi issue list. 1721897 4 achristensen 2021-01-23 22:19:33 -0800 We're currently using FormData::appendNonMultiPartKeyValuePairItems in these tests and it appears we need to use something more like URLParser::serialize to be compatible with other browsers. 1721898 5 achristensen 2021-01-23 22:45:31 -0800 We'll need more test coverage than that PR has. Right now we use the encoding of the DOMFormData but other browsers seem to always use UTF-8. 1721910 6 andreu 2021-01-24 08:55:53 -0800 (In reply to Alex Christensen from comment #5) > We'll need more test coverage than that PR has. Right now we use the > encoding of the DOMFormData but other browsers seem to always use UTF-8. The two "characters not in encoding..." tests set the form's accept-charset to windows-1252 and pass in all browsers. If Firefox and Chrome used UTF-8, they would instead fail with "a%C9%99b=c%EF%BF%BDd" and "%C3%A1=%F0%9F%92%A9". But yeah, that PR was initially only meant to test newline normalizations on filenames in the urlencoded enctype, and since there were no other tests for urlencoded forms, I thought I'd add some corner cases as well. But in the time since we realized that that bug affected more than we initially thought. But feel free to comment on that PR to propose more test cases. 1762373 7 andreu 2021-05-21 05:21:30 -0700 The WPT PR has now been merged for over a month, and as I explained in comment #6, the issue is not with Firefox and Chrome using only UTF-8. I'm not very familiar with WebKit code, but it seems like FormData::appendNonMultiPartKeyValuePairItems ultimately defers to FormDataBuilder::appendFormURLEncoded, which aims to implement the application/x-www-form-urlencoded percent-encode set (https://url.spec.whatwg.org/#application-x-www-form-urlencoded-percent-encode-set) but apparently misses the fact that the strchr in line 93 can also match a null character. 1814853 8 444175 andreu 2021-11-14 02:55:32 -0800 Created attachment 444175 Patch 1815118 9 ews-feeder 2021-11-15 10:36:51 -0800 Committed r285810 (244254@main): <https://commits.webkit.org/244254@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 444175. 1815132 10 achristensen 2021-11-15 11:11:34 -0800 This was certainly a small step in the right direction, but I feel like it was probably incomplete. We probably still need logic more like serializeURLEncodedForm 1837564 11 bfulgham 2022-02-04 12:51:10 -0800 This change should be present in STP 139, iOS 15.4 Beta, and macOS 12.3 Beta. 444175 2021-11-14 02:55:32 -0800 2021-11-15 10:36:52 -0800 Patch bug-220780-20211114115531.patch text/plain 4859 andreu U3VidmVyc2lvbiBSZXZpc2lvbjogMjg1Nzg4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMWUzOWFmZjM3NDQ4ODM4 OTBmMDUxMDhiMTEwNWUyZjJmYTk1NWQzNi4uYWQyODVlMWJjZTk2M2E0MjRjZWJiYWJhMWNiMzFm M2IxNGRiNTg4OCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDIxLTExLTE0ICBBbmRy ZXUgQm90ZWxsYSAgPGFuZHJldUBhbmRyZXVib3RlbGxhLmNvbT4KKworICAgICAgICBOdWxsIGJ5 dGVzIGFyZW4ndCBwZXJjZW50LWVuY29kZWQgb24gdXJsZW5jb2RlZCBvdmVyIFBPU1QKKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIyMDc4MAorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoaXMgY2hhbmdlIGZp eGVzIGEgYnVnIHdoZXJlIHN0cmNociB3YXMgYmVpbmcgdXNlZCB0byBtYXRjaCBhIGNoYXJhY3Rl ciBhZ2FpbnN0IGEgc2V0CisgICAgICAgIGNoYXJhY3RlciBsaXN0LCB3aXRob3V0IGNoZWNraW5n IHdoZXRoZXIgdGhlIGNoYXJhY3RlciB3YXMgTlVMLiBUaGlzIHJlc3VsdGVkIGluIE5VTCBieXRl cworICAgICAgICBiZWluZyBwYXNzZWQgdGhyb3VnaCBpbiB0aGUgdXJsZW5jb2RlZCBlbmN0eXBl IG92ZXIgUE9TVCwgcmF0aGVyIHRoYW4gYmVpbmcgcGVyY2VudC1lc2NhcGVkLgorCisgICAgICAg IFRlc3RzOiBpbXBvcnRlZC93M2Mvd2ViLXBsYXRmb3JtLXRlc3RzL2h0bWwvc2VtYW50aWNzL2Zv cm1zL2Zvcm0tc3VibWlzc2lvbi0wL3VybGVuY29kZWQyLndpbmRvdy5odG1sCisKKyAgICAgICAg KiBwbGF0Zm9ybS9uZXR3b3JrL0Zvcm1EYXRhQnVpbGRlci5jcHA6CisgICAgICAgIChXZWJDb3Jl OjpGb3JtRGF0YUJ1aWxkZXI6OmFwcGVuZEZvcm1VUkxFbmNvZGVkKToKKwogMjAyMS0xMS0xMyAg U2ltb24gRnJhc2VyICA8c2ltb24uZnJhc2VyQGFwcGxlLmNvbT4KIAogICAgICAgICBSdW4gYSBT Y3JvbGxBbmltYXRpb25Nb21lbnR1bSBmb3IgdGhlIG1vbWVudHVtIHBoYXNlIG9mIGEgc2Nyb2xs CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL0Zvcm1EYXRhQnVp bGRlci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL0Zvcm1EYXRhQnVpbGRl ci5jcHAKaW5kZXggZGI3NTcxNjBmY2JkYmZhYWVhMGRhZTBhZTE2MmY0MDM4MTNlMTA1YS4uY2M2 YTAyMmNjNTk0ODJmMjc5Y2MwMTYxNDZhN2E3ZDU1MDFkMzc5OCAxMDA2NDQKLS0tIGEvU291cmNl L1dlYkNvcmUvcGxhdGZvcm0vbmV0d29yay9Gb3JtRGF0YUJ1aWxkZXIuY3BwCisrKyBiL1NvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL25ldHdvcmsvRm9ybURhdGFCdWlsZGVyLmNwcApAQCAtOTAsNyAr OTAsOCBAQCBzdGF0aWMgdm9pZCBhcHBlbmRGb3JtVVJMRW5jb2RlZChWZWN0b3I8Y2hhcj4mIGJ1 ZmZlciwgY29uc3QgdWludDhfdCogc3RyaW5nLCBzaQogICAgIHN0YXRpYyBjb25zdCBjaGFyIHNh ZmVDaGFyYWN0ZXJzW10gPSAiLS5fKiI7CiAgICAgZm9yIChzaXplX3QgaSA9IDA7IGkgPCBsZW5n dGg7ICsraSkgewogICAgICAgICBhdXRvIGNoYXJhY3RlciA9IHN0cmluZ1tpXTsKLSAgICAgICAg aWYgKGlzQVNDSUlBbHBoYW51bWVyaWMoY2hhcmFjdGVyKSB8fCBzdHJjaHIoc2FmZUNoYXJhY3Rl cnMsIGNoYXJhY3RlcikpCisgICAgICAgIGlmIChpc0FTQ0lJQWxwaGFudW1lcmljKGNoYXJhY3Rl cikKKyAgICAgICAgICAgIHx8IChjaGFyYWN0ZXIgIT0gJ1wwJyAmJiBzdHJjaHIoc2FmZUNoYXJh Y3RlcnMsIGNoYXJhY3RlcikpKQogICAgICAgICAgICAgYXBwZW5kKGJ1ZmZlciwgY2hhcmFjdGVy KTsKICAgICAgICAgZWxzZSBpZiAoY2hhcmFjdGVyID09ICcgJykKICAgICAgICAgICAgIGFwcGVu ZChidWZmZXIsICcrJyk7CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9pbXBvcnRlZC93M2MvQ2hh bmdlTG9nIGIvTGF5b3V0VGVzdHMvaW1wb3J0ZWQvdzNjL0NoYW5nZUxvZwppbmRleCBiMWRkMWQ1 YjI0ZTRlNTg1NmVhOWE0ZjY2NjM3ZWMxY2E1NmEzMzNhLi5hMTc5YTYxOWY0NTZkMDhhNDdmMmE5 ZmI5Nzg2MTcyYmRiNDkzY2EzIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9pbXBvcnRlZC93M2Mv Q2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3RzL2ltcG9ydGVkL3czYy9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxMiBAQAorMjAyMS0xMS0xNCAgQW5kcmV1IEJvdGVsbGEgIDxhbmRyZXVAYW5kcmV1Ym90 ZWxsYS5jb20+CisKKyAgICAgICAgTnVsbCBieXRlcyBhcmVuJ3QgcGVyY2VudC1lbmNvZGVkIG9u IHVybGVuY29kZWQgb3ZlciBQT1NUCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0yMjA3ODAKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICAqIHdlYi1wbGF0Zm9ybS10ZXN0cy9odG1sL3NlbWFudGljcy9mb3Jtcy9m b3JtLXN1Ym1pc3Npb24tMC91cmxlbmNvZGVkMi53aW5kb3ctZXhwZWN0ZWQudHh0OgorCiAyMDIx LTExLTEyICBSeWFuIEhhZGRhZCAgPHJ5YW5oYWRkYWRAYXBwbGUuY29tPgogCiAgICAgICAgIFVu cmV2aWV3ZWQsIHJldmVydGluZyByMjg1NTgzLgpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvaW1w b3J0ZWQvdzNjL3dlYi1wbGF0Zm9ybS10ZXN0cy9odG1sL3NlbWFudGljcy9mb3Jtcy9mb3JtLXN1 Ym1pc3Npb24tMC91cmxlbmNvZGVkMi53aW5kb3ctZXhwZWN0ZWQudHh0IGIvTGF5b3V0VGVzdHMv aW1wb3J0ZWQvdzNjL3dlYi1wbGF0Zm9ybS10ZXN0cy9odG1sL3NlbWFudGljcy9mb3Jtcy9mb3Jt LXN1Ym1pc3Npb24tMC91cmxlbmNvZGVkMi53aW5kb3ctZXhwZWN0ZWQudHh0CmluZGV4IGQxN2Zl ZTZhNjljYTZiOWZiZTFjNDA1MGQ4ZTU0MzBjM2VjNjJjOWUuLmY2YTk3Yzk0OGFlNzAwZjExMzYx MTlmODIwZmYxOGMyYWI5NGJjNzcgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3RzL2ltcG9ydGVkL3cz Yy93ZWItcGxhdGZvcm0tdGVzdHMvaHRtbC9zZW1hbnRpY3MvZm9ybXMvZm9ybS1zdWJtaXNzaW9u LTAvdXJsZW5jb2RlZDIud2luZG93LWV4cGVjdGVkLnR4dAorKysgYi9MYXlvdXRUZXN0cy9pbXBv cnRlZC93M2Mvd2ViLXBsYXRmb3JtLXRlc3RzL2h0bWwvc2VtYW50aWNzL2Zvcm1zL2Zvcm0tc3Vi bWlzc2lvbi0wL3VybGVuY29kZWQyLndpbmRvdy1leHBlY3RlZC50eHQKQEAgLTMsMTEgKzMsMTEg QEAgUEFTUyBhcHBsaWNhdGlvbi94LXd3dy1mb3JtLXVybGVuY29kZWQ6IEJhc2ljIHRlc3QgKG5v cm1hbCBmb3JtKQogUEFTUyBhcHBsaWNhdGlvbi94LXd3dy1mb3JtLXVybGVuY29kZWQ6IEJhc2lj IHRlc3QgKGZvcm1kYXRhIGV2ZW50KQogUEFTUyBhcHBsaWNhdGlvbi94LXd3dy1mb3JtLXVybGVu Y29kZWQ6IEJhc2ljIEZpbGUgdGVzdCAobm9ybWFsIGZvcm0pCiBGQUlMIGFwcGxpY2F0aW9uL3gt d3d3LWZvcm0tdXJsZW5jb2RlZDogQmFzaWMgRmlsZSB0ZXN0IChmb3JtZGF0YSBldmVudCkgYXNz ZXJ0X2VxdWFsczogZXhwZWN0ZWQgImJhc2ljPWZpbGUtdGVzdC50eHQiIGJ1dCBnb3QgIiIKLUZB SUwgYXBwbGljYXRpb24veC13d3ctZm9ybS11cmxlbmNvZGVkOiAweDAwIGluIG5hbWUgKG5vcm1h bCBmb3JtKSBhc3NlcnRfZXF1YWxzOiBleHBlY3RlZCAiYSUwMGI9YyIgYnV0IGdvdCAiYVwwYj1j IgotRkFJTCBhcHBsaWNhdGlvbi94LXd3dy1mb3JtLXVybGVuY29kZWQ6IDB4MDAgaW4gbmFtZSAo Zm9ybWRhdGEgZXZlbnQpIGFzc2VydF9lcXVhbHM6IGV4cGVjdGVkICJhJTAwYj1jIiBidXQgZ290 ICJhXDBiPWMiCi1GQUlMIGFwcGxpY2F0aW9uL3gtd3d3LWZvcm0tdXJsZW5jb2RlZDogMHgwMCBp biB2YWx1ZSAobm9ybWFsIGZvcm0pIGFzc2VydF9lcXVhbHM6IGV4cGVjdGVkICJhPWIlMDBjIiBi dXQgZ290ICJhPWJcMGMiCi1GQUlMIGFwcGxpY2F0aW9uL3gtd3d3LWZvcm0tdXJsZW5jb2RlZDog MHgwMCBpbiB2YWx1ZSAoZm9ybWRhdGEgZXZlbnQpIGFzc2VydF9lcXVhbHM6IGV4cGVjdGVkICJh PWIlMDBjIiBidXQgZ290ICJhPWJcMGMiCi1GQUlMIGFwcGxpY2F0aW9uL3gtd3d3LWZvcm0tdXJs ZW5jb2RlZDogMHgwMCBpbiBmaWxlbmFtZSAobm9ybWFsIGZvcm0pIGFzc2VydF9lcXVhbHM6IGV4 cGVjdGVkICJhPWIlMDBjIiBidXQgZ290ICJhPWJcMGMiCitQQVNTIGFwcGxpY2F0aW9uL3gtd3d3 LWZvcm0tdXJsZW5jb2RlZDogMHgwMCBpbiBuYW1lIChub3JtYWwgZm9ybSkKK1BBU1MgYXBwbGlj YXRpb24veC13d3ctZm9ybS11cmxlbmNvZGVkOiAweDAwIGluIG5hbWUgKGZvcm1kYXRhIGV2ZW50 KQorUEFTUyBhcHBsaWNhdGlvbi94LXd3dy1mb3JtLXVybGVuY29kZWQ6IDB4MDAgaW4gdmFsdWUg KG5vcm1hbCBmb3JtKQorUEFTUyBhcHBsaWNhdGlvbi94LXd3dy1mb3JtLXVybGVuY29kZWQ6IDB4 MDAgaW4gdmFsdWUgKGZvcm1kYXRhIGV2ZW50KQorUEFTUyBhcHBsaWNhdGlvbi94LXd3dy1mb3Jt LXVybGVuY29kZWQ6IDB4MDAgaW4gZmlsZW5hbWUgKG5vcm1hbCBmb3JtKQogRkFJTCBhcHBsaWNh dGlvbi94LXd3dy1mb3JtLXVybGVuY29kZWQ6IDB4MDAgaW4gZmlsZW5hbWUgKGZvcm1kYXRhIGV2 ZW50KSBhc3NlcnRfZXF1YWxzOiBleHBlY3RlZCAiYT1iJTAwYyIgYnV0IGdvdCAiIgogUEFTUyBh cHBsaWNhdGlvbi94LXd3dy1mb3JtLXVybGVuY29kZWQ6IFxuIGluIG5hbWUgKG5vcm1hbCBmb3Jt KQogUEFTUyBhcHBsaWNhdGlvbi94LXd3dy1mb3JtLXVybGVuY29kZWQ6IFxuIGluIG5hbWUgKGZv cm1kYXRhIGV2ZW50KQo=