Immediately reject empty cookie headers.

Immediately reject empty cookie headers.

BUG=54785
TEST=DoNotSaveEmptyCookies

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=60621

[jochen (gone - plz use gerrit), 2010-09-24 11:56:34]

please review

[Randy Smith (Not in Mondays), 2010-09-24 12:54:56]

On 2010/09/24 11:56:34, jochen wrote:
> please review

This change LGTM; it's probably the right fix for the crash.  In an ideal world,
EnumerateHeaders would specify in the comment what the behavior was on a null
header (my assumption looking at the code would be enumerating over a null list,
which wouldn't have caused this bug), but I think that's probably out of scope
for this change.

I'd also like to put some tests and protection at the cookie monster level for
this--I don't like code crashing based on bad inputs.  Do you have a sense as to
what this input will look like when it gets down to the CM?  Just a zero length
name and value string?  I want to make sure that doesn't crash the system.

[jochen (gone - plz use gerrit), 2010-09-24 13:00:14]

On 2010/09/24 12:54:56, rdsmith wrote:
> On 2010/09/24 11:56:34, jochen wrote:
> > please review
> 
> This change LGTM; it's probably the right fix for the crash.  In an ideal
world,
> EnumerateHeaders would specify in the comment what the behavior was on a null
> header (my assumption looking at the code would be enumerating over a null
list,
> which wouldn't have caused this bug), but I think that's probably out of scope
> for this change.
> 
> I'd also like to put some tests and protection at the cookie monster level for
> this--I don't like code crashing based on bad inputs.  Do you have a sense as
to
> what this input will look like when it gets down to the CM?  Just a zero
length
> name and value string?  I want to make sure that doesn't crash the system.

It's an empty cookie line, i.e. ""

the cookie monster changed a lot since then, and I couldn't get it to crash with
a ToT build when passing an empty cookie line.

let's wait what Darin thinks?

[darin (slow to review), 2010-09-24 15:35:28]

LGTM

There are legit use cases for headers that have no value.  The presence of a
header name may be significant for some applications.  Therefore, it is correct
for EnumerateHeader to return empty values.

[darin (slow to review), 2010-09-24 15:36:40]

On 2010/09/24 15:35:28, darin wrote:
> LGTM
> 
> There are legit use cases for headers that have no value.  The presence of a
> header name may be significant for some applications.  Therefore, it is
correct
> for EnumerateHeader to return empty values.

Can you include a comment on EnumerateHeader that indicates that value may be
empty?