Bug 213565 – REGRESSION(r262341) Use UTF-8 to decode CFURLRefs from IPC

• Component: New Bugs
• Status: RESOLVED FIXED

[Alex Christensen]

REGRESSION(r262341) Use UTF-8 to decode CFURLRefs from IPC

[Alex Christensen]

Created attachment 402660 [details]
Patch

[Alex Christensen]

<rdar://problem/64408924>

[Yusuke Suzuki]

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.

[Darin Adler]

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?

[Alex Christensen]

Created attachment 402680 [details]
Patch

[Alex Christensen]

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.

[EWS]

Committed r263475: <https://trac.webkit.org/changeset/263475>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 402680 [details].