Closed Bug 631967 Opened 9 years ago Closed 9 years ago

Unsafe use of mResponseHead in HttpBaseChannel::GetEntityID


(Core :: Networking: HTTP, defect)

Not set





(Reporter: jdm, Assigned: edsrzf)


(Whiteboard: [good first bug][mentor=jdm][inbound])


(1 file, 1 obsolete file)

Either mResponseHead can be null in GetEntityID, or it can't, but these bits need to change one way or another:

>1203   const char* acceptRanges =
>1204       mResponseHead->PeekHeader(nsHttp::Accept_Ranges);
>1212   if (mResponseHead) {
>1213     size = mResponseHead->TotalEntitySize();
This is a very simple bug to resolve. Whoever ends up working on it should get input from Jason or Honza on whether to add a null check or remove the existing one.
Whiteboard: [good first bug][mentor=jdm]
After on OnStartRequest we have to return something, nevertheless we have the response head.

The change with acceptRanges has been added in bug 462707 [1].  Ehsan apparently forgot to add a null check, I would say.

We should move his code into the if block.

Attached patch Proposed patch (obsolete) — Splinter Review
I've chosen this bug for my first patch. Hopefully I got it right. :)
Attachment #547345 - Flags: review?(josh)
Comment on attachment 547345 [details] [diff] [review]
Proposed patch

Thanks very much for your patch, Evan! It looks right to me, but I will defer to Honza's review on this, since he is a peer of this code. However, if you could upload another version that follows the guidelines at then it will be easier to check in this patch once the review process is finished.
Attachment #547345 - Flags: review?(josh) → review?(honzab.moz)
Attached patch Patch 2Splinter Review
Patch 2 attached; hopefully in the correct format this time.
Attachment #547345 - Attachment is obsolete: true
Attachment #547613 - Flags: review?(honzab.moz)
Attachment #547345 - Flags: review?(honzab.moz)
Comment on attachment 547613 [details] [diff] [review]
Patch 2

Review of attachment 547613 [details] [diff] [review]:

Attachment #547613 - Flags: review?(honzab.moz) → review+

Thanks Evan!
Assignee: nobody → edsrzf
Whiteboard: [good first bug][mentor=jdm] → [good first bug][mentor=jdm][inbound]
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.