Last Comment Bug 631967 - Unsafe use of mResponseHead in HttpBaseChannel::GetEntityID
: Unsafe use of mResponseHead in HttpBaseChannel::GetEntityID
[good first bug][mentor=jdm][inbound]
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: unspecified
: x86 Linux
: -- normal (vote)
: mozilla8
Assigned To: Evan Shaw
Depends on:
  Show dependency treegraph
Reported: 2011-02-06 22:49 PST by Josh Matthews [:jdm]
Modified: 2011-08-03 02:18 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Proposed patch (1.68 KB, patch)
2011-07-21 02:17 PDT, Evan Shaw
no flags Details | Diff | Review
Patch 2 (1.80 KB, patch)
2011-07-21 22:29 PDT, Evan Shaw
honzab.moz: review+
Details | Diff | Review

Description Josh Matthews [:jdm] 2011-02-06 22:49:19 PST
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();
Comment 1 Josh Matthews [:jdm] 2011-05-30 11:49:05 PDT
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.
Comment 2 Honza Bambas (:mayhemer) 2011-06-10 08:24:34 PDT
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.

Comment 3 Evan Shaw 2011-07-21 02:17:58 PDT
Created attachment 547345 [details] [diff] [review]
Proposed patch

I've chosen this bug for my first patch. Hopefully I got it right. :)
Comment 4 Josh Matthews [:jdm] 2011-07-21 07:43:21 PDT
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.
Comment 5 Evan Shaw 2011-07-21 22:29:13 PDT
Created attachment 547613 [details] [diff] [review]
Patch 2

Patch 2 attached; hopefully in the correct format this time.
Comment 6 Honza Bambas (:mayhemer) 2011-07-29 12:18:32 PDT
Comment on attachment 547613 [details] [diff] [review]
Patch 2

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

Comment 7 Josh Matthews [:jdm] 2011-08-02 11:30:20 PDT

Thanks Evan!
Comment 8 Marco Bonardo [::mak] 2011-08-03 02:18:09 PDT

Note You need to log in before you can comment on or make changes to this bug.