1318426 - (url-oxidation) Metabug for rust-url oxidation

Status: RESOLVED WONTFIX

(Core :: Networking, defect, P3)

Description

[Manish Goregaokar [:manishearth]]

The code for rust-url oxidation lands in bug 1151899, however it is disabled by default. This bug tracks all the work we need to do before enabling it.

Comment 1

[Manish Goregaokar [:manishearth]]

At some point after bug 1151899 lands I'll update the vendored rust-url so that we get the IDNA fixes, and file bugs for any remaining mismatches then.

Comment 2

[Manish Goregaokar [:manishearth]]

https://bugzilla.mozilla.org/show_bug.cgi?id=906714 will be useful for finding other bugs/mismatches; we should ensure that rust-url is not prone to any of those.

Comment 3

[Manish Goregaokar [:manishearth]]

As pointed out in https://bugzilla.mozilla.org/show_bug.cgi?id=1324193#c6 , we have a bunch of Gecko bugs causing mismatches.

In particular, we have bug 945240, bug 1324243, bug 1324251, bug 1324244, bug 1324246, bug 1324247

On top of that, rust-url makes a whole bunch of the WPT url tests pass, at https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b09455e1f766fc9f215057f0a1012c3119892ca&selectedJob=32931347 . Some of this is related to the bugs filed above, some isn't.

Should fixing these block shipping rust url side-by-side with telemetry? I'm afraid that we'll have an overabundance of mismatches due to Gecko not being spec-compliant. On the other hand, I'm afraid that this will become an endless rabbit hole of Gecko fixes. Probably not. I'm not sure how spec-compliant the current parser is.

Thoughts?

Comment 4

[Patrick McManus [:mcmanus]]

(In reply to Manish Goregaokar [:manishearth] from comment #3)

> Should fixing these block shipping rust url side-by-side with telemetry? I'm
> afraid that we'll have an overabundance of mismatches due to Gecko not being
> spec-compliant. On the other hand, I'm afraid that this will become an
> endless rabbit hole of Gecko fixes. Probably not. I'm not sure how
> spec-compliant the current parser is.
> 

spec-compliant is not especially interesting. web-compliant is. And yes, that's a lot more annoying.

To the extent that rust and gecko diverge they need to be brought together in a web compatible way. That might be the gecko way which we know 'works'. It might be the chrome way which also 'works'.

This is not something to lose users over - urls have always been a messy space.

While the rust results are not being used, except for data gathering via telemetry, I don't have requirements for how consistent this needs to be.

Comment 5

[Anne (:annevk)]

Manish, have you looked at url/ on WPT? That is in line with the standard and would allow for somewhat easy comparison across a range of issues. I'll also point out that Safari is shipping an implementation that is fairly close to the standard in their Technology Preview.

Comment 6

[Manish Goregaokar [:manishearth]]

We have; I think most of those mismatches have been fixed. The remaining mismatches are all Gecko-side bugs as mentioning in comment 3.

Comment 7

[(no longer active)]

Has there been a full code audit to ensure that RustURL matches all of the Gecko API intricacies?  I'm asking because I found bug 1340695 during a code audit.

Comment 8

[Manish Goregaokar [:manishearth]]

No, such an audit has not happened yet.

Comment 9

[Anne (:annevk)]

What is the relationship to bug 1228115?

Comment 10

[(no longer active)]

(In reply to Anne (:annevk) from comment #9)
> What is the relationship to bug 1228115?

nsIURI and its related APIs are currently exposed to add-ons, and, add-ons can inject themselves into our URI parsing machinery, for example, by taking over the parsing of a foobar: scheme.  Since the add-on code in the said machinery is written in JS, and JS in this environment (XPConnect) can only run on the main thread, it prevents us from doing things like parsing URIs inside Gecko off the main thread.  rust-url is affected in that it implements nsIURI, but with Gecko 57 where we won't support XPCOM extensions any more, these restrictions will not exist any more.

Comment 11

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 12

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 13

[Manish Goregaokar [:manishearth]]

Folks decided against this in bug 1415206

Comment 14

[Valentin Gosu [:valentin] (he/him)]

(In reply to Manish Goregaokar [:manishearth] from comment #13)
> Folks decided against this in bug 1415206

Just as a side note, rust-url is still used in MozURL.