| Summary: | REGRESSION(r262341) Use UTF-8 to decode CFURLRefs from IPC | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alex Christensen <achristensen> | ||||||
| Component: | New Bugs | Assignee: | Alex Christensen <achristensen> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | darin, mark.lam, webkit-bug-importer, ysuzuki | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Alex Christensen
2020-06-24 10:03:24 PDT
Created attachment 402660 [details]
Patch
Comment on attachment 402660 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402660&action=review Cool! > LayoutTests/fast/url/navigate-non-ascii.html:2 > +location.assign('//\u2000'); Can we have a test in TestWTF too? The reason is that it is possible that we add URL validation check in `location.assign` since it is specified in the spec. https://html.spec.whatwg.org/multipage/history.html#dom-location-assign "Parse url relative to the entry settings object. If that failed, throw a "SyntaxError" DOMException." If it is added, this test becomes one which does not stress ArgumentCodersCF. Comment on attachment 402660 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=402660&action=review r=me but please do improve the test coverage >> LayoutTests/fast/url/navigate-non-ascii.html:2 >> +location.assign('//\u2000'); > > Can we have a test in TestWTF too? The reason is that it is possible that we add URL validation check in `location.assign` since it is specified in the spec. > https://html.spec.whatwg.org/multipage/history.html#dom-location-assign > "Parse url relative to the entry settings object. If that failed, throw a "SyntaxError" DOMException." > > If it is added, this test becomes one which does not stress ArgumentCodersCF. I agree that we need a more durable test that will exercise the code path even if we fix location assignment. Not as sure as Yusuke that a test written in C++ in TestWTF is the best option. Are there other ways to exercise this? Created attachment 402680 [details]
Patch
Comment on attachment 402680 [details]
Patch
I added an API test that would terminate the web process before this change, and fails to load an invalid url after this change.
Committed r263475: <https://trac.webkit.org/changeset/263475> All reviewed patches have been landed. Closing bug and clearing flags on attachment 402680 [details]. |