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

Issue 2683793010: Block domain labels made of Cyrillic letters that look alike Latin (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months, 1 week ago by jungshik at Google
Modified:
6 months, 3 weeks ago
Reviewers:
lgarron, Peter Kasting, sffc
CC:
markdavis, sffc, emilyschechter
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Block domain labels made of Cyrillic letters that look alike Latin Block a label made entirely of Latin-look-alike Cyrillic letters when the TLD is not an IDN (i.e. this check is ON only for TLDs like 'com', 'net', 'uk', but not applied for IDN TLDs like рф. BUG=683314 TEST=components_unittests --gtest_filter=U*IDN* Review-Url: https://codereview.chromium.org/2683793010 Cr-Commit-Position: refs/heads/master@{#459226} Committed: https://chromium.googlesource.com/chromium/src/+/08cb718ba7c3961c1006176c9faba0a5841ec792

Patch Set 1 #

Patch Set 2 : check Cyrl-Latn alikes only for non-IDN tlds #

Total comments: 11

Patch Set 3 : exclude non-Letter when checking against Cyrl-Latn look-alikes #

Patch Set 4 : Streamline isTLD ASCII check #

Patch Set 5 : Add ӕъЬвҽвԍнкпгмтцѵѡү to the list of Cyr-Lat alike set #

Patch Set 6 : Do not include "ӕ вҽвԍн мтц" in the Cyr-Lat alike list #

Patch Set 7 : Do not include "ҽк ѡү" in Cyr-Lat alike list #

Patch Set 8 : Add back ҽ and ѡ to the Cyr-Lat alike-set #

Patch Set 9 : a bit more tweaking; no change in the blocked set #

Patch Set 10 : add a test with U+00B7 and Latin-alike Cyrillics #

Total comments: 8

Patch Set 11 : address review comments by Peter #

Patch Set 12 : remove unncessary check #

Patch Set 13 : go back to ps11 #

Messages

Total messages: 25 (9 generated)
jungshik at Google
8 months ago (2017-02-15 18:47:55 UTC) #3
jungshik at Google
https://codereview.chromium.org/2683793010/diff/20001/components/url_formatter/url_formatter.cc File components/url_formatter/url_formatter.cc (right): https://codereview.chromium.org/2683793010/diff/20001/components/url_formatter/url_formatter.cc#newcode331 components/url_formatter/url_formatter.cc:331: icu::UnicodeString("[аеорсухьѕіјһмӏтнв]"), status); "м, т, н, в" look like smallcap ...
8 months ago (2017-02-15 18:50:21 UTC) #4
sffc
https://codereview.chromium.org/2683793010/diff/20001/components/url_formatter/url_formatter.cc File components/url_formatter/url_formatter.cc (right): https://codereview.chromium.org/2683793010/diff/20001/components/url_formatter/url_formatter.cc#newcode209 components/url_formatter/url_formatter.cc:209: is_tld_ascii = false; I don't really understand what this ...
8 months ago (2017-02-15 19:57:31 UTC) #6
lgarron
https://codereview.chromium.org/2683793010/diff/20001/components/url_formatter/url_formatter.cc File components/url_formatter/url_formatter.cc (right): https://codereview.chromium.org/2683793010/diff/20001/components/url_formatter/url_formatter.cc#newcode331 components/url_formatter/url_formatter.cc:331: icu::UnicodeString("[аеорсухьѕіјһмӏтнв]"), status); Also consider ԁ, Ӏ, and maybe ѵ, ...
8 months ago (2017-02-15 20:24:49 UTC) #8
lgarron
https://codereview.chromium.org/2683793010/diff/20001/components/url_formatter/url_formatter.cc File components/url_formatter/url_formatter.cc (right): https://codereview.chromium.org/2683793010/diff/20001/components/url_formatter/url_formatter.cc#newcode331 components/url_formatter/url_formatter.cc:331: icu::UnicodeString("[аеорсухьѕіјһмӏтнв]"), status); On 2017/02/15 at 20:24:49, lgarron wrote: > ...
8 months ago (2017-02-15 20:25:56 UTC) #9
jungshik at Google
https://codereview.chromium.org/2683793010/diff/20001/components/url_formatter/url_formatter.cc File components/url_formatter/url_formatter.cc (right): https://codereview.chromium.org/2683793010/diff/20001/components/url_formatter/url_formatter.cc#newcode209 components/url_formatter/url_formatter.cc:209: is_tld_ascii = false; On 2017/02/15 19:57:30, sffc wrote: > ...
8 months ago (2017-02-15 20:55:52 UTC) #10
lgarron
*ping* (Motivated by a bug that uses Turkish, although it's not a perfect spoof like ...
6 months, 4 weeks ago (2017-03-21 20:45:41 UTC) #11
jungshik at Google
Something is not right here. My reply to comments are not included.
6 months, 4 weeks ago (2017-03-21 21:51:59 UTC) #12
jungshik at Google
On 2017/03/21 21:51:59, jungshik at Google wrote: > Something is not right here. My reply ...
6 months, 4 weeks ago (2017-03-21 22:45:45 UTC) #13
jungshik at Google
Can you take a look? I'll not try to fine-tune a Latin-alike Cyrillic set to ...
6 months, 4 weeks ago (2017-03-22 19:10:59 UTC) #15
Peter Kasting
LGTM https://codereview.chromium.org/2683793010/diff/180001/components/url_formatter/url_formatter.cc File components/url_formatter/url_formatter.cc (right): https://codereview.chromium.org/2683793010/diff/180001/components/url_formatter/url_formatter.cc#newcode253 components/url_formatter/url_formatter.cc:253: // Returns true if |label| is safe to ...
6 months, 4 weeks ago (2017-03-22 22:14:08 UTC) #16
jungshik at Google
Thanks, Peter for the review. What's the best practice of landing a 'private' CL? I'm ...
6 months, 3 weeks ago (2017-03-23 20:10:17 UTC) #17
lgarron
On 2017/03/23 at 20:10:17, jshin wrote: > Thanks, Peter for the review. > > What's ...
6 months, 3 weeks ago (2017-03-23 20:12:12 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2683793010/240001
6 months, 3 weeks ago (2017-03-23 20:13:56 UTC) #21
jungshik at Google
On 2017/03/23 20:12:12, lgarron wrote: > On 2017/03/23 at 20:10:17, jshin wrote: > > Thanks, ...
6 months, 3 weeks ago (2017-03-23 20:14:00 UTC) #22
commit-bot: I haz the power
6 months, 3 weeks ago (2017-03-23 21:26:23 UTC) #25
Message was sent while issue was closed.
Committed patchset #13 (id:240001) as
https://chromium.googlesource.com/chromium/src/+/08cb718ba7c3961c1006176c9fab...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 81bcdb8aa