nsZipWriter::addEntryChannel(queue=true) creates corrupt zip file

RESOLVED FIXED in mozilla10

Status

()

Core
Networking: JAR
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: kmag, Assigned: kmag)

Tracking

unspecified
mozilla10
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

6 years ago
Created attachment 557938 [details]
Proposed fix

When channels are processed from a queue, the header for an entry is initialized but never written out to the zip file, which leaves the file corrupt. The attached patch removes the code duplication for stream and channel processing and fixes the issue.
(Assignee)

Comment 1

6 years ago
Created attachment 557939 [details] [diff] [review]
Proposed fix
Attachment #557938 - Attachment is obsolete: true
Attachment #557939 - Flags: review?(dtownsend)
Comment on attachment 557939 [details] [diff] [review]
Proposed fix

I'd like to see some unit tests for this before reviewing it
Attachment #557939 - Flags: review?(dtownsend)
(Assignee)

Comment 3

6 years ago
Do you want a separate test for this bug, or should I add them to test_asyncadd.js where they already fit comfortably?
(Assignee)

Comment 4

6 years ago
Created attachment 558713 [details] [diff] [review]
Updated patch

Updated patch with tests.
Assignee: nobody → maglione.k
Attachment #557939 - Attachment is obsolete: true
Attachment #558713 - Flags: review?(dtownsend)
Attachment #558713 - Flags: review?(dtownsend) → review+
(Assignee)

Updated

6 years ago
Whiteboard: [needs landing]
(Assignee)

Comment 5

6 years ago
Created attachment 569533 [details] [diff] [review]
Updated patch

Updated after PRBool code churn. Can someone land this please?
Attachment #558713 - Attachment is obsolete: true

Updated

6 years ago
Keywords: checkin-needed

Comment 6

6 years ago
Actually Kris, if you could attach a patch that follows the conventions in http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed, it will be a lot easier to check in the patch.
Keywords: checkin-needed
(Assignee)

Comment 7

6 years ago
Created attachment 569543 [details] [diff] [review]
Updated commit message and authorship info.

jdm: Thanks.
Attachment #569533 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Keywords: checkin-needed

Comment 8

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/926699a40a9b
Keywords: checkin-needed
Whiteboard: [needs landing] → [inbound]

Comment 9

6 years ago
https://hg.mozilla.org/mozilla-central/rev/926699a40a9b
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
You need to log in before you can comment on or make changes to this bug.