WPT url/url-setters.html CRASH investigation

[CYBAI]

I just noticed this test is currently in CRASH status.

With doing a little bit investigation, I found it's caused by setting hostname for this test case

https://github.com/web-platform-tests/wpt/blob/c3328424a904e34226abd0cb85b2559dcc1cf037/url/resources/setters_tests.json#L1339-L1348

We can reproduce this issue with following script:

<script>
  const url = new URL('non-spec:/.//p');
  url.hostname = '';
  console.log(url);
</script>

and we will get following panic logs

begin <= end (11 <= 9) when slicing `non-spec://p` (thread ScriptThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(1) }, at /Users/cybai/.cargo/registry/src/github.com-1ecc6299db9ec823/url-2.2.0/src/lib.rs:2419)
   0: backtrace::backtrace::libunwind::trace
             at /Users/cybai/.cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.54/src/backtrace/libunwind.rs:90:5
      backtrace::backtrace::trace_unsynchronized
             at /Users/cybai/.cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.54/src/backtrace/mod.rs:66:5
   1: <servo::backtrace::Print as core::fmt::Debug>::fmt
             at /Volumes/Transcend/codespace/servo/servo/ports/winit/backtrace.rs:53:13
   2: core::fmt::write
             at /rustc/4a8b6f708c38342a6c74aa00cf4323774c7381a6/library/core/src/fmt/mod.rs:1092:17
   3: std::io::Write::write_fmt
             at /Users/cybai/.rustup/toolchains/nightly-2021-03-12-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/io/mod.rs:1567:15
   4: servo::backtrace::print
             at /Volumes/Transcend/codespace/servo/servo/ports/winit/backtrace.rs:17:5
   5: servo::main::{{closure}}
             at /Volumes/Transcend/codespace/servo/servo/ports/winit/main2.rs:134:21
   6: std::panicking::rust_panic_with_hook
             at /rustc/4a8b6f708c38342a6c74aa00cf4323774c7381a6/library/std/src/panicking.rs:595:17
   7: std::panicking::begin_panic_handler::{{closure}}
             at /rustc/4a8b6f708c38342a6c74aa00cf4323774c7381a6/library/std/src/panicking.rs:497:13
   8: std::sys_common::backtrace::__rust_end_short_backtrace
             at /rustc/4a8b6f708c38342a6c74aa00cf4323774c7381a6/library/std/src/sys_common/backtrace.rs:141:18
   9: rust_begin_unwind
             at /rustc/4a8b6f708c38342a6c74aa00cf4323774c7381a6/library/std/src/panicking.rs:493:5
  10: core::panicking::panic_fmt
             at /rustc/4a8b6f708c38342a6c74aa00cf4323774c7381a6/library/core/src/panicking.rs:92:14
  11: core::str::slice_error_fail
  12: core::str::traits::<impl core::slice::index::SliceIndex<str> for core::ops::range::Range<usize>>::index
             at /Users/cybai/.rustup/toolchains/nightly-2021-03-12-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/str/traits.rs:214:21
  13: core::str::traits::<impl core::ops::index::Index<I> for str>::index
             at /Users/cybai/.rustup/toolchains/nightly-2021-03-12-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/str/traits.rs:64:9
  14: <core::ops::range::Range<u32> as url::RangeArg>::slice_of
             at /Users/cybai/.cargo/registry/src/github.com-1ecc6299db9ec823/url-2.2.0/src/lib.rs:2419:10
  15: url::Url::slice
             at /Users/cybai/.cargo/registry/src/github.com-1ecc6299db9ec823/url-2.2.0/src/lib.rs:2315:9
  16: url::Url::username
             at /Users/cybai/.cargo/registry/src/github.com-1ecc6299db9ec823/url-2.2.0/src/lib.rs:721:13
  17: url::quirks::set_hostname
             at /Users/cybai/.cargo/registry/src/github.com-1ecc6299db9ec823/url-2.2.0/src/quirks.rs:177:25
  18: script::dom::urlhelper::UrlHelper::SetHostname
             at /Volumes/Transcend/codespace/servo/servo/components/script/dom/urlhelper.rs:64:17
  19: <script::dom::url::URL as script::dom::bindings::codegen::Bindings::URLBinding::URLBinding::URLMethods>::SetHostname
             at /Volumes/Transcend/codespace/servo/servo/components/script/dom/url.rs:192:9
  20: script::dom::bindings::codegen::Bindings::URLBinding::URLBinding::set_hostname::{{closure}}::{{closure}}
             at /Volumes/Transcend/codespace/servo/servo/target/debug/build/script-9ca5645a959cde5b/out/Bindings/URLBinding.rs:1037:26
  21: script::dom::bindings::codegen::Bindings::URLBinding::URLBinding::set_hostname::{{closure}}
             at /Volumes/Transcend/codespace/servo/servo/target/debug/build/script-9ca5645a959cde5b/out/Bindings/URLBinding.rs:1024:33
  22: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &mut F>::call_once
             at /Users/cybai/.rustup/toolchains/nightly-2021-03-12-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:280:13
  23: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /Users/cybai/.rustup/toolchains/nightly-2021-03-12-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panic.rs:344:9
  24: std::panicking::try::do_call
             at /Users/cybai/.rustup/toolchains/nightly-2021-03-12-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:379:40
  25: ___rust_try

I will also file an issue to rust-url repo.

[BurgundyWillow]

If nobody else is working on this issue, I would like to take this issue.

[jdm]

Probably the best way to work on it is to fix servo/rust-url#700 and upgrade Servo to use the version that contains the fix.

[BurgundyWillow]

This would need clarification based on the URL specification. Reason being that,

typically double slashes are not allowed if the URL does not contain an authority component. However, the same on entry is still allowed in browsers. The clarification that is needed is how the rust-url should parse this. Should it remove any double slashes it encounters, effectively making:

non-spec:/.//p => non-spec:/p as it would have done for non-spec:/./p

or should it retain a single slash, like non-spec://p, but ends up changing the format of the URL to one having a domain name and path, thereby changing the structure of the URL.

eg. non-spec:/.//localhost//thispath would become non-spec://localhost/thispath to illustrate the quandary.
If the latter is true, then the test case failing is due to the self parsing tests that will fail.