Variant tests with parens and pipes in URL fail to run on Safari

[foolip]

https://wpt.fyi/results/html/dom?run_id=264030002&run_id=248260003&run_id=262080001&run_id=234440001 has the four variants of Safari on macOS that we currently run: stable + preview on Azure Pipelines and Buildbot.

All of them fail /html/dom/interfaces.https.html?exclude=(Document|Window|HTML.*) and /html/dom/interfaces.https.html?include=(Document|Window) like this:

ERROR message: Could not parse requested URL 'https://web-platform.test:8443/html/dom/interfaces.https.html?include=(Document|Window)'
Traceback (most recent call last):
  File "/Users/kazooie/worker/macOS_Chunked_Runner/build/tools/wptrunner/wptrunner/executors/executorwebdriver.py", line 282, in _run
    self.result = True, self.func(self.protocol, self.url, self.timeout)
  File "/Users/kazooie/worker/macOS_Chunked_Runner/build/tools/wptrunner/wptrunner/executors/executorwebdriver.py", line 351, in do_testharness
    protocol.webdriver.url = url
  File "/Users/kazooie/worker/macOS_Chunked_Runner/build/tools/webdriver/webdriver/client.py", line 20, in inner
    return func(self, *args, **kwargs)
  File "/Users/kazooie/worker/macOS_Chunked_Runner/build/tools/webdriver/webdriver/client.py", line 521, in url
    return self.send_session_command("POST", "url", body)
  File "/Users/kazooie/worker/macOS_Chunked_Runner/build/tools/webdriver/webdriver/client.py", line 508, in send_session_command
    return self.send_command(method, url, body)
  File "/Users/kazooie/worker/macOS_Chunked_Runner/build/tools/webdriver/webdriver/client.py", line 472, in send_command
    raise err
InvalidArgumentException: invalid argument (400): Could not parse requested URL 'https://web-platform.test:8443/html/dom/interfaces.https.html?include=(Document|Window)'

Python on macOS doesn't like https://web-platform.test:8443/html/dom/interfaces.https.html?include=(Document|Window), and I suspect it's because of the parenthesis or pipe characters.

@zcorpan do you know where this pipe character is put into the manifest? I'd like to try escaping it as %7C to see if that fixes the problem.

[gsnedders]

That's not Python on macOS not liking that, it's the WebDriver server (in this case in SafariDriver) responding with 400 Bad Request. @burg? I presume it's something like using a URL parser that treats the pipe character as invalid (which it is per RFC3986, for example).

[zcorpan]

From what I understand of https://url.spec.whatwg.org/#query-state the pipe character is a validation error but does not return failure (i.e., it should parse successfully).

So I think there are multiple ways to address this problem, not mutually exclusive:

• Fix SafariDriver's URL parser
• URL-encode the URL so that invalid characters are URL-escaped before entering the manifest
• Change tests to have valid URL strings in <meta name=variant content=...>
• Previous point + lint checker

[gsnedders]

If I'm reading the URL spec correctly, there's no valid URL equivalent to https://web-platform.test:8443/html/dom/interfaces.https.html?include=(Document|Window) (escaping it doesn't provide an equivalent URL, which is part of whatwg/url#369).

[foolip]

@gsnedders in this instance does it matter if the URL is considered equivalent, as long as the code that parses include=(Document|Window) (where?) handles the URL correctly?

Regardless, if the bug is in SafariDriver and it only affects a few tests, leaving this blocked on a SafariDriver fix might be reasonable. @burg can you file a bug, or should one be filed through https://feedbackassistant.apple.com/?

[burg]

Please file via the feedback assistant. I’m on parent leave until August.

…

Sent from my Apple IIc

On Jun 4, 2019, at 06:55, Philip Jägenstedt ***@***.***> wrote: @gsnedders in this instance does it matter if the URL is considered equivalent, as long as the code that parses include=(Document|Window) (where?) handles the URL correctly? Regardless, if the bug is in SafariDriver and it only affects a few tests, leaving this blocked on a SafariDriver fix might be reasonable. @burg can you file a bug, or should one be filed through https://feedbackassistant.apple.com/? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[foolip]

https://wpt.fyi/results/html/dom?run_id=352740001&run_id=376000001 shows that this works in WebKitGTK but not Safari.

@clopez could you look into this and file an issue for safaridriver, or for WebKit if you can spot the problem there?

[gsnedders]

IIRC, @burg filed a Radar issue for this.

[burg]

I have filed an issue and haven't looked into it yet.

…

On Nov 19, 2019, at 8:51 AM, Sam Sneddon ***@***.***> wrote: IIRC, @burg <https://github.com/burg> filed a Radar issue for this. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17027?email_source=notifications&email_token=AAARIGFQZ5DIIXOV3HN7CATQUQKQLA5CNFSM4HP2HECKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEO4TII#issuecomment-555600289>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAARIGDI7XDIWRTUGUB52ELQUQKQLANCNFSM4HP2HECA>.

[foolip]

Alright, great! Is there a public ID for it in case I want to reference it?

[burg]

It's tracked internally as rdar://problem/55240821 <rdar://problem/55240821>

…

On Nov 19, 2019, at 11:38 AM, Philip Jägenstedt ***@***.***> wrote: Alright, great! Is there a public ID for it in case I want to reference it? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17027?email_source=notifications&email_token=AAARIGG7ZTEW3JPZDJVBAHDQUQ6CDA5CNFSM4HP2HECKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEPOZYI#issuecomment-555674849>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAARIGFLAZ2VUY5YPXM3UOLQUQ6CDANCNFSM4HP2HECA>.

[burg]

Sorry about the crazy wait on this one. If all goes well, the fix should be out with Safari Technology Preview 106.

[foolip]

Sweet, thanks for fixing this, Brian!

[foolip]

It looks like this was already fixed in STP 105:
https://wpt.fyi/results/html/dom?run_id=510170002&run_id=503710001

Yay!

[burg]

Oops, yes this was recently fixed. Brian (he/him)

…

On Apr 30, 2020, at 01:24, Philip Jägenstedt ***@***.***> wrote:  Closed #17027. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.