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

Status: RESOLVED FIXED

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

Description

[Danny Lin]

Attachment: A screenshot showing the error

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.

Comment 1

[Martin Giger [:freaktechnik]]

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.

Comment 2

[Shane Caraveo (:mixedpuppy)]

Daniel, this look's like the issue from bug 1328865, but in the query string.  Wondering if you could take a look.

Comment 3

[Daniel Stenberg [:bagder]]

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...

Comment 4

[Shane Caraveo (:mixedpuppy)]

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.

Comment 5

[Shane Caraveo (:mixedpuppy)]

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.

Comment 6

[Gary Chen [:xeonchen]]

(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?

Comment 7

[Danny Lin]

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".

Comment 8

[Shane Caraveo (:mixedpuppy)]

(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 9

[Gary Chen [:xeonchen]]

Attachment: Bug 1415574 - make nsJARURI::SetSpecWithBase ignore URL string;

Review commit: https://reviewboard.mozilla.org/r/201514/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/201514/

Comment 10

[Daniel Stenberg [:bagder]]

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?

Comment 11

[Gary Chen [:xeonchen]]

Comment on attachment 8930375 [details]
Bug 1415574 - make nsJARURI::SetSpecWithBase ignore URL string;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/201514/diff/1-2/

Comment 12

[Gary Chen [:xeonchen]]

Comment on attachment 8930375 [details]
Bug 1415574 - make nsJARURI::SetSpecWithBase ignore URL string;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/201514/diff/2-3/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c76ff8b1c4d4
make nsJARURI::SetSpecWithBase ignore URL string; r=bagder

Comment 14

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/c76ff8b1c4d4