1502097 - Figure out a better way of defining the network.IDN.blacklist_chars pref

Status: RESOLVED FIXED

(Core :: Networking, enhancement, P3)

Description

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

Right now the blacklist is defined as a pref in all.js and has around 200 characters.
Bug 1349316 would require us adding about 200 more.
Moreover, I don't like that the list has no proper structure.
It may be a good idea to define them in StaticPrefList.h instead.
Another question is if we want it to be a list of individual blacklisted characters, or a list of blacklisted ranges of characters.

Comment 1

[Jonathan Kingston [:jkt] he/him]

Can we rename the prefs blocklist as part of this change?

Comment 2

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

(In reply to Jonathan Kingston [:jkt] from comment #1)
> Can we rename the prefs blocklist as part of this change?

We could. Any specific reasons to rename?

Comment 3

[Jonathan Kingston [:jkt] he/him]

As per https://bugzilla.mozilla.org/show_bug.cgi?id=1149485 many things within standards etc are trying to move from black/whitelist to block/allowlist.

See also http://www.agmweb.ca/2015-01-07-allowed/

Comment 4

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

(In reply to Jonathan Kingston [:jkt] from comment #3)
> As per https://bugzilla.mozilla.org/show_bug.cgi?id=1149485 many things
> within standards etc are trying to move from black/whitelist to
> block/allowlist.
> See also http://www.agmweb.ca/2015-01-07-allowed/

I wasn't aware of that. Thanks for pointing it out. I'll make sure to change the pref names too.

Comment 5

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9d55658005d063e5b7d26ba17b25766483072dac

Comment 6

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

Attachment: Bug 1502097 - (Part 1) Move pref network.IDN.blacklist_chars to separate hardcoded file IDNCharacterBlocklist.inc

* Moves the value of the pref and also the fallback definition in nsTextToSubURI.cpp to a separate file.
* The file has better formatting, so we may follow its history more easily. Each range of consecutive values is defined on a separate line.
* Renames `blacklist` to `blocklist` for pref and variable names (for this individual pref. network.IDN.whitelist.* needs to be handled in a separate bug)
* Changes nsIDNService::mIDNBlocklist from being an nsString to sorted nsTArray<char16> and uses mozilla::BinarySearch() to check for characters.

Comment 7

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

Attachment: Bug 1502097 - (Part 2) Define IDN blocklist as ranges of characters [ {firstChar, lastChar}* ]

* Changes the format of the blocklist from a list of characters to a list of
  character ranges. Binary search still works, and it is easier to include
  large ranges of characters in the blocklist.
* Moves logic for handling the blocklist to IDNBlocklistUtils.h/.cpp
* Changes NS_EscapeURL to take a function that determines if a character
  is blocked. This way the type of the array doesn't matter.

Depends on D12209

Comment 8

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6410da90cf8cf1bd089a6b0fca562152200a5a0

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ed995224a05a
(Part 1) Move pref network.IDN.blacklist_chars to separate hardcoded file IDNCharacterBlocklist.inc r=jfkthame,dragana
https://hg.mozilla.org/integration/autoland/rev/6040483f1f0a
(Part 2) Define IDN blocklist as ranges of characters [ {firstChar, lastChar}* ] r=jfkthame,dragana

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/ed995224a05a
https://hg.mozilla.org/mozilla-central/rev/6040483f1f0a