1260836 - Add support for CRX file extraction

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Robin Steuber (she/her) [:bytesized]]

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.

Comment 1

[Robin Steuber (she/her) [:bytesized]]

Attachment: MozReview Request: Bug 1260836 - Add functionality to allow CRX files to be handled as ZIP files

Review commit: https://reviewboard.mozilla.org/r/45133/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/45133/

Comment 2

[Stephen A Pohl [:spohl] (OOO until 7/30, contact :haik if urgent)]

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/

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Fix build on MacOSX

MozReview-Commit-ID: 1YyA0CC21wi

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

(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.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

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!

Comment 6

[Stephen A Pohl [:spohl] (OOO until 7/30, contact :haik if urgent)]

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.

Comment 7

[Stephen A Pohl [:spohl] (OOO until 7/30, contact :haik if urgent)]

Discussed offline.

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=42fafe581102

Comment 9

[Robin Steuber (she/her) [:bytesized]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d9defdf0ed1

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/85a28619dd12113740470b2a3b6178ddc5c1c1bd
Bug 1260836 - Add functionality to allow CRX files to be handled as ZIP files. r=spohl

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/85a28619dd12

Comment 12

[Robin Steuber (she/her) [:bytesized]]

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 13

[Robin Steuber (she/her) [:bytesized]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff95b7791235

Comment 14

[Robin Steuber (she/her) [:bytesized]]

Comment on attachment 8739284 [details] [diff] [review]
Fix build on MacOSX

My updated patch includes these changes.

Comment 15

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7c3602602019

Comment 16

[Chris Pearce [:cpearce (Not reading bugmail)]]

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.

Comment 17

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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.