Closed Bug 1260836 Opened 4 years ago Closed 4 years ago

Add support for CRX file extraction

Categories

(Core Graveyard :: File Handling, defect)

defect
Not set

Tracking

(firefox47 fixed, firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: bytesized, Assigned: bytesized)

References

Details

Attachments

(1 file, 1 obsolete file)

We want to add support for extracting CRX files, described here: https://developer.chrome.com/extensions/crx.

For the time being, we will likely just strip out and ignore the package header, then treat it as a zip file.
Attachment #8739233 - Flags: review?(spohl.mozilla.bugs)
Comment on attachment 8739233 [details]
MozReview Request: Bug 1260836 - Add functionality to allow CRX files to be handled as ZIP files

https://reviewboard.mozilla.org/r/45133/#review41643

Looks great! r+ with the nit addressed.

::: modules/libjar/nsZipArchive.cpp:268
(Diff revision 1)
> +// this is just the start of the file. If the file is a CRX file, the start of
> +// the data is after the CRX header.
> +// CRX header reference: (CRX version 2)
> +//    Header requires little-endian byte ordering with 4-byte alignment.
> +//    32 bits       : magicNumber   - Defined as a |char m[] = "Cr24"|.
> +//                                    Equivilant to |uint32_t m = 0x34327243|.

nit: s/Equivilant/Equivalent/
Attachment #8739233 - Flags: review?(spohl.mozilla.bugs) → review+
Attached patch Fix build on MacOSX (obsolete) — Splinter Review
MozReview-Commit-ID: 1YyA0CC21wi
Assignee: ksteuber → cpearce
Status: NEW → ASSIGNED
(In reply to Chris Pearce (:cpearce) from comment #3)
> Created attachment 8739284 [details] [diff] [review]
> Fix build on MacOSX
> 
> MozReview-Commit-ID: 1YyA0CC21wi

Patch has compile errors on MacOSX. Kirk, you'll need to fold this into your commit before landing.
Assignee: cpearce → ksteuber
Comment on attachment 8739284 [details] [diff] [review]
Fix build on MacOSX

Review of attachment 8739284 [details] [diff] [review]:
-----------------------------------------------------------------

::: modules/libjar/nsZipArchive.cpp
@@ +165,5 @@
>    , mLen(0)
>    , mMap(nullptr)
>    , mRefCnt(0)
> +  , mFileStart(nullptr)
> +  , mTotalLen(0) 

oops, trailing whitespace should be removed!
Kirk, I didn't mention this but it's good practice to send your patches to try before landing, or even better before requesting review. Let me know if I can help you get set up.
Flags: needinfo?(ksteuber)
Discussed offline.
Flags: needinfo?(ksteuber)
https://hg.mozilla.org/mozilla-central/rev/85a28619dd12
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment on attachment 8739233 [details]
MozReview Request: Bug 1260836 - Add functionality to allow CRX files to be handled as ZIP files

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45133/diff/1-2/
Comment on attachment 8739284 [details] [diff] [review]
Fix build on MacOSX

My updated patch includes these changes.
Attachment #8739284 - Attachment is obsolete: true
Comment on attachment 8739233 [details]
MozReview Request: Bug 1260836 - Add functionality to allow CRX files to be handled as ZIP files

Requesting uplift for Firefox 47.

Approval Request Comment
[Feature/regressing bug #]: Widevine EME support
[User impact if declined]: No Widevine CDM downloaded, so no Widevine DRM video playback.
[Describe test coverage new/current, TreeHerder]: We have test coverage of the GMP downloader code.
[Risks and why]: Low.
[String/UUID change made/needed]: None.
Attachment #8739233 - Flags: approval-mozilla-aurora?
Comment on attachment 8739233 [details]
MozReview Request: Bug 1260836 - Add functionality to allow CRX files to be handled as ZIP files

Widevine related uplifts were pre-approved.
Attachment #8739233 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Depends on: 1269185
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.