Skip to content

Add version control URI schemes to registerProtocolHandler safelist#1829

Open
joshtriplett wants to merge 1 commit intowhatwg:mainfrom
joshtriplett:vcs
Open

Add version control URI schemes to registerProtocolHandler safelist#1829
joshtriplett wants to merge 1 commit intowhatwg:mainfrom
joshtriplett:vcs

Conversation

@joshtriplett
Copy link
Copy Markdown

@joshtriplett joshtriplett commented Sep 27, 2016

This allows sites to register protocol handlers for version control
repositories.

I've confirmed uses of every one of these schemes in the wild.


💥 Error: Wattsi server error 💥

PR Preview failed to build. (Last tried on Jan 15, 2021, 7:57 AM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 Wattsi Server - Wattsi Server is the web service used to build the WHATWG HTML spec.

🔗 Related URL

Parsing MDN data...
Parsing...
Generating HTML variant...
Error: Element found with dfn type name and redundant export attribute; dfn is <h4> element containing "StructuredCloneWithTransfer ( input,   transferList, targetRealm )"; previous heading contents are "StructuredCloneWithTransfer ( input,   transferList, targetRealm )"
Error: Element found with dfn type name and redundant export attribute; dfn is <h4> element containing "StructuredClone ( input, targetRealm   [ , memory ] )"; previous heading contents are "StructuredClone ( input, targetRealm   [ , memory ] )"
Error: Element found with dfn type name and redundant export attribute; dfn is <h4> element containing "IsTransferable ( O )"; previous heading contents are "IsTransferable ( O )"
Error: Element found with dfn type name and redundant export attribute; dfn is <h4> element containing "Transfer ( input, targetRealm )"; previous heading contents are "Transfer ( input, targetRealm )"
Error: Element found with dfn type name and redundant export attribute; dfn is <dfn> element containing "[HTMLConstructor]"; previous heading contents are "3.2.3 HTML element constructors"
Error: Element found with dfn type name and redundant export attribute; dfn is <dfn> element containing "content"; previous heading contents are "4.2.5 The meta element"
Error: Element found with dfn type name and redundant export attribute; dfn is <dfn> element containing "http-equiv"; previous heading contents are "4.2.5.3 Pragma directives"
Error: Element found with dfn type name and redundant export attribute; dfn is <dfn> element containing "Content security policy state"; previous heading contents are "4.2.5.3 Pragma directives"
Error: Element found with dfn type name and redundant export attribute; dfn is <dfn> element containing 


If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

@domenic
Copy link
Copy Markdown
Member

domenic commented Sep 27, 2016

Are you saying browsers already include these on the safelist, or is this a proposal and you're hoping to get implementer interest?

@joshtriplett
Copy link
Copy Markdown
Author

joshtriplett commented Sep 28, 2016

This is a proposal. I intend to submit patches to browsers in parallel.

This seemed like a change for which the implementation was straightforward enough (extending an existing safelist) that submitting the standards patch to kick off discussion seemed appropriate.

@domenic domenic added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest do not merge yet Pull request must not be merged per rationale in comment labels Sep 28, 2016
@annevk
Copy link
Copy Markdown
Member

annevk commented Sep 28, 2016

That is certainly fair. Please keep this issue informed about any patches/bugs against browsers.

@domenic
Copy link
Copy Markdown
Member

domenic commented Sep 28, 2016

This may also be a good place to put your reasoning, if browsers ask why adding these is important. E.g. what new applications this allows you to build, and why this deserves the same status as the existing safelisted schemes, and so on.

This allows sites to register protocol handlers for version control
repositories.

I've confirmed uses of every one of these schemes in the wild.
@joshtriplett
Copy link
Copy Markdown
Author

(Updated the list to include one missing scheme.)

@joshtriplett
Copy link
Copy Markdown
Author

@joshtriplett
Copy link
Copy Markdown
Author

joshtriplett commented Sep 29, 2016

Firefox, as far as I can tell, doesn't use a safelist for registerProtocolHandler; it uses a denylist instead. So Firefox already supports all these schemes.

@asankah
Copy link
Copy Markdown
Contributor

asankah commented Aug 17, 2018

+1 from Chromium. Will follow up on Chromium issue.

@domenic domenic removed the needs implementer interest Moving the issue forward requires implementers to express interest label Aug 17, 2018
@domenic
Copy link
Copy Markdown
Member

domenic commented Aug 17, 2018

@joshtriplett, are you still interested in this? If so, could you sign the Participant Agreement? It's a new requirement since you first submitted this PR.

@domenic domenic removed the do not merge yet Pull request must not be merged per rationale in comment label Aug 17, 2018
@joshtriplett
Copy link
Copy Markdown
Author

joshtriplett commented Aug 18, 2018 via email

@mgiuca
Copy link
Copy Markdown
Contributor

mgiuca commented Sep 5, 2018

This has been discussed on a blink-dev thread.

Pasting my relevant comments:

I think it "smells" a bit when we start adding specific companies or services' names to web specs. Most of the existing ones are generic, but it's a bit irksome that we have "bitcoin" on there, for example, being just one cryptocurrency scheme.

  • bzr, cvs, darcs, git, hg, svn: A little "smelly" that we would favour a specific set of revision control systems, but at least these represent distinct technologies, not specific companies or sites (so I'm not fully opposed to this).
  • lp: This URL scheme is (as far as I'm aware) used by the Bazaar revision control system specifically as a short-hand for repos hosted on the https://launchpad.net website. It doesn't feel generic enough that it should be exposed in a web standard.

My preference would be to switch from a whitelist to a blacklist, which would be significantly smaller than an ever-expanding list of all protocols that anybody has ever wanted to expose on the web. #3998

@joshtriplett
Copy link
Copy Markdown
Author

@mgiuca Personally, I'm entirely in favor of switching to a blocklist, which AFAICT is what Firefox does as well.

In the meantime...I feel like this is a list of unique version control system protocols, not a list of specific products; for instance, there are a few pieces of software out there that speak the bzr or hg or git protocols, not just the original piece of software.

As for lp and bzr+lp, the only reason I'm proposing them is that bzr has native support for those that ships in the stock version, not via custom configuration but as a built-in part of the standard client, and because of that people regularly pass around such URLs and just hand them to bzr clone and similar tools, without further explanation or alternates. People expect those URLs to Just Work.

@mgiuca
Copy link
Copy Markdown
Contributor

mgiuca commented Sep 5, 2018

As for lp and bzr+lp, the only reason I'm proposing them is that bzr has native support for those that ships in the stock version, not via custom configuration but as a built-in part of the standard client, and because of that people regularly pass around such URLs and just hand them to bzr clone and similar tools, without further explanation or alternates. People expect those URLs to Just Work.

Yeah, I'm aware of that (it's tough to find people who know Bzr these days, but I was 100% in the Bzr camp back in the day!). It's a fair point, this is part of the set of URLs recognised by Bzr itself. On the other hand, as far as I know, websites don't typically include direct links to "lp:" URLs (or, rather, any raw revision control URLs) since they pretty much don't work right now. While those URLs are useful to paste into revision control clients, they aren't seen in <a href>s on the web. So it's not unrealistic to expect that if, say, we added "bzr" to the whitelist but not "lp", then websites would just expand "lp:" links to the full "bzr:" format.

In other words, we don't have to accept every possible URL scheme accepted by the client itself, in order for this to be useful.

Having said that, I'd be in favour of switching to a blacklist as well. It would remove all of these questions. If Firefox is, as you say, already open-ended, then it should be much less of an issue for Chrome and other browsers to migrate over (note: I'm kinda-sorta-responsible for registerProtocolHandler on Chrome so I can help here, but it looks like @asankah has it covered). Please discuss on #3998.

@annevk
Copy link
Copy Markdown
Member

annevk commented Sep 5, 2018

A blocklist would be tricky if we ever need a new scheme for some web platform feature, though I suppose we could reserve a prefix of sorts.

@mgiuca
Copy link
Copy Markdown
Contributor

mgiuca commented Sep 6, 2018

I'm not sure it's that bad if an allowed scheme becomes disallowed.

RPH won't generally be a core function of a website (for one thing, many browsers don't even support this feature). Generally they're just a nice-to-have feature. A future change to add a new scheme to the blocklist will disrupt handling of certain schemes, but not stop the user from getting to that resource through some other means. Same applies to user-agent-specific blocked schemes. Let's say some site registers to handle "chrome:" URLs for some reason, and it works in Firefox but not Chrome, that isn't a very big deal.

There may be a list of schemes we want to guarantee never get blocked by any browser (e.g. "mailto"). Perhaps just the list of things we already have. So we could say:

User agents MUST NOT allow sites to register the following schemes: (blocklist): "https", "ftp", etc.

User agents MUST allow sites to register the following schemes: (whitelist): "mailto", "bitcoin", etc. And any scheme starting with "web+".

User agents SHOULD allow sites to register any other scheme, but MAY disallow any specific scheme.

@anssiko
Copy link
Copy Markdown

anssiko commented Nov 19, 2018

If so, could you sign the Participant Agreement? It's a new requirement since you first submitted this PR.

@joshtriplett, I signed whatwg/participant-data@2c4a895. @domenic I expect that to satisfy the requirement.

@annevk annevk added the needs tests Moving the issue forward requires someone to write tests label Nov 19, 2018
@annevk
Copy link
Copy Markdown
Member

annevk commented Nov 19, 2018

Great, one other thing that'd be good here is tests.

@domenic
Copy link
Copy Markdown
Member

domenic commented Nov 19, 2018

Awesome, thanks @anssiko!

Do we know how to test protocol handlers? And, how's our safelist-vs.-blocklist discussion coming? If I recall I found @mgiuca's positions fairly convincing, but IIRC @annevk had some concerns about how blocklists are only easier for browsers with larger market share, and I'm not sure those were addressed.

@Honry
Copy link
Copy Markdown

Honry commented Nov 29, 2018

Do we know how to test protocol handlers?

We have some basic tests in wpt, and I just imported more chromium tests to wpt in PR web-platform-tests/wpt#14294.

@anssiko
Copy link
Copy Markdown

anssiko commented Dec 14, 2018

web-platform-tests/wpt#14294 was merged. Anything else blocking this PR we could help with?

@annevk
Copy link
Copy Markdown
Member

annevk commented Dec 14, 2018

@anssiko is there a bug for Safari?

@annevk
Copy link
Copy Markdown
Member

annevk commented Dec 14, 2018

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1514203 to keep track of switching Firefox to a safelist-based approach. Firefox uses a safelist.

@anssiko
Copy link
Copy Markdown

anssiko commented Dec 19, 2018

@annevk I’m not aware of a Safari bug. @joshtriplett submitted the Chromium bug https://bugs.chromium.org/p/chromium/issues/detail?id=651311 quite a while ago, and I believe is waiting for the spec patch to land before advancing to other browsers.

@domenic
Copy link
Copy Markdown
Member

domenic commented Dec 19, 2018

What I recall is that @asankah worked to advance this in Blink, but ran into disagreements with other Blink folks (I remember @mgiuca, but there may have been others) and the project stalled :-/.

@joshtriplett
Copy link
Copy Markdown
Author

joshtriplett commented Jul 29, 2024

I'm still interested in seeing this move forward. Is there anything that would help unstick this?

The live VCS landscape has changed in the ~8 years since I originally submitted this, and I'd be happy to prune back the list substantially if that would make it easier to accept.

It looks like, in #3998 , there's a proposal to switch to a blocklist instead, which would be even better. What would it take to move that proposal forwards?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

addition/proposal New features or enhancements needs tests Moving the issue forward requires someone to write tests topic: custom protocols

Development

Successfully merging this pull request may close these issues.

7 participants