Added characters that look like padlocks to net IDN character blacklist.

Added characters that look like padlocks to net IDN character blacklist.

This adds the following Unicode characters to the blacklist:
- U+1F50F LOCK WITH INK PEN
- U+1F510 CLOSED LOCK WITH KEY
- U+1F512 LOCK
- U+1F513 OPEN LOCK

This prevents LOCK characters from appearing in an internationalized
domain names, potentially looking like an SSL padlock icon (e.g.,
"🔒google.com" should be rewritten as "xn--google-hj64e.com"). NOTE: This
is just a precaution; IsIDNComponentSafe will generally already not
allow the LOCK character because it is not considered a character in any
language. However, it would previously allow it if the set of languages
was empty. Now it is explicitly blacklisted.

BUG=495934
Committed: https://crrev.com/23285d3e9142d14bb162cd808692deacc5440330
Cr-Commit-Position: refs/heads/master@{#333454}

[Matt Giuca, 2015-06-04 01:38:23]

mgiuca@chromium.org changed reviewers:
+ mmenke@chromium.org

[mmenke, 2015-06-04 02:31:50]

LGTM.  I don't suppose there's some weird "lang" in a "GetExemplarSetForLang"
sense that has all the weird symbol characters that we could just blacklist?

[mmenke, 2015-06-04 02:37:00]

On 2015/06/04 02:31:50, mmenke wrote:
> LGTM.  I don't suppose there's some weird "lang" in a "GetExemplarSetForLang"
> sense that has all the weird symbol characters that we could just blacklist?

Just so we're on the same page:  You suggest blacklisting the symbol from
display to the user at all, but this only affects it when in the domain name,
right?

[Matt Giuca, 2015-06-04 04:16:22]

On 2015/06/04 02:37:00, mmenke wrote:
> LGTM.  I don't suppose there's some weird "lang" in a "GetExemplarSetForLang"
> sense that has all the weird symbol characters that we could just blacklist?

We could blacklist all characters in the "Symbol, Other" category
(http://www.fileformat.info/info/unicode/category/So/list.htm); see the
discussion on the bug. I'm not sure we want to do that.

> Just so we're on the same page:  You suggest blacklisting the symbol from
> display to the user at all, but this only affects it when in the domain name,
> right?

Yes, this only affects the domain name. The other part of this (less important
from a spoofing perspective) is blocking it from the rest of the URL by
replacing it with its UTF-8 escape form, %F0%9F%94%92. I don't attempt that
here.

[Matt Giuca, 2015-06-05 03:00:26]

I'm just going to go ahead with the padlock only one; maybe we can blacklist all
the symbols later on.

Note: I had to bypass hooks to avoid the uploader complaining about
ClangFormatting the blacklist *and* tests. I do not want to have to reformat all
of that so I'm hoping you (and the presubmit script) will let me get away with
it.

PTAL. (Blacklisted all four padlock characters U+1F50F, U+1F510, U+1F512 and
U+1F513.)

[mmenke, 2015-06-05 15:08:06]

LGTM.  Fine bypassing gl format for these.

https://codereview.chromium.org/1158023004/diff/20001/net/base/net_util_icu.cc
File net/base/net_util_icu.cc (right):

https://codereview.chromium.org/1158023004/diff/20001/net/base/net_util_icu.c...
net/base/net_util_icu.cc:206: status);
status is indented strangely here - should just line up with the previous L
(Could try and reformat to make other things clearer, but as-is, this
indentation is weird).

https://codereview.chromium.org/1158023004/diff/20001/net/base/net_util_icu_u...
File net/base/net_util_icu_unittest.cc (right):

https://codereview.chromium.org/1158023004/diff/20001/net/base/net_util_icu_u...
net/base/net_util_icu_unittest.cc:342: // blacklisted all strings with the
surrogate '\xdd12'.
nit:  Don't use "we" in comments

[Matt Giuca, 2015-06-09 04:10:35]

https://codereview.chromium.org/1158023004/diff/20001/net/base/net_util_icu.cc
File net/base/net_util_icu.cc (right):

https://codereview.chromium.org/1158023004/diff/20001/net/base/net_util_icu.c...
net/base/net_util_icu.cc:206: status);
Lining up with the previous L is incorrect (it would imply that 'status' is a
parameter to UnicodeString, which it isn't).

Just reformatted this line.

https://codereview.chromium.org/1158023004/diff/20001/net/base/net_util_icu_u...
File net/base/net_util_icu_unittest.cc (right):

https://codereview.chromium.org/1158023004/diff/20001/net/base/net_util_icu_u...
net/base/net_util_icu_unittest.cc:342: // blacklisted all strings with the
surrogate '\xdd12'.
Hmm, not a fan of this rule (it means rewriting active sentences as passive
sentences, and this one is a pretty bad one). But OK.

Also, the word "we" already appears immediately above, and 17 times in
net_util_icu.cc.

[Matt Giuca, 2015-06-09 04:10:40]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2015-06-09 04:10:40]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/1158023004/#ps40001
(title: "Respond to comments.")

[commit-bot: I haz the power, 2015-06-09 04:11:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1158023004/40001

[commit-bot: I haz the power, 2015-06-09 04:42:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-09 04:42:05]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[Matt Giuca, 2015-06-09 05:30:24]

Had to fix Windows compile (using \Uxxxxxxxx escapes instead of \uxxxx\uxxxx
surrogate pairs).

[Matt Giuca, 2015-06-09 05:30:36]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2015-06-09 05:30:36]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/1158023004/#ps80001
(title: "Fix compile on Windows.")

[commit-bot: I haz the power, 2015-06-09 05:30:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1158023004/80001

[commit-bot: I haz the power, 2015-06-09 06:31:51]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-06-09 06:32:43]

Patchset 5 (id:??) landed as
https://crrev.com/23285d3e9142d14bb162cd808692deacc5440330
Cr-Commit-Position: refs/heads/master@{#333454}