Closed Bug 1415574 Opened 2 years ago Closed 2 years ago

Unable to load an extension page if URL search contains "!/"

Categories

(Core :: Networking: JAR, defect, P2)

56 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- fixed

People

(Reporter: danny0838, Assigned: xeonchen)

References

Details

(Whiteboard: [necko-triaged])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20171024165158

Steps to reproduce:

1. Assume moz-extension://8c435e6a-def1-44a0-8116-2cf71731d4b6/blocked.html is a valid addon page.
2. Load moz-extension://8c435e6a-def1-44a0-8116-2cf71731d4b6/blocked.html?sometext!/



Actual results:

An error occurs and the extension page is not loaded.


Expected results:

The extension page should be loaded.
Component: General → WebExtensions: Android
Product: Firefox for Android → Toolkit
Version: Firefox 56 → 56 Branch
On desktop this results in a page loading with what seems to be a directory listing of the extension XPI, so this is not Android-only.
See Also: → 1328865
Daniel, this look's like the issue from bug 1328865, but in the query string.  Wondering if you could take a look.
Flags: needinfo?(daniel)
Yes, it looks similar to that issue. The nsJARURI::SetSpecWithBase() method doesn't care for any query part, finds the first "!/" occurrence and consider that the end of the working URI.

I don't know the moz-extension:// URI format (is there a spec somewhere?) or what it says about using !/ in the query part, but maybe the code should also consider not looking for the !/ within the query part? It seems a little strange that this hasn't caused more problems in the past if this is indeed the case...
Flags: needinfo?(daniel)
moz-extension should operate as an http url would.  An extension can be configured to have external moz-extension resources (linkable to from the web) made availabe.  I don't know if jar URIs make use of !/ within the hash or query as some kind of additional pointer into the jar, but it seems to me it shouldn't.
I'll further the prior comment, I think that !/ anywhere in a moz-extension url should never be considered when generating the jar uri, maybe it should just be escaped prior.
Component: WebExtensions: Android → Networking: JAR
Product: Toolkit → Core
See Also: → 719905
Assignee: nobody → xeonchen
Priority: -- → P2
Whiteboard: [necko-triaged]
(In reply to Shane Caraveo (:mixedpuppy) from comment #5)
> I'll further the prior comment, I think that !/ anywhere in a moz-extension
> url should never be considered when generating the jar uri, maybe it should
> just be escaped prior.

Can we just ignore the trailing part after '#' and '?'?
Or just for "moz-extension" protocol (I presume this is our proprietary protocol, maybe we can just find someone to review it?).

I don't know if there's any use case for this bug? Are we going to fix it?
Danny?
Flags: needinfo?(danny0838)
Of course (In reply to Gary Chen [:xeonchen] (needinfo plz) from comment #6)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #5)
> > I'll further the prior comment, I think that !/ anywhere in a moz-extension
> > url should never be considered when generating the jar uri, maybe it should
> > just be escaped prior.
> 
> Can we just ignore the trailing part after '#' and '?'?
> Or just for "moz-extension" protocol (I presume this is our proprietary
> protocol, maybe we can just find someone to review it?).
> 
> I don't know if there's any use case for this bug? Are we going to fix it?
> Danny?

Of course there is at least one use case (so I saw this bug). If the extension page uses the search param to refer to a source web page, which happens to include "!/" in any position of URL (possibly search or hash), this bug occurs.

NOTE: this bug happens even if "!/" is encoded as "!%2F".
Flags: needinfo?(danny0838)
(In reply to Gary Chen [:xeonchen] (needinfo plz) from comment #6)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #5)
> > I'll further the prior comment, I think that !/ anywhere in a moz-extension
> > url should never be considered when generating the jar uri, maybe it should
> > just be escaped prior.
> 
> Can we just ignore the trailing part after '#' and '?'?

I'm not sure how "!/" is handled in the jar protocol, but the occurrence of that anywhere in moz-extension should not end up being used by the jar protocol/channel as if it were part of a jar uri.

> Or just for "moz-extension" protocol (I presume this is our proprietary
> protocol, maybe we can just find someone to review it?).

I'm not sure what you mean by this.  

> I don't know if there's any use case for this bug? Are we going to fix it?

It needs to be fixed.  If it could be done for 59 that would be good.
Comment on attachment 8930375 [details]
Bug 1415574 - make nsJARURI::SetSpecWithBase ignore URL string;

https://reviewboard.mozilla.org/r/201514/#review210044

::: modules/libjar/nsJARURI.cpp:320
(Diff revision 1)
>      nsACString::const_iterator frag = begin;
> -    while (frag != end && *frag != '#') {
> +    while (frag != end && (*frag != '#' && *frag != '?')) {
>          ++frag;
>      }
>      if (frag != end) {
>          // there was a fragment, mark that as the end of the URL to scan

probably update this comment as well?
Attachment #8930375 - Flags: review?(daniel) → review+
Pushed by gachen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c76ff8b1c4d4
make nsJARURI::SetSpecWithBase ignore URL string; r=bagder
https://hg.mozilla.org/mozilla-central/rev/c76ff8b1c4d4
Status: UNCONFIRMED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.