Add fuzz testers to //net for some functions with simple parsing APIs.

Add fuzz testers to //net for some functions with simple parsing APIs.

TBR=jshin@chromium.org

Committed: https://crrev.com/e6264fd9d125c729b60aad95924a6464e6d1b071
Cr-Commit-Position: refs/heads/master@{#378865}

[eroman, 2016-02-26 02:25:14]

Patchset #2 (id:20001) has been deleted

[eroman, 2016-02-26 02:25:25]

Patchset #1 (id:1) has been deleted

[eroman, 2016-02-26 02:37:38]

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

[eroman, 2016-02-26 02:39:02]

Description was changed from

==========
Add fuzz tester to //net for some functions with simple parsing APIs.
==========

to

==========
Add fuzz testers to //net for some functions with simple parsing APIs.
==========

[mmoroz, 2016-02-26 09:18:45]

Thanks for writing these fuzzers. That's awesome!

I left a couple of minor nits. If you don't mind, could you please include
<stddef.h> and <stdint.h> for each fuzzer. We tend to do this to be sure that
|uint8_t| and |size_t| types are always defined, regardless of other headers
included.

LGTM

https://codereview.chromium.org/1735043004/diff/60001/net/base/registry_contr...
File net/base/registry_controlled_domains/get_domain_and_registry_fuzzer.cc
(right):

https://codereview.chromium.org/1735043004/diff/60001/net/base/registry_contr...
net/base/registry_controlled_domains/get_domain_and_registry_fuzzer.cc:27:
size--;
Since we use first byte of |data| and decrease |size|, it may be a good idea to
increase |data| pointer too.

However, if we need only 1 bit here, we also can do something like:
if (size > 1) {
  if ((data[0] ^ data[size - 1]) & 1)
    filter = ...; 
}

Formally speaking, that leaves some 'dependency' between |filter| and |data|
buffer passed to a target function, but we are ok with this.

https://codereview.chromium.org/1735043004/diff/60001/net/base/registry_contr...
net/base/registry_controlled_domains/get_domain_and_registry_fuzzer.cc:32:
std::string input(data, data + size);
Unused variable |input|?

[eroman, 2016-02-26 20:37:19]

https://codereview.chromium.org/1735043004/diff/60001/net/base/registry_contr...
File net/base/registry_controlled_domains/get_domain_and_registry_fuzzer.cc
(right):

https://codereview.chromium.org/1735043004/diff/60001/net/base/registry_contr...
net/base/registry_controlled_domains/get_domain_and_registry_fuzzer.cc:27:
size--;
On 2016/02/26 09:18:45, mmoroz wrote:
> Since we use first byte of |data| and decrease |size|, it may be a good idea
to
> increase |data| pointer too.

Oops yes, that was my intent :)

> 
> However, if we need only 1 bit here, we also can do something like:
> if (size > 1) {
>   if ((data[0] ^ data[size - 1]) & 1)
>     filter = ...; 
> }
> 
> Formally speaking, that leaves some 'dependency' between |filter| and |data|
> buffer passed to a target function, but we are ok with this.

Done.

https://codereview.chromium.org/1735043004/diff/60001/net/base/registry_contr...
net/base/registry_controlled_domains/get_domain_and_registry_fuzzer.cc:32:
std::string input(data, data + size);
On 2016/02/26 09:18:45, mmoroz wrote:
> Unused variable |input|?

Fixed.

[eroman, 2016-02-26 20:38:30]

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

[eroman, 2016-02-26 20:38:31]

+jshin for the change to net/DEPS.

This is just adding a dependency on i18n for the sake of initializing ICU for a
fuzztest.

[mmoroz, 2016-03-01 10:37:07]

On 2016/02/26 20:38:31, eroman wrote:
> +jshin for the change to net/DEPS.
> 
> This is just adding a dependency on i18n for the sake of initializing ICU for
a
> fuzztest.

jshin@, could you please take a look?

[mmenke, 2016-03-02 16:51:43]

mmenke@chromium.org changed reviewers:
+ mmenke@chromium.org

[mmenke, 2016-03-02 16:51:44]

https://codereview.chromium.org/1735043004/diff/100001/net/base/registry_cont...
File net/base/registry_controlled_domains/get_domain_and_registry_fuzzer.cc
(right):

https://codereview.chromium.org/1735043004/diff/100001/net/base/registry_cont...
net/base/registry_controlled_domains/get_domain_and_registry_fuzzer.cc:29: if
((data[0] ^ data[size - 1]) & 1)
So if size is 1, this is always false?  Maybe add a shift to one or the other?

Or could we just do two calls to GetDomainAndRegistry?

[eroman, 2016-03-02 18:28:43]

https://codereview.chromium.org/1735043004/diff/100001/net/base/registry_cont...
File net/base/registry_controlled_domains/get_domain_and_registry_fuzzer.cc
(right):

https://codereview.chromium.org/1735043004/diff/100001/net/base/registry_cont...
net/base/registry_controlled_domains/get_domain_and_registry_fuzzer.cc:29: if
((data[0] ^ data[size - 1]) & 1)
On 2016/03/02 16:51:44, mmenke wrote:
> So if size is 1, this is always false?  Maybe add a shift to one or the other?
> 
> Or could we just do two calls to GetDomainAndRegistry?

Done, made two calls.

[mmenke, 2016-03-02 18:54:29]

On 2016/03/02 18:28:43, eroman wrote:
>
https://codereview.chromium.org/1735043004/diff/100001/net/base/registry_cont...
> File net/base/registry_controlled_domains/get_domain_and_registry_fuzzer.cc
> (right):
> 
>
https://codereview.chromium.org/1735043004/diff/100001/net/base/registry_cont...
> net/base/registry_controlled_domains/get_domain_and_registry_fuzzer.cc:29: if
> ((data[0] ^ data[size - 1]) & 1)
> On 2016/03/02 16:51:44, mmenke wrote:
> > So if size is 1, this is always false?  Maybe add a shift to one or the
other?
> > 
> > Or could we just do two calls to GetDomainAndRegistry?
> 
> Done, made two calls.

No need to wait for my signoff, was just curious about the fetchers you were
working on, from another email.  Thinking I may write a couple myself, at some
point.

[mmoroz, 2016-03-02 19:02:00]

LGTM

[eroman, 2016-03-02 19:04:33]

Cool!

For the record they were very easy to write.

With the exception of WebCrypto having a more challenging interface, since for
key import the parsing data is tightly coupled to the expected parameters.

That said, I haven't actually turned up any bugs yet. Not sure if that is
because the code under test is high quality, or the tests are insufficient...

I will probably experiment with uploading some sample inputs and/or dictionaries
to help the fuzz driver and see if that leads to success :)

[eroman, 2016-03-02 20:35:13]

Description was changed from

==========
Add fuzz testers to //net for some functions with simple parsing APIs.
==========

to

==========
Add fuzz testers to //net for some functions with simple parsing APIs.

TBR=jshin@chromium.org
==========

[eroman, 2016-03-02 20:40:55]

TBR=jshin@chromium.org

For the the change to net/BUILD.gn, which adds a dependency on base/i18n for an
individual test.

I can rollout that test or rework it if you are unhappy from that change, but I
believe it is consistent with what other tests are doing so reasonable to TBR.

[eroman, 2016-03-02 20:41:21]

The CQ bit was checked by eroman@chromium.org

[eroman, 2016-03-02 20:41:22]

The patchset sent to the CQ was uploaded after l-g-t-m from mmoroz@chromium.org
Link to the patchset: https://codereview.chromium.org/1735043004/#ps140001
(title: "rebase")

[commit-bot: I haz the power, 2016-03-02 20:41:46]

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

[commit-bot: I haz the power, 2016-03-02 22:46:44]

Description was changed from

==========
Add fuzz testers to //net for some functions with simple parsing APIs.

TBR=jshin@chromium.org
==========

to

==========
Add fuzz testers to //net for some functions with simple parsing APIs.

TBR=jshin@chromium.org
==========

[commit-bot: I haz the power, 2016-03-02 22:46:45]

Committed patchset #6 (id:140001)

[commit-bot: I haz the power, 2016-03-02 22:48:55]

Description was changed from

==========
Add fuzz testers to //net for some functions with simple parsing APIs.

TBR=jshin@chromium.org
==========

to

==========
Add fuzz testers to //net for some functions with simple parsing APIs.

TBR=jshin@chromium.org

Committed: https://crrev.com/e6264fd9d125c729b60aad95924a6464e6d1b071
Cr-Commit-Position: refs/heads/master@{#378865}
==========

[commit-bot: I haz the power, 2016-03-02 22:48:56]

Patchset 6 (id:??) landed as
https://crrev.com/e6264fd9d125c729b60aad95924a6464e6d1b071
Cr-Commit-Position: refs/heads/master@{#378865}