MatchPatternSet::Subsumes("*://*/*") does not match even if the set contains http(s)
Categories
(WebExtensions :: General, defect, P5)
Tracking
(Not tracked)
People
(Reporter: robwu, Unassigned)
References
Details
When matchpatternset.subsumes(matchpattern) is called, the implementation of MatchPatternSetCore::Subsumes returns true only if any of the patterns in the MatchPatternSet subsume matchpattern.
This logic assumes that the only way for a match pattern to be subsumed by the set if it is subsumed by any of the individual match patterns.
This assumption is incorrect: if the matchpattern is *://*/*, then it may also be subsumed if any of the match patterns in the set match the individual wildcard schemes (https, http, wss, ws).
matchpatternset.subsumes is mostly used for permission testing in the context of document/request access requests. In these contexts, only https and http are meaningful.
Therefore, new MatchPatternSet(["https://*/*", "http://*/*"]).subsumes(new MatchPattern("*://*/*")) should ideally be true.
Related: bug 1853409, part 1 introduces the matchesAllWebUrls method, which could allow the use case to be filled by matchpatternset.matchesAllWebUrls && matchpattern.matchesAllWebUrls
Hey Tom, I think this is a followup bug spun off from a phabricator revision you may have reviewed, what priority would be reasonable to assign to this issue from your perspective? do you have some insight about the priority this would have from Rob perspective?
Updated•2 years ago
|
Comment 2•2 years ago
|
||
I think the impact of this is very minimal, an extension which lists http://* and https://* (optional) permissions separately is not able to register a content script [1] or request a host permission [2] using the *://*/ match pattern.
Since the * scheme used to cover more than just http and https (and might again in the future), I'm not sure these use cases should be supported. I'm even inclined to wontfix this.
| Reporter | ||
Comment 3•2 years ago
|
||
(In reply to Tomislav Jovanovic :zombie from comment #2)
I think the impact of this is very minimal,
This bug is causing bug 1856383. But as noted in bug 1856383, that one can be fixed in another way. That's why I filed these two bugs (root cause & consequence) separately.
is not able to register a content script [1] or request a host permission [2]
For future reference:
[1] is https://searchfox.org/mozilla-central/rev/413b88689f3ca2a30b3c49465730c0e7d40f9188/toolkit/components/extensions/parent/ext-contentScripts.js#181
[2] is https://searchfox.org/mozilla-central/rev/413b88689f3ca2a30b3c49465730c0e7d40f9188/toolkit/components/extensions/parent/ext-permissions.js#92
The UI version of the API at [2] is at https://searchfox.org/mozilla-central/rev/413b88689f3ca2a30b3c49465730c0e7d40f9188/toolkit/mozapps/extensions/content/aboutaddons.js#2464
Since the
*scheme used to cover more than just http and https (and might again in the future), I'm not sure these use cases should be supported.
* scheme covers ws(s) in Firefox, in addition to http(s) that are supposed to match http(s) only. The ws(s) part is a Firefox-only hack to support the WebRequest (and proxy, and DNR) APIs (requests to improve test cases are in bug 1635929 and bug 1828151). It is not super relevant in permissions checks. I think that it would make sense to let *://*/* subsume http(s) even if ws(s) is not present. For permission checks, ws(s) should ideally be treated as http(s).
I'm even inclined to wontfix this.
This is a valid bug that causes non-obvious other bugs, so I recommend to keep it open to serve as a reference in case a work-around is needed. This bug number is referenced at https://searchfox.org/mozilla-central/rev/413b88689f3ca2a30b3c49465730c0e7d40f9188/toolkit/components/extensions/MatchPattern.cpp#546-547
Description
•