1305316 - Changing URI.spec to be hostless, with a different protocol, no longer works on an http URI with host (Component returned failure code: 0x804b000a (NS_ERROR_MALFORMED_URI))

Status: RESOLVED INVALID

(Core :: Networking, defect)

Description

[Diego Casorran [:diegocr]]

Open an about:newtab page and then the webconsole, next type the following:

uri = Services.io.newURI('https://mysite.com/#section1', null, null);
uri.spec = 'myProto:#section1'

In Firefox 49 it does work fine (latest stable, at time of this write), while in Firefox 50 (Dev Edition) it throws the following exception:

[Exception... "Component returned failure code: 0x804b000a (NS_ERROR_MALFORMED_URI) [nsIURI.spec]"  nsresult: "0x804b000a (NS_ERROR_MALFORMED_URI)"  location: "JS frame :: debugger eval code :: <TOP_LEVEL> :: line 1"  data: no]

I think "myProto:#section1" should be - and continue to be - a host-less valid URI, right? at least that's how we create protocol handlers using nsIProtocolHandler

Comment 1

[:Gijs (he/him)]

Some of the URI-parsing stuff changed semi-recently.

Can you find a regression window?

Does it work with newURI("myProto:#foo", null, null) ?

Valentin, any chance this is caused by bug 1275746?

Comment 2

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=81bd8453740f30844982f13ca739ce960889d208&tochange=5fc004cf355f0321c2f6de841f84c20822bc2a91

So seems pretty clear this was bug 1275746.

Valentin, is this expected? Is there a way around this?

Comment 3

[Valentin Gosu [:valentin] (he/him)]

(In reply to :Gijs Kruitbosch from comment #2)
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=81bd8453740f30844982f13ca739ce960889d208&tochange=5fc0
> 04cf355f0321c2f6de841f84c20822bc2a91
> 
> So seems pretty clear this was bug 1275746.
> 
> Valentin, is this expected? Is there a way around this?

When we init an nsStandardURL we call nsIStandardURL.init(type,port,spec,charset,baseURL) where type is any of nsIStandardURL.URLTYPE_STANDARD, URLTYPE_AUTHORITY or URLTYPE_NO_AUTHORITY.
If you want to set a spec that doesn't have a hostname, please init with URLTYPE_NO_AUTHORITY.

TL;DR: instead of url.spec = "myProto:#foo"
you can use url = Services.io.newURI("myProto:#foo", null, null);
This works. Also, changing an nsIURI's to another _type_ of protocol isn't the best idea. If you know of other places in the code where we do this, we should try to fix them.

Comment 4

[:Gijs (he/him)]

From a quick check, I couldn't spot anywhere where we do this apart from with uninit'd nsISimpleURIs inside protocol handlers, which I think is fine. A quick check on addons DXR also mostly shows nsISimpleURI usage and sometimes nsIStandardURL usage. I'll mark the blocking bug addon-compat, but I think there's not much else we can do in here.

Comment 5

[Diego Casorran [:diegocr]]

(In reply to Valentin Gosu [:valentin] from comment #3)
> TL;DR: instead of url.spec = "myProto:#foo"
> you can use url = Services.io.newURI("myProto:#foo", null, null);

Well, the use-case here is that `url` comes/was created by some Firefox's internal code, so we can't replace it, we use `uri.spec` to redirect the user from an http/s website to our extension's web app (chrome:// page, through a protocol-handler)

> changing an nsIURI's to another _type_ of protocol isn't the best idea.

I did heard the same in the past, but no reasons given as "the why" - can you elaborate on this please?

We do it that way because it's apparently more lightweight that setting up an http observer (Ie, http-on-modify-request)

Comment 6

[:Gijs (he/him)]

(In reply to Diego Casorran [:diegocr] from comment #5)
> (In reply to Valentin Gosu [:valentin] from comment #3)
> > TL;DR: instead of url.spec = "myProto:#foo"
> > you can use url = Services.io.newURI("myProto:#foo", null, null);
> 
> Well, the use-case here is that `url` comes/was created by some Firefox's
> internal code, so we can't replace it, we use `uri.spec` to redirect the
> user from an http/s website to our extension's web app (chrome:// page,
> through a protocol-handler)

Can you elaborate? Where does this URL come from and where exactly are you replacing it?

Comment 7

[Valentin Gosu [:valentin] (he/him)]

(In reply to Diego Casorran [:diegocr] from comment #5)
> > changing an nsIURI's to another _type_ of protocol isn't the best idea.
> I did heard the same in the past, but no reasons given as "the why" - can
> you elaborate on this please?

It's because an nsIURI could have different objects implementing it. For example nsStandardURL, nsSimpleURI, nsJSURI, etc.
Calling uri.spec = ... doesn't make the object turn from nsStandardURL to nsJSURI, and it can lead to odd situations.

If you want something that for most URLs, I suggest the DOM URL [1]. Changing the spec or the protocol actually parses the URL again, making sure you don't end up with an invalid URL.
[1] https://developer.mozilla.org/en-US/docs/Web/API/URL/URL

Comment 8

[Diego Casorran [:diegocr]]

(In reply to :Gijs Kruitbosch from comment #6)
> Can you elaborate? Where does this URL come from and where exactly are you
> replacing it?

Sorry, i should have mentioned it. This is being handled through an nsIContentPolicy

When aContentType is either TYPE_DOCUMENT or TYPE_SUBDOCUMENT we do that `uri.spec` replacing to redirect the user.

Comment 9

[Diego Casorran [:diegocr]]

(In reply to Valentin Gosu [:valentin] from comment #7)
> It's because an nsIURI could have different objects implementing it. For
> example nsStandardURL, nsSimpleURI, nsJSURI, etc.
> Calling uri.spec = ... doesn't make the object turn from nsStandardURL to
> nsJSURI, and it can lead to odd situations.
> 

I see, thanks for the details. Do you think the nsIContentPolicy case could have such odd situations? I didn't noticed any in years, but i might be missing something (neither i know what these "odd situations" are exactly :))

> If you want something that for most URLs, I suggest the DOM URL [1].
> Changing the spec or the protocol actually parses the URL again, making sure
> you don't end up with an invalid URL.
> [1] https://developer.mozilla.org/en-US/docs/Web/API/URL/URL

Unfortunately this doesn't seem usable here, i cannot replace the URI provided on nsIContentPolicy's shouldLoad()

Comment 10

[Valentin Gosu [:valentin] (he/him)]

(In reply to Diego Casorran [:diegocr] from comment #9)
> I see, thanks for the details. Do you think the nsIContentPolicy case could
> have such odd situations? I didn't noticed any in years, but i might be
> missing something (neither i know what these "odd situations" are exactly :))

If your URI layout and behaviour is pretty much the same it should be fine. It becomes weird when you change the protocol/host/etc then try to create a new URI based on the spec, and it fails. Or various other changes that are protocol dependent. Comment 0 is a good example. nsSimpleURI might allow an empty hostname, but nsStandardURL might not.

Comment 11

[Diego Casorran [:diegocr]]

(In reply to Valentin Gosu [:valentin] from comment #10)
> Comment 0 is a good example. nsSimpleURI might allow an empty hostname, but nsStandardURL might not.

Yep, but it's a good example starting with Fx 50+ only ;)

Anyway, in my case i could just use `uri.spec = 'myProto:meh#section1'` and it'll continue to work, just keep in mind these hostless changes might break some other add-ons out there.

Thanks guys for your time/info/advice.

Comment 12

[:Gijs (he/him)]

(In reply to Diego Casorran [:diegocr] from comment #8)
> (In reply to :Gijs Kruitbosch from comment #6)
> > Can you elaborate? Where does this URL come from and where exactly are you
> > replacing it?
> 
> Sorry, i should have mentioned it. This is being handled through an
> nsIContentPolicy
> 
> When aContentType is either TYPE_DOCUMENT or TYPE_SUBDOCUMENT we do that
> `uri.spec` replacing to redirect the user.

The URI is an "in" parameter to that API. It seems pretty... odd... to try and modify it, and that's explicitly not what the API is meant for. It would not surprise me if we broke more and more of that functionality in the future. I know people are looking at just making URI objects immutable.

Is your add-on on AMO? Do you know if/where there are other add-ons that use this mechanism?

If this is something that's widely used maybe we should add a different type of method that lets you do what you want. Not sure why http-on-modify-request isn't suitable, but I'm not an expert.

Comment 13

[Diego Casorran [:diegocr]]

I can't say if this approach is widely used. I do use it in some of my add-ons, that one using the protocol-handler is for my company's website which is on AMO as unlisted, I do have some other addons such as Bluhell that does the same, without a protocol-handler though, just a `uri.spec = 'resource://..bla'` which obviously must not be affected by those hostless changes.

Whether http-on-modify-request isn't suitable, well IIRC nsIContentPolicy is invoked before the http channel is created and such, hence it must be less overkill, also i can quickly decide if the resource going to be loaded should be allowed, rejected, or redirected (with the uri.spec trick) by merely primitives comparison, while the http observer might require to QueryInterface the http channel etc AFAICT