Fix ParseCIDRBlock() to no longer accept invalid inputs.

Fix ParseCIDRBlock() to no longer accept invalid inputs.

ParseCIDRBlock() currently accepts invalid inputs when the prefix contains a leading plus sign: "192.168.1.1/+1". This CL updates ParseCIDRBlock() to reject these inputs as invalid.

BUG=596258
Committed: https://crrev.com/8a14cbc731ec72162def8a3122a91ce1a235752b
Cr-Commit-Position: refs/heads/master@{#382366}

[martijnc, 2016-03-20 21:22:43]

Patchset #1 (id:1) has been deleted

[martijnc, 2016-03-20 22:48:03]

Patchset #2 (id:40001) has been deleted

[martijnc, 2016-03-21 16:31:37]

Description was changed from

==========
Fix ParseCIDRBlock() to no longer accept invalid inputs.

BUG=596258
==========

to

==========
Fix ParseCIDRBlock() to no longer accept invalid inputs.

ParseCIDRBlock() currently accepts invalid inputs where the prefix contains a
leading plus sign: "192.168.1.1/+1". This CL updates ParseCIDRBlock() to reject
these inputs as invalid.

BUG=596258
==========

[martijnc, 2016-03-21 16:31:47]

martijn@martijnc.be changed reviewers:
+ eroman@chromium.org

[martijnc, 2016-03-21 16:32:08]

Description was changed from

==========
Fix ParseCIDRBlock() to no longer accept invalid inputs.

ParseCIDRBlock() currently accepts invalid inputs where the prefix contains a
leading plus sign: "192.168.1.1/+1". This CL updates ParseCIDRBlock() to reject
these inputs as invalid.

BUG=596258
==========

to

==========
Fix ParseCIDRBlock() to no longer accept invalid inputs.

ParseCIDRBlock() currently accepts invalid inputs when the prefix contains a
leading plus sign: "192.168.1.1/+1". This CL updates ParseCIDRBlock() to reject
these inputs as invalid.

BUG=596258
==========

[martijnc, 2016-03-21 16:49:54]

This is a small follow up CL to fix ParseCIDRBlock(). Added a simple check for
the leading '+'.

Can you take a look? Thanks!

[eroman, 2016-03-21 17:28:31]

lgtm

https://codereview.chromium.org/1816903002/diff/60001/net/base/ip_address.cc
File net/base/ip_address.cc (right):

https://codereview.chromium.org/1816903002/diff/60001/net/base/ip_address.cc#...
net/base/ip_address.cc:161: const base::StringPiece& prefix_length = parts[1];
Can you add a TODO linking to crbug.com/596523 ?

This is a fine solution for this instance, but I think we want to make a bigger
change to address the other cases.

[martijnc, 2016-03-21 18:25:50]

Patchset #3 (id:80001) has been deleted

[martijnc, 2016-03-21 18:29:04]

Thank you.

https://codereview.chromium.org/1816903002/diff/60001/net/base/ip_address.cc
File net/base/ip_address.cc (right):

https://codereview.chromium.org/1816903002/diff/60001/net/base/ip_address.cc#...
net/base/ip_address.cc:161: const base::StringPiece& prefix_length = parts[1];
On 2016/03/21 at 17:28:31, eroman wrote:
> Can you add a TODO linking to crbug.com/596523 ?
> 
> This is a fine solution for this instance, but I think we want to make a
bigger change to address the other cases.

Done.

[martijnc, 2016-03-21 19:10:34]

Would it be better (since this isn't a regression) to hold of landing this until
there is a better, more general solution for this issue?

[eroman, 2016-03-21 19:13:40]

Since you have already written and tested this, I am fine with landing it. More
tests == good :)

There are other places in the codebase (not just this one) which could be
unified with a more general solution, so we will need to do fixups later
regardless.

As long as there is a TODO to help guide that works, seems safe to land.

[martijnc, 2016-03-21 19:17:42]

The CQ bit was checked by martijn@martijnc.be

[martijnc, 2016-03-21 19:17:43]

The patchset sent to the CQ was uploaded after l-g-t-m from eroman@chromium.org
Link to the patchset: https://codereview.chromium.org/1816903002/#ps100001
(title: "added a TODO")

[commit-bot: I haz the power, 2016-03-21 19:18:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1816903002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1816903002/100001

[commit-bot: I haz the power, 2016-03-21 20:11:31]

Committed patchset #3 (id:100001)

[commit-bot: I haz the power, 2016-03-21 20:12:48]

Description was changed from

==========
Fix ParseCIDRBlock() to no longer accept invalid inputs.

ParseCIDRBlock() currently accepts invalid inputs when the prefix contains a
leading plus sign: "192.168.1.1/+1". This CL updates ParseCIDRBlock() to reject
these inputs as invalid.

BUG=596258
==========

to

==========
Fix ParseCIDRBlock() to no longer accept invalid inputs.

ParseCIDRBlock() currently accepts invalid inputs when the prefix contains a
leading plus sign: "192.168.1.1/+1". This CL updates ParseCIDRBlock() to reject
these inputs as invalid.

BUG=596258

Committed: https://crrev.com/8a14cbc731ec72162def8a3122a91ce1a235752b
Cr-Commit-Position: refs/heads/master@{#382366}
==========

[commit-bot: I haz the power, 2016-03-21 20:12:49]

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