1547882 - Some characters are incorrectly escaped in the query part of web-extension URLs

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Description

[Marian]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:66.0) Gecko/20100101 Firefox/66.0

Steps to reproduce:

let page = chrome.runtime.getURL("page.html");
page += "&u="+ encodeURIComponent("
https://developer.chrome.com/extensions/tabs#type-MutedInfoReason");

chrome.tabs.update(id, { url: page });

Actual results:

Why is decode the url parameter ?

From: https%3A%2F%2Fdeveloper.chrome.com%2Fextensions%2Ftabs%23type-MutedInfoReason
Is: https%3A//developer.chrome.com/extensions/tabs%23type-MutedInfoReason

This is not happend in Google Chrome.

Comment 1

[Liviu Seplecan]

Hi @Andronachi Marian, first we need some clarifications:

1. The encodeURIComponent() function encodes a Uniform Resource Identifier (URI) component by replacing each instance of certain characters by one, two, three, or four escape sequences representing the UTF-8 encoding of the character (will only be four escape sequences for characters composed of two "surrogate" characters).
2. You provide a URL at steps to reproduce and on at "Actual results" there are two URL encoded. Those are the results after invoking the URL ?
3. I've tried the link provided at steps to reproduce in: https://meyerweb.com/eric/tools/dencoder/ => pressing the "Encode" button the results was (https%3A%2F%2Fdeveloper.chrome.com%2Fextensions%2Ftabs%23type-MutedInfoReason) and pressing the "Decode" button was (https://developer.chrome.com/extensions/tabs#type-MutedInfoReason).
4. So, what is your error?
5. Please make the things clear and let me known after. Thanks for your contribution.

Comment 2

[Marian]

When you pass an EncodedURL to chrome.tabs.update(); or maybe when pressing a URL. The URL in the TAB_BAR will decode automatically all characters %2F to /. How the first post is From -> To.

Why the TAB_BAR URL is decoding automatically the %2F to / ?

Comment 3

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

Can you provide some clear steps to reproduce that actually work in Firefox?

chrome.runtime.getURL is not something that works when I paste it into the devtools console.
Is this for an addon?

Please be very clear and explicit with every step we need to take to reproduce the bug.

Comment 4

[Marian]

Yes is from addon.

Comment 5

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

In that case moving to the Webextensions component. I hope they have enough info to reproduce.

Comment 6

[Marian]

https://drive.google.com/file/d/1kdSyA1vQax2wQSvGkLfxzdhwzFiWV-hA/view?usp=sharing

Comment 7

[Kris Maglione [:kmag]]

We decode some characters for readability when we display URLs in the location bar, but underneath we still load them fully encoded.

Comment 8

[Marian]

Is not only in the location bar. If i say in DevTools "window.location.href". It's decoded.
Yes is a good idea. But its change the URL from window.location.href. In this case is not a good idea.

Comment 9

[Marian]

If i want to compare a URL and a cache URL i cant because is different from the first time.

Comment 10

[Kris Maglione [:kmag]]

(In reply to Andronachi Marian from comment #8)

Is not only in the location bar. If i say in DevTools "window.location.href". It's decoded.

Huh. That's interesting. I don't know what layer that's happening in, then. We pass the URL with entities intact to linkedBrowser.loadURI.

Comment 11

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

I contacted the reporter and I think I finally figured out what the problem is.

STR:
Install https://addons.mozilla.org/en-US/firefox/addon/holddingtab/
Either drag a bookmark into the URL bar, or drag one of the top sites from the about:newtab page into the URL bar.
The extension will take you to something that looks like: moz-extension://a7d1572e-3beb-4d93-a920-c408fa09e8ea/_source/holding.html?t=news.ycombinator.com&i=icons/holding.png&u=https%3A//news.ycombinator.com/
You'll notice that the u= has unescaped slashes

This can be more easily reproduced as:

var url = new URL("moz-extension://a7d1572e-3beb-4d93-a920-c408fa09e8ea/_source/holding.html?u=https%3A%2F%2Fnews.ycombinator.com%2F");
console.log(url.href); // moz-extension://a7d1572e-3beb-4d93-a920-c408fa09e8ea/_source/holding.html?u=https%3A//news.ycombinator.com/

// but you can do this:
url.search="u=https%3A%2F%2Fnews.ycombinator.com%2F";
console.log(url.href); // moz-extension://a7d1572e-3beb-4d93-a920-c408fa09e8ea/_source/holding.html?u=https%3A%2F%2Fnews.ycombinator.com%2F

Comment 12

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

Attachment: Bug 1547882 - Don't unescape %2e and %2f in the query or hash or moz-extension URLs. r=kershaw

Since the SubstitutingProtocolHandler is also extended by nsResProtocolHandler, this change would also apply to resource:// URLs.

Comment 13

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

Attachment: Bug 1547882 - Fix test_bug337744.js that expects no %2f in the query of resource URLs r=kmag!,kershaw

Depends on D30344

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/948aaa76cfe9
Don't unescape %2e and %2f in the query or hash or moz-extension URLs. r=kershaw
https://hg.mozilla.org/integration/autoland/rev/18861619a4b3
Fix test_bug337744.js that expects no %2f in the query of resource URLs r=kmag,kershaw

Comment 15

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/948aaa76cfe9
https://hg.mozilla.org/mozilla-central/rev/18861619a4b3