Fix bug in InitParseWithoutQuestion

Fix bug in InitParseWithoutQuestion

Fix crash in cases where the buffer is smaller than the size of the DNS header.

BUG=241891
TEST=DnsResponseTest.InitParseWithoutQueryPacketTooShort

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=201007

[szym, 2013-05-17 20:40:33]

Please re-upload Patch 2.

[Noam Samuel, 2013-05-17 20:42:32]

On 2013/05/17 20:40:33, szym wrote:
> Please re-upload Patch 2.

Are there any steps I should take differently when re-uploading?

[szym, 2013-05-17 20:43:21]

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

https://codereview.chromium.org/15326002/diff/1/net/dns/dns_response.cc#newco...
net/dns/dns_response.cc:193: if (nbytes < 0 ||
nbytes < 0 is an error. You should probably DCHECK(nbytes >= 0);

[szym, 2013-05-17 20:43:39]

Just try again.


On Fri, May 17, 2013 at 4:42 PM, <noamsml@chromium.org> wrote:

> On 2013/05/17 20:40:33, szym wrote:
>
>> Please re-upload Patch 2.
>>
>
> Are there any steps I should take differently when re-uploading?
>
>
https://codereview.chromium.**org/15326002/<https://codereview.chromium.org/1...
>

[Noam Samuel, 2013-05-17 20:52:44]

Re-uploaded with change to use DCHECK instead of checking via an if statement.

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

https://codereview.chromium.org/15326002/diff/1/net/dns/dns_response.cc#newco...
net/dns/dns_response.cc:193: if (nbytes < 0 ||
On 2013/05/17 20:43:21, szym wrote:
> nbytes < 0 is an error. You should probably DCHECK(nbytes >= 0);

Done.

[szym, 2013-05-17 21:07:00]

LGTM after a few fixes.

https://codereview.chromium.org/15326002/diff/13001/net/dns/dns_response.cc
File net/dns/dns_response.cc (right):

https://codereview.chromium.org/15326002/diff/13001/net/dns/dns_response.cc#n...
net/dns/dns_response.cc:165: DCHECK(nbytes >= 0);
nit: DCHECK_GE(nbytes, 0)

https://codereview.chromium.org/15326002/diff/13001/net/dns/dns_response.cc#n...
net/dns/dns_response.cc:194: DCHECK(nbytes >= 0);
ditto

https://codereview.chromium.org/15326002/diff/13001/net/dns/dns_response.cc#n...
net/dns/dns_response.cc:196: int hdr_size = (int)sizeof(dns_protocol::Header);
static_cast<int>, but I'd prefer if you used size_t here and static_cast in the
comparison below below.

https://codereview.chromium.org/15326002/diff/13001/net/dns/dns_response.cc#n...
net/dns/dns_response.cc:199: nbytes >= io_buffer_->size())
nit: single line.

[szym, 2013-05-17 21:07:59]

Please file a bug and update BUG= in the CL description. Also, since you are
adding a test specifically for that, also add TEST= line to the CL description.

[Noam Samuel, 2013-05-17 21:23:50]

Added bug and test lines.

https://codereview.chromium.org/15326002/diff/13001/net/dns/dns_response.cc
File net/dns/dns_response.cc (right):

https://codereview.chromium.org/15326002/diff/13001/net/dns/dns_response.cc#n...
net/dns/dns_response.cc:165: DCHECK(nbytes >= 0);
On 2013/05/17 21:07:00, szym wrote:
> nit: DCHECK_GE(nbytes, 0)

Done.

https://codereview.chromium.org/15326002/diff/13001/net/dns/dns_response.cc#n...
net/dns/dns_response.cc:194: DCHECK(nbytes >= 0);
On 2013/05/17 21:07:00, szym wrote:
> ditto

Done.

https://codereview.chromium.org/15326002/diff/13001/net/dns/dns_response.cc#n...
net/dns/dns_response.cc:196: int hdr_size = (int)sizeof(dns_protocol::Header);
On 2013/05/17 21:07:00, szym wrote:
> static_cast<int>, but I'd prefer if you used size_t here and static_cast in
the
> comparison below below.

Done.

https://codereview.chromium.org/15326002/diff/13001/net/dns/dns_response.cc#n...
net/dns/dns_response.cc:199: nbytes >= io_buffer_->size())
On 2013/05/17 21:07:00, szym wrote:
> nit: single line.

Done.

[commit-bot: I haz the power, 2013-05-17 22:36:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/noamsml@chromium.org/15326002/20001

[commit-bot: I haz the power, 2013-05-20 03:50:41]

Change committed as 201007