Fail DNS resolution if the result contains 127.0.53.53.

Fail DNS resolution if the result contains 127.0.53.53.

ICANN designated the IPv4 address 127.0.53.53 as meaning a name collision occurred during resolution (as a way to discover real world conflicts around the new gTLDs).

This change makes such resolutions now fail with:
  ICANN_NAME_COLLISION.

IP literals are exempt from the policy and will continue to work as before (meaning you can still navigate to http://127.0.53.53).

This only affects IPv4 results.

BUG=470704
Committed: https://crrev.com/91dd3606d627036287f32bb449b09c170a0765cf
Cr-Commit-Position: refs/heads/master@{#322309}

[eroman, 2015-03-26 00:01:21]

eroman@chromium.org changed reviewers:
+ rsleevi@chromium.org

[eroman, 2015-03-26 00:01:21]

This applies to both the system host resolver, and the chrome dns resolver
(however I do not yet have tests for the chrome resolver).

[mmenke, 2015-03-26 00:17:40]

On 2015/03/26 00:01:21, eroman wrote:
> This applies to both the system host resolver, and the chrome dns resolver
> (however I do not yet have tests for the chrome resolver).

I'm assuming we should add error text for this, too?

[eroman, 2015-03-26 00:32:14]

@mmenke: Correct. I was planning on sending that as a separate CL

[Ryan Sleevi, 2015-03-26 00:32:30]

https://codereview.chromium.org/1035803003/diff/1/net/dns/host_resolver_impl.cc
File net/dns/host_resolver_impl.cc (right):

https://codereview.chromium.org/1035803003/diff/1/net/dns/host_resolver_impl....
net/dns/host_resolver_impl.cc:729: error = ERR_INTERNET_DISCONNECTED;
Well this is weird -_- Swallows error codes.

https://codereview.chromium.org/1035803003/diff/1/net/dns/host_resolver_impl_...
File net/dns/host_resolver_impl_unittest.cc (right):

https://codereview.chromium.org/1035803003/diff/1/net/dns/host_resolver_impl_...
net/dns/host_resolver_impl_unittest.cc:1336: EXPECT_EQ(ERR_FAILED,
request->WaitForResult());
We don't give a better error here? To ensure that it gets the ICANN flag?

[eroman, 2015-03-26 00:39:31]

https://codereview.chromium.org/1035803003/diff/1/net/dns/host_resolver_impl_...
File net/dns/host_resolver_impl_unittest.cc (right):

https://codereview.chromium.org/1035803003/diff/1/net/dns/host_resolver_impl_...
net/dns/host_resolver_impl_unittest.cc:1336: EXPECT_EQ(ERR_FAILED,
request->WaitForResult());
On 2015/03/26 00:32:29, Ryan Sleevi wrote:
> We don't give a better error here? To ensure that it gets the ICANN flag?

d'oh, forgot to update the unittest after adding the error code.
Fixed.

[Ryan Sleevi, 2015-03-26 00:44:36]

lgtm

https://codereview.chromium.org/1035803003/diff/20001/net/dns/host_resolver_i...
File net/dns/host_resolver_impl_unittest.cc (right):

https://codereview.chromium.org/1035803003/diff/20001/net/dns/host_resolver_i...
net/dns/host_resolver_impl_unittest.cc:1329:
proc_->AddRuleForAllFamilies("ipv6", "::127.0.53.53");
I can't quite put my finger on why, but is it useful to also specify 53.53.0.127
and make sure it doesn't error out? I can never quite remember the
representational rules and whether we're guaranteed to preserve wire order
(which I would assume is true)

This may be a bad idea though

[eroman, 2015-03-26 00:55:08]

https://codereview.chromium.org/1035803003/diff/20001/net/dns/host_resolver_i...
File net/dns/host_resolver_impl_unittest.cc (right):

https://codereview.chromium.org/1035803003/diff/20001/net/dns/host_resolver_i...
net/dns/host_resolver_impl_unittest.cc:1329:
proc_->AddRuleForAllFamilies("ipv6", "::127.0.53.53");
On 2015/03/26 00:44:36, Ryan Sleevi wrote:
> I can't quite put my finger on why, but is it useful to also specify
53.53.0.127
> and make sure it doesn't error out? I can never quite remember the
> representational rules and whether we're guaranteed to preserve wire order
> (which I would assume is true)
> 
> This may be a bad idea though

Done. More tests is almost never a bad idea!

Added:
  53.53.0.127
  127.0.53.54
  10.0.53.53

[eroman, 2015-03-26 03:00:30]

The CQ bit was checked by eroman@chromium.org

[eroman, 2015-03-26 03:00:30]

The patchset sent to the CQ was uploaded after l-g-t-m from rsleevi@chromium.org
Link to the patchset: https://codereview.chromium.org/1035803003/#ps40001
(title: "Add 3 more test cases")

[commit-bot: I haz the power, 2015-03-26 03:01:01]

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

[commit-bot: I haz the power, 2015-03-26 03:46:39]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-26 03:47:51]

Patchset 3 (id:??) landed as
https://crrev.com/91dd3606d627036287f32bb449b09c170a0765cf
Cr-Commit-Position: refs/heads/master@{#322309}