Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(32)

Issue 1158023004: Added characters that look like padlocks to net IDN character blacklist. (Closed)

Created:
4 years, 4 months ago by Matt Giuca
Modified:
4 years, 4 months ago
Reviewers:
mmenke
CC:
chromium-reviews, cbentzel+watch_chromium.org, jshin+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

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}

Patch Set 1 #

Patch Set 2 : Blacklist the other padlock symbols, and more/fixed tests. #

Total comments: 4

Patch Set 3 : Respond to comments. #

Patch Set 4 : Reword comment. #

Patch Set 5 : Fix compile on Windows. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+34 lines, -12 lines) Patch
M net/base/net_util_icu.cc View 1 2 3 4 2 chunks +13 lines, -10 lines 0 comments Download
M net/base/net_util_icu_unittest.cc View 1 2 3 3 chunks +21 lines, -2 lines 0 comments Download

Messages

Total messages: 19 (6 generated)
Matt Giuca
4 years, 4 months ago (2015-06-04 01:38:23 UTC) #2
mmenke
LGTM. I don't suppose there's some weird "lang" in a "GetExemplarSetForLang" sense that has all ...
4 years, 4 months ago (2015-06-04 02:31:50 UTC) #3
mmenke
On 2015/06/04 02:31:50, mmenke wrote: > LGTM. I don't suppose there's some weird "lang" in ...
4 years, 4 months ago (2015-06-04 02:37:00 UTC) #4
Matt Giuca
On 2015/06/04 02:37:00, mmenke wrote: > LGTM. I don't suppose there's some weird "lang" in ...
4 years, 4 months ago (2015-06-04 04:16:22 UTC) #5
Matt Giuca
I'm just going to go ahead with the padlock only one; maybe we can blacklist ...
4 years, 4 months ago (2015-06-05 03:00:26 UTC) #6
mmenke
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.cc#newcode206 net/base/net_util_icu.cc:206: status); status ...
4 years, 4 months ago (2015-06-05 15:08:06 UTC) #7
Matt Giuca
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.cc#newcode206 net/base/net_util_icu.cc:206: status); Lining up with the previous L is incorrect ...
4 years, 4 months ago (2015-06-09 04:10:35 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1158023004/40001
4 years, 4 months ago (2015-06-09 04:11:44 UTC) #11
commit-bot: I haz the power
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/builds/4379)
4 years, 4 months ago (2015-06-09 04:42:05 UTC) #13
Matt Giuca
Had to fix Windows compile (using \Uxxxxxxxx escapes instead of \uxxxx\uxxxx surrogate pairs).
4 years, 4 months ago (2015-06-09 05:30:24 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1158023004/80001
4 years, 4 months ago (2015-06-09 05:30:49 UTC) #17
commit-bot: I haz the power
Committed patchset #5 (id:80001)
4 years, 4 months ago (2015-06-09 06:31:51 UTC) #18
commit-bot: I haz the power
4 years, 4 months ago (2015-06-09 06:32:43 UTC) #19
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/23285d3e9142d14bb162cd808692deacc5440330
Cr-Commit-Position: refs/heads/master@{#333454}

Powered by Google App Engine
This is Rietveld 408576698