-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inconsistency: runtime.getURL()
#281
Comments
This is great @carlosjeurissen! A lot of the Safari (and Firefox) behaviors boil down to us returning an absolute resolved URL, that tries to proactively resolve I agree case 11 is odd! |
Thanks for your investigation! Inconsistency 1: Agree to throw on non-string arguments. Inconsistency 2: I agree this looks like a Safari issue. Inconsistency 3: passing a full external URL For those 5 cases, I believe it should act as the following:
Example: getURL('/') // browser-extension://extension1/
getURL('http://example.com') // error
getURL('browser-extension://extension1/file') // return same string
getURL('browser-extension://extension2/file') // error
getURL('//') // error
getURL('///') // error
getURL('//extension1/file') // browser-extension://extension1/file
getURL('//extension2/file') // error Inconsistency 8: Is |
@xeenon You mentioned you will follow up with the Safari related items. What inconsistencies would this cover? I assume 1, 2 and 11? This leaves the other ones to be discussed on how to handle them. @Jack-Works thanks for your proposals! Seems like this approach makes a lot of sense. You have my support! As for the |
Yes, I am filing internal bugs about issue 1, 2, and 11. |
In today's meeting there was consensus among browser vendors that our main concern with the return value of Additionally, @Rob--W and @xeenon expressed a desire to investigate Inconsistency 3. I think we may also want to align on how to handle Inconsistency 4, as the differences in this case may cause an implementation to work in one browser but not another. |
I have filed https://bugzilla.mozilla.org/show_bug.cgi?id=1795082 for Firefox's handling of |
Firefox has fixed the inconsistencies in Firefox 130 (https://bugzilla.mozilla.org/show_bug.cgi?id=1795082). The URL normalization has been removed, and I included almost every test case here in unit tests: https://searchfox.org/mozilla-central/rev/c4966e1c1e6a8cb38c15a27693e8ecd63082055e/toolkit/components/extensions/test/xpcshell/test_ext_geturl.js#21-70 |
Background
During TPAC we discussed our intentions to start writing spec and tackle browser inconsistencies. We proposed to tackle a small chunks of API hoping that after some time all the APIs which are out there right now have been discussed and documented properly.
To start, off, inspired by #270 and #273, I want to tackle
runtime.getURL()
as it is a very basic API, has simple syntax and is supposed to be non ambiguous. This API is / was previously (also) available asextension.getURL()
.Definition
In short, the idea behind
runtime.getURL()
is to get the URL of a resource which sits within the extension.Defined by the Chrome Docs:
Converts a relative path within an app/extension install directory to a fully-qualified URL.
And by MDN:
Given a relative path from the manifest.json to a resource packaged with the extension, return a fully-qualified URL.
The API takes a single argument being the path, which is supposed to be a string. Generally speaking the API returns the origin of the extension and adds the path to it. If the argument starts with a slash, the slash is excluded in the response.
from runtime.json:
Going forward
We should determine what the right behaviour for each inconsistency listed below is. And start writing down an exact description on how the algorithm should handle edge cases.
In addition, we should determine when to ship those resolutions. Can they be updated for manifest v2 and v3, or do some need to wait till v4. Some could lead to breaking changes, while others do not have real use cases right now and can be shipped without much trouble.
Inconsistencies
As simple as the API might look, after doing intensive testing with more than 1000 test cases, it turns out there exists many inconsistencies across browsers. To make it easy to go over them one by one, I have grouped the inconsistencies and described how each browser is behaving and my proposed preferred behaviour. In below examples, sometimes the protocol
browser-extension
is being used as generic protocol. Read it aschrome-extension
in Chrome,moz-extension
in Firefox andsafari-web-extension
in Safari. This makes it easier to compare what is returned by the APIs.Inconsistency 1: passing non-string parameter (Safari will match others)
Example: runtime.getURL(200);
Chrome and Firefox currently throw an exception when a non-string parameter is passed to getURL.
Safari currently attempts to call .toString() and proceeds without throwing an exception. If however .toString() fails, for example when passing null or undefined, Safari will throw an exception just like Chrome and Firefox.
Inconsistency 2: passing lesser known utf-8 characters (Safari will match others)
Example: runtime.getURL('Ω');
Safari currently returns
null
when the string passed contains characters like emoji and the greek alphabet.Chrome and Firefox handle these characters just fine.
Seems this is a Safari issue and should be solved.
Inconsistency 3: passing a full external URL
Example: runtime.getURL('https://www.example.com');
Chrome prefixes the URL with the extension origin as such:
browser-extension://$extension_uuid/https://www.example.com/
Firefox and Safari just return the full URL:
https://www.example.com/
Prefixing the URL with the extension origin seems more true to the definition of the API and can potentially reduce attack surfaces. An exception and edge-case to this is inconsistency #4, see below.
Inconsistency 4: passing a full extension-owned URL
Example: runtime.getURL('browser-extension://$extension_uuid/');
Chrome prefixes the URL with the extension origin:
browser-extension://$extension_uuid/browser-extension://$extension_uuid/
Firefox and Safari just return the full URL:
browser-extension://$extension_uuid/
The behaviour of Firefox and Safari can be more useful. Yet the Chrome behaviour would align more with how full external URLs are also handled.
Inconsistency 5: passing URL which is exactly
//
Example: runtime.getURL('//');
Chrome returns
browser-extension://$extension_uuid//
Firefox returns
browser-extension://
Safari returns
browser-extension://$extension_uuid/
Inconsistency 6: passing URL which starts with or is three slashes
///
Example: runtime.getURL('///');
Chrome will prefix with extension origin except trailing slash:
browser-extension://$extension_uuid///
Firefox will prefix with
browser-extension:
:browser-extension:///
Safari has same behaviour as Chrome yet removes two slashes from the start:
browser-extension://$extension_uuid/
Inconsistency 7: passing URL which starts with exactly two slashes and at least one non-slash character
Example: runtime.getURL('//example');
Chrome will prefix with extension origin except trailing slash:
browser-extension://$extension_uuid//example
Firefox and Safari will prefix with
browser-extension:
:browser-extension://example
Inconsistency 8: passing URL which is exactly
.
Example: runtime.getURL('.');
Chrome returns
browser-extension://$extension_uuid/.
Firefox and Safari returns
browser-extension://$extension_uuid/
Inconsistency 9: passing URL which starts with
./
Background: ./ in URLs generally starts resolving URLs
Example: runtime.getURL('././/example');
Chrome will prefix with extension origin:
browser-extension://$extension_uuid/././/example
Firefox and Safari will remove as many
./
prefixes as exists, skips the removal of the first slash, and prefixes it with the extension origin:browser-extension://$extension_uuid//example"
Inconsistency 10: passing URL which contains
../
Background: ../ in URLs generally lets you go up one directory
Example: runtime.getURL('../../example/..//test/');
Chrome will not do anything special:
browser-extension://$extension_uuid/../../example/..//test/
Firefox and Safari will resolve the ../, which in the above example means:
browser-extension://$extension_uuid//test/
Inconsistency 11: Safari
../
artifact edge-cases (Safari will match Firefox)When resolving the
../
in URLs Safari sometimes returns exactlybrowser-extension://$extension_uuid/../
. Example input paths are:../
,/../
,../././
,./.././
,.././
,..//../
,././../
,/./../
,/../../
,../../
,./../
,.././.
,../../.
,./../.
,../.
,../a/..
,..//..
And sometimes it returns exactly
browser-extension://$extension_uuid/..
. Example input paths are:..
,/..
,./..
,../..
,/../..
,./../..
,a/../..
,/./..
,././..
,.././..
,/././..
While some just return
browser-extension://$extension_uuid/
. Example paths are:/.././
,/.././.
,/./../.
,/../.
Bonus for those who can decode the logic for inconsistency 11.
The text was updated successfully, but these errors were encountered: