Disallow navigations to ccTLDs from ChromeOmniboxNavigationObserver |
|||||
Issue descriptionSplit from bug 479620. ChromeOmniboxNavigationObserver should check whether the alternate match's destination is a ccTLD per the eTLD service. If so, it should not send it a ping. See comment 46 of the original bug.
,
Nov 30 2016
Thanks for the summary. I believe #2 may not be a strict superset of #1, if a TLD's rules are something like "foo.xx / bar.xx" (note lack of inclusion of "xx" itself). Then "xx" isn't an eTLD and would be allowed by #2, but not by #1. It depends on the implementation. (Also, if this difference _does_ occur, I don't know if that's a good or a bad thing.) I think given the potential downsides here, it would be nice to pick up the wins of #2/3 so bad or incompetent actors don't get user data even briefly. It also seems like coding #2 would be easier than #1, as it's likely a single (existing) function call in a place that is not perf-critical, whereas to build #1 we'd actually need to enumerate the real country codes (just assuming any two-character name is a country code would break non-cc intranets). The reason I made the comments about "you seem to be saying that anything that clashes with a TLD will be broken now or later" is that, if that's true, it reduces the motivation to do #3, sufficiently such that I'm not inclined to do it. Accordingly, it seems like we should do #2, and potentially worry about the "strict superset" case.
,
Nov 30 2016
.xx is supposed to be covered in the PSL as part of the PSL having all of the root zone data (which would include both IANA and ICANN delegations, but not reserved domains) But I agree with #2 as both the easiest and most pragmatic case, though acknowledging it will cause some (intended) breakage.
,
Feb 14 2017
rsleevi -- should this really be P1? I'm trying to decide whether to punt it off my plate into the general pool.
,
Feb 14 2017
Given the state of Issue 479620, it probably is a P2
,
Feb 14 2017
,
Jun 22 2017
If no one has touched this bug in five months, is it really a P2? If it is, someone should say so now so the appropriate fixers (whoever they may be) know to allocate time to fix it this coming quarter.
,
Jan 23 2018
,
Feb 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0b180e48e0c416729c0913bc41574a1c9b874306 commit 0b180e48e0c416729c0913bc41574a1c9b874306 Author: Kevin Bailey <krb@chromium.org> Date: Sat Feb 03 03:41:04 2018 [omnibox] Don't ping TLD for alternate nav query We "ping" (with http) queries that might be host names. Some TLDs respond as hosts. Filter these out from pinging. (Assume the user would not want to navigate there.) Bug: 669785 Change-Id: I11905edf854e52f346048f7f705acab913d02984 Reviewed-on: https://chromium-review.googlesource.com/886562 Commit-Queue: Kevin Bailey <krb@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#534253} [modify] https://crrev.com/0b180e48e0c416729c0913bc41574a1c9b874306/components/omnibox/browser/autocomplete_input.cc [modify] https://crrev.com/0b180e48e0c416729c0913bc41574a1c9b874306/components/omnibox/browser/autocomplete_input_unittest.cc [modify] https://crrev.com/0b180e48e0c416729c0913bc41574a1c9b874306/net/base/registry_controlled_domains/registry_controlled_domain.cc [modify] https://crrev.com/0b180e48e0c416729c0913bc41574a1c9b874306/net/base/registry_controlled_domains/registry_controlled_domain.h [modify] https://crrev.com/0b180e48e0c416729c0913bc41574a1c9b874306/net/base/registry_controlled_domains/registry_controlled_domain_unittest.cc
,
Feb 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b0d920d494329b2f6f40704c3b0e5f8733da35dd commit b0d920d494329b2f6f40704c3b0e5f8733da35dd Author: Kevin Bailey <krb@chromium.org> Date: Thu Feb 08 02:18:14 2018 Revert "[omnibox] Don't ping TLD for alternate nav query" This reverts commit 0b180e48e0c416729c0913bc41574a1c9b874306. Reason for revert: The change is causing DCHECKs, and blank Omnibox suggestions in production code. The fix is taking longer than expected. Original change's description: > [omnibox] Don't ping TLD for alternate nav query > > We "ping" (with http) queries that might be host names. Some TLDs > respond as hosts. Filter these out from pinging. (Assume the user > would not want to navigate there.) > > Bug: 669785 > Change-Id: I11905edf854e52f346048f7f705acab913d02984 > Reviewed-on: https://chromium-review.googlesource.com/886562 > Commit-Queue: Kevin Bailey <krb@chromium.org> > Reviewed-by: Peter Kasting <pkasting@chromium.org> > Cr-Commit-Position: refs/heads/master@{#534253} TBR=pkasting@chromium.org,krb@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 669785 Change-Id: If065e96368cd81d94c0de6ad98451b4e8b21c9db Reviewed-on: https://chromium-review.googlesource.com/906973 Reviewed-by: Kevin Bailey <krb@chromium.org> Commit-Queue: Kevin Bailey <krb@chromium.org> Cr-Commit-Position: refs/heads/master@{#535259} [modify] https://crrev.com/b0d920d494329b2f6f40704c3b0e5f8733da35dd/components/omnibox/browser/autocomplete_input.cc [modify] https://crrev.com/b0d920d494329b2f6f40704c3b0e5f8733da35dd/components/omnibox/browser/autocomplete_input_unittest.cc [modify] https://crrev.com/b0d920d494329b2f6f40704c3b0e5f8733da35dd/net/base/registry_controlled_domains/registry_controlled_domain.cc [modify] https://crrev.com/b0d920d494329b2f6f40704c3b0e5f8733da35dd/net/base/registry_controlled_domains/registry_controlled_domain.h [modify] https://crrev.com/b0d920d494329b2f6f40704c3b0e5f8733da35dd/net/base/registry_controlled_domains/registry_controlled_domain_unittest.cc
,
Feb 13 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by pkasting@chromium.org
, Nov 30 2016