Closed Bug 370103 Opened 19 years ago Closed 18 years ago

Crash [@ nsJARChannel::OnStartRequest] when passing null listener to AsyncOpen

Categories

(Core :: Networking: JAR, defect)

Product:
Core
Component:
Networking: JAR
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: jwkbugzilla, Assigned: jwkbugzilla)

Details

Attachments

(2 files, 2 obsolete files)

JAR file for the test
128 bytes, application/octet-stream
Details
Same patch and unit test (using own JAR file)
1.99 KB, patch
darin.moz
: review+
Biesinger
: superreview+
Details | Diff | Splinter Review
I passed a null pointer to nsJARChannel::AsyncOpen accidentally and it crashed. There is a check missing in this method, patch to re-add it coming.
Attached patch Adding missing check (obsolete) — — Splinter Review
Blame says Darin removed this check, asking him to review.
Assignee: nobody → trev
Status: NEW → ASSIGNED
Attachment #254763 - Flags: review?(darin.moz)
Sorry, my bad. Darin didn't remove this check, it never was there in the first place. I was looking at the wrong code in bug 176919.
Attachment #254763 - Flags: review?(darin.moz) → review+
Attachment #254763 - Flags: superreview?(cbiesinger)
Could you write a test for this and add it to modules/libjar/test/unit/, preferably as part of the patch?
Component: Networking → Networking: JAR
QA Contact: networking → networking.jar
Attached patch Same patch and unit test (obsolete) — — Splinter Review
Attachment #254763 - Attachment is obsolete: true
Attachment #254791 - Flags: superreview?(jwalden+bmo)
Attachment #254791 - Flags: review+
Attachment #254763 - Flags: superreview?(cbiesinger)
Comment on attachment 254791 [details] [diff] [review] Same patch and unit test why not use a jar: URI here? the file doesn't even have to exist. and you'd avoid having the unit test break if someone uses flat chrome. (I'd also declare Cc and Ci at global scope) (Waldo's no superreviewer, but you could ask him for a second review)
Attachment #254791 - Flags: superreview?(jwalden+bmo) → superreview-
Attached file JAR file for the test —
This is the modules/libjar/test/unit/data/test_bug370103.jar file used in the test.
Attachment #254791 - Attachment is obsolete: true
Attachment #254819 - Flags: superreview?(cbiesinger)
Attachment #254819 - Flags: review?(darin.moz)
Comment on attachment 254819 [details] [diff] [review] Same patch and unit test (using own JAR file) I'd have reused the existing .zip file...
Attachment #254819 - Flags: superreview?(cbiesinger) → superreview+
I didn't want a dependency on the testcase from another bug. If that testcase changes again, it shouldn't break this testcase.
Attachment #254819 - Flags: review?(darin.moz) → review+
Christian, can you check in this patch?
Whiteboard: [checkin needed]
mozilla/modules/libjar/nsJARChannel.cpp 1.125 mozilla/modules/libjar/test/unit/test_bug370103.js 1.1
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Shouldn't have forgotten this one: mozilla/modules/libjar/test/unit/data/test_bug370103.jar 1.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: