Open Bug 1788127 Opened 4 years ago Updated 2 years ago

Do we really need to strip URI schemes other than http and https?

Categories

(Firefox :: Address Bar, task, P3)

task

Tracking

()

People

(Reporter: scunnane, Unassigned)

References

Details

(Whiteboard: [sng])

Address bar code should only be stripping out the http and https URI schemes before running the appropriate providers. However, over the years, additional schemes also came to be stripped out. With this bug, we want to discover why the additional schemes besides http and https came to be stripped out.

I did a fair amount of code archeology to trace how we’ve historically handled stripping the scheme from the URI. This gets at the "what", but a lot of the "why" is still missing.

  1. Back in 2014, Marco added this code to strip out just http, https and ftp.
  2. Then in 2018, as part of a series of patches to improve autofill, Drew changed Marco's stripPrefix function to strip out all schemes - see the diff here
  3. In 2020, Harry added the stripURLPrefix method to the UrlbarUtils object. It does the exact same things as Drew’s stripPrefix function, but Harry slightly refactored the REGEXP_STRIP_PREFIX and scoped it to the method - see diff here
  4. At some point, stripPrefix was renamed stripAnyPrefix - I couldn’t find where, but only the function name changed, nothing in the comments or code itself. Then, in 2021, Harry removed the stripAnyPrefix function as part of cleaning up the code in UrlbarProviderPlaces - see the diff here
  5. Finally, James landed a patch this past May that introduces a UrlbarUtils.PROTOCOLS_WITHOUT_AUTHORITY constant. Here’s the associated bug, and here’s the diff. His patch strips the prefix when the scheme ends in : (not ://) and the scheme is NOT on a safelist. That safelist includes about:, data:, file:, javascript: and view-source:.

It seems like investigating number 2 above is the best place to start.

I've landed a patch so that about: is no longer stripped. To ensure that this patch and any future patches handling non-http(s) URIs don't trample over previous reasoning, we need to determine why the additional schemes besides http and https came to be stripped out.

See Also: → 1495057

We should really understand whether we need that complication or not, and in case we don't, just revert to only strip http and https.

Summary: Code archeology re: why we strip URI schemes other than http and https → Do we really need to strip URI schemes other than http and https?
Severity: -- → N/A
Priority: -- → P3
Whiteboard: [sng]
You need to log in before you can comment on or make changes to this bug.