1825179 - Some canonicalization should still apply to query string

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect, P2)

Description

[Hector Zhao [:hectorz]]

Hi, we're looking at some other URLs in our safe browsing list but not blocked in Fx, and I think there's a client side bug from the previous fix.

In https://developers.google.com/safe-browsing/v4/urls-hashing?hl=en#canonicalization, when talking about "Do not apply these path canonicalizations to the query parameters" I think it's only referring to the two slash related ones above the sentence, while the "percent-unescape" above and "percent-escape" below that part should still apply.

While the fix in https://hg.mozilla.org/mozilla-central/rev/2e19e69edfcb means the query string will be returned by GetKeyForURI w/o going through any of that.

Comment 1

[Hector Zhao [:hectorz]]

As an example, while both https://wiki.mozilla.org/index.php?title=MozillaWiki:Help&action=history and https://wiki.mozilla.org/index.php?title=MozillaWiki%3AHelp&action=history can reach the same page,

» let urlUtils = Cc["@mozilla.org/url-classifier/utils;1"].getService(Ci.nsIUrlClassifierUtils);
← <XPCWrappedNative_NoHelper ...>
» urlUtils.getKeyForURI(Services.io.newURI("https://wiki.mozilla.org/index.php?title=MozillaWiki:Help&action=history"))
← "wiki.mozilla.org/index.php?title=MozillaWiki:Help&action=history"
» urlUtils.getKeyForURI(Services.io.newURI("https://wiki.mozilla.org/index.php?title=MozillaWiki%3AHelp&action=history"))
← "wiki.mozilla.org/index.php?title=MozillaWiki%3AHelp&action=history"

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1676804

:dimi, since you are the author of the regressor, bug 1676804, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1676804

Comment 4

[Harshit Sohaney [:hsohaney]]

Could you clarify the example case the reporter was mentioning. It reads incomplete right now, am I missing something?

Comment 5

[Harshit Sohaney [:hsohaney]]

Attachment: Bug 1825179 - Added canonicalization to query parameters for Google Safe Browsing. r?timhuang

Comment 6

[Harshit Sohaney [:hsohaney]]

From discussion with :timhuang, the query parameters currently are not being canonicalized as per the algorithm in https://developers.google.com/safe-browsing/v4/urls-hashing?hl=en#canonicalization

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/28b4eaeb7028
Added canonicalization to query parameters for Google Safe Browsing. r=timhuang

Comment 8

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/28b4eaeb7028