unRAR licensed code shipped as part of 7zstub

VERIFIED FIXED in Firefox -esr60

Status

()

defect
P1
normal
VERIFIED FIXED
9 months ago
9 months ago

People

(Reporter: astieger, Assigned: mhowell)

Tracking

60 Branch
Firefox 63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox-esr6062+ fixed, firefox61 wontfix, firefox62 wontfix, firefox63 fixed)

Details

Attachments

(8 attachments, 2 obsolete attachments)

7.20 MB, patch
agashlin
: review+
Details | Diff | Splinter Review
4.47 MB, patch
agashlin
: review+
Details | Diff | Splinter Review
42.86 KB, patch
agashlin
: review+
Details | Diff | Splinter Review
1.27 KB, patch
agashlin
: review+
Details | Diff | Splinter Review
150.33 KB, patch
agashlin
: review+
Details | Diff | Splinter Review
4.17 KB, patch
agashlin
: review+
Details | Diff | Splinter Review
211.16 KB, patch
agashlin
: review+
Details | Diff | Splinter Review
6.32 MB, patch
mhowell
: review+
Details | Diff | Splinter Review
Reporter

Description

9 months ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID: 20180621121604

Steps to reproduce:

SUSE legal team flagged up the Thunderbird 60.0 tarball prior to the inclusion into openSUSE as not being freely redistributable:
https://build.opensuse.org/request/show/629371
"contains unrar, which is non-commerical only."
I think the comment actually relates to the usage restrictions.


Actual results:

Tarball contains 7zstub for building the self-extracting Windows installer. Part of this code is covered under the unRar license:
other-licenses/7zstub/src/DOC/unRarLicense.txt


Expected results:

A freely distributable tarball, excluding usage restrictions.

Updated

9 months ago
Component: Untriaged → Build Config
Maybe we should take 7zstub from LZMA SDK to clarify the license.
Apparently RAR-derived files did not exist until bug 1436475. Since we do not depend on RAR-derived files at all, there is no reason to include non-free files in our tree.

Matt, would you mind replacing the 7-zip 18.01 code with LZMA SDK's one?
LZMA SDK 18.01 is available from <https://www.7-zip.org/sdk.html>. LZMA SDK is a subset of 7-Zip, but it excludes non-free files.
Blocks: 1436475
Status: UNCONFIRMED → NEW
Component: Build Config → Installer
Ever confirmed: true
Flags: needinfo?(mhowell)
Product: Thunderbird → Firefox
Version: 60 → 60 Branch
Assignee

Comment 3

9 months ago
Thanks for reporting this, Andreas. This is my mistake for not paying enough attention to the licenses on this code, so I'll absolutely fix it.

(In reply to Masatoshi Kimura [:emk] from comment #2)
> Apparently RAR-derived files did not exist until bug 1436475. Since we do
> not depend on RAR-derived files at all, there is no reason to include
> non-free files in our tree.

The way we were getting around this prior to bug 1436475 was that we had just removed those files from our tree, meaning we were distributing an incomplete copy of the 7-zip source. I don't know if that's a violation of one of the applicable licenses or not.

> Matt, would you mind replacing the 7-zip 18.01 code with LZMA SDK's one?
> LZMA SDK 18.01 is available from <https://www.7-zip.org/sdk.html>. LZMA SDK
> is a subset of 7-Zip, but it excludes non-free files.

There might be other options for fixing this, but this is the one that I'll go for because it seems like the best way to be absolutely sure we don't have any non-free code; the LZMA SDK is in the public domain. Thanks for the suggestion, :emk.
Assignee: nobody → mhowell
Status: NEW → ASSIGNED
Flags: needinfo?(mhowell)
Priority: -- → P1
Assignee

Comment 6

9 months ago
This simply applies the file other-licenses/7zstub/mozila_customizations.diff,
which did not need to be changed in this patch series because it applies cleanly
to the LZMA SDK code, which is organized in the same way as the 7-zip code.
Assignee

Comment 8

9 months ago
Also added line breaks to make the file more readable (while making the diff
harder to review).
Assignee

Updated

9 months ago
Attachment #9005448 - Flags: review?(agashlin)
Assignee

Updated

9 months ago
Attachment #9005450 - Flags: review?(agashlin)
Assignee

Updated

9 months ago
Attachment #9005451 - Flags: review?(agashlin)
Assignee

Updated

9 months ago
Attachment #9005452 - Flags: review?(agashlin)
Assignee

Updated

9 months ago
Attachment #9005453 - Flags: review?(agashlin)
Attachment #9005448 - Flags: review?(agashlin) → review+
Assignee

Updated

9 months ago
Attachment #9005452 - Attachment is obsolete: true
Attachment #9005452 - Flags: review?(agashlin)
Assignee

Updated

9 months ago
Attachment #9005453 - Attachment is obsolete: true
Attachment #9005453 - Flags: review?(agashlin)
Assignee

Comment 11

9 months ago
Also added line breaks to make the file more readable (while making the diff
harder to review).
Assignee

Updated

9 months ago
Attachment #9005472 - Flags: review?(agashlin)
Assignee

Updated

9 months ago
Attachment #9005473 - Flags: review?(agashlin)
Assignee

Updated

9 months ago
Attachment #9005474 - Flags: review?(agashlin)
Attachment #9005450 - Flags: review?(agashlin) → review+
Attachment #9005451 - Flags: review?(agashlin) → review+
Attachment #9005472 - Flags: review?(agashlin) → review+
Attachment #9005473 - Flags: review?(agashlin) → review+
Attachment #9005474 - Flags: review?(agashlin) → review+

Comment 12

9 months ago
Pushed by mhowell@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e66fd6806bbf
Part 1 - Remove existing copy of the 7-zip code. r=agashlin
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0427508d13a
Part 2 - Add a copy of the LZMA SDK. r=agashlin
https://hg.mozilla.org/integration/mozilla-inbound/rev/7cf0234dc9e8
Part 3 - Apply our existing patches to the LZMA SDK code. r=agashlin
https://hg.mozilla.org/integration/mozilla-inbound/rev/e442565e6d4b
Part 4 - Update the version number in the 7-zip SFX manifest. r=agashlin
https://hg.mozilla.org/integration/mozilla-inbound/rev/12156c3935c3
Part 5 - Rebuild the binary SFX stub. r=agashlin
https://hg.mozilla.org/integration/mozilla-inbound/rev/da29148fc085
Part 6 - Update the 7zstub readme to reflect the new contents. r=agashlin
Did you re-apply <https://hg.mozilla.org/mozilla-central/rev/48a0e24a2b3e>? Unfortunately, this changeset did not update mozila_customizations.diff.

Also, <https://hg.mozilla.org/integration/mozilla-inbound/diff/7cf0234dc9e8/other-licenses/7zstub/src/CPP/7zip/Bundles/SFXSetup/SfxSetup.cpp> does not show the diff correctly. (Maybe because of the newline character difference?)
Flags: needinfo?(mhowell)
Assignee

Comment 14

9 months ago
(In reply to Masatoshi Kimura [:emk] from comment #13)
> Did you re-apply <https://hg.mozilla.org/mozilla-central/rev/48a0e24a2b3e>?
> Unfortunately, this changeset did not update mozila_customizations.diff.

Good catch, thanks; I'll get that fixed.

> Also,
> <https://hg.mozilla.org/integration/mozilla-inbound/diff/7cf0234dc9e8/other-
> licenses/7zstub/src/CPP/7zip/Bundles/SFXSetup/SfxSetup.cpp> does not show
> the diff correctly. (Maybe because of the newline character difference?)

Yeah, I noticed that and honestly just didn't think it was worth the effort to deal with it properly.
Flags: needinfo?(mhowell)
Assignee

Comment 15

9 months ago
Also take the opportunity to add that change to mozilla_customizations.diff and
to correct the line endings in the files that it contains changes to.
Attachment #9005494 - Flags: review?(agashlin)
Attachment #9005494 - Flags: review?(agashlin) → review+

Comment 16

9 months ago
Pushed by mhowell@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1cdb27fcd6b
Part 7 - Reapply the portion of bug 1425468 that involves this code. r=agashlin
[Tracking Requested - why for this release]:
We should uplift this to ESR60 to fix the original issue regarding Thunderbird.
(In reply to Masatoshi Kimura [:emk] from comment #18)
> [Tracking Requested - why for this release]:
> We should uplift this to ESR60 to fix the original issue regarding
> Thunderbird.

Note that bug 1425468 is not fixed in 60, so only part 1 to 6 should be uplifted.
Matt, can you create an ESR60-specific patch that we can consider for uplift? It's probably too late for 60.2.0 at this point (the RC is already built and with QA), but I'd definitely consider it as a ride-along for a 60.2.x dot release or for 60.3.
Assignee

Comment 22

9 months ago
This is a rollup of parts 1 through 6 along with the line ending fix that I added to part 7 (but not the rest of part 7; comment 19 correctly points out that bug 1425468 was not uplifted to 60, so the functional changes don't apply there).

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
This is a fix for a licensing problem which was reported against Thunderbird (but does affect Firefox also), so it would need to be on 60 for them to pick it up easily.

User impact if declined:
Continued distribution of non-free code in our trees.

Fix Landed on Version:
63

Risk to taking this patch (and alternatives if risky):
Very low. We're effectively replacing one distribution of a vendored project with a different distribution which leaves out a lot of code that we weren't using anyway. We're also updating that project to the latest release, but the only changes from that are minor performance enhancements.

String or UUID changes made by this patch:
None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Flags: needinfo?(mhowell)
Attachment #9005669 - Flags: review+
Attachment #9005669 - Flags: approval-mozilla-esr60?
Comment on attachment 9005669 [details] [diff] [review]
Patch for ESR60

Turns out we need a second RC build to fix some partial update generation issues, so I'm approving this for ESR 60.2. Thanks for the quick roll-up patch, Matt!
Attachment #9005669 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Reporter

Comment 25

9 months ago
Seen as fixed in 60.2.0esr tarball, thanks.
Status: RESOLVED → VERIFIED
Depends on: 1490264
You need to log in before you can comment on or make changes to this bug.