check bundled JAR and XPI files for corruption

RESOLVED FIXED in Firefox 68

Status

()

enhancement
P2
normal
RESOLVED FIXED
6 months ago
10 days ago

People

(Reporter: rhelmer, Assigned: rhelmer)

Tracking

(Blocks 1 bug)

unspecified
Firefox 68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(4 attachments, 5 obsolete attachments)

Assignee

Description

6 months ago
We've seen problems for quite a while related to JAR and XPI files that are bundled with Firefox being corrupted, such as crashes. Let's start measuring this.
Priority: -- → P2
Assignee

Comment 4

5 months ago
Attachment #9035095 - Flags: review?(chutten)

Comment 5

5 months ago

Comment on attachment 9035095 [details]
Data Collection Review for Bug 1515712

Preliminary note:

When it comes to Linux distributions who build their own Firefox the channels they set do not necessarily map to our nightly/beta/release set. By having releaseChannelCollection: opt-out you are enabling collection for all non-mozilla channels. By turning corroborator off for beta and release by pref you are only turning them off for those two mozilla-built channels. It is possible that Linux distros may specifically disable corroborator or build using their own firefox.js or prefs.js that supercede the one you edit, but I don't expect them to keep up with all of our changes.

In short, you may be collecting data from more Firefox installs than you think with the code written the way it's currently written. From a Data Collection Review POV this is acceptable: it's Category 1 data. It just may be unintentional or unexpected.

Another note: since you state in Q9 that you're interested in the current prevalence of these errors it seems as though you could start these collections as expiring collections to serve that need and "upgrade" them to eternal collection pending the results of your analysis. Something to consider.

DATA COLLECTION REVIEW RESPONSE:

Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes. These collections are Telemetry so are documented in their definitions file (Scalars.yaml), the Probe Dictionary, and on telemetry.mozilla.org's Measurement Dashboards.

Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. These collections are Telemetry so can be controlled through Firefox's Preferences. Also the functionality of the underlying feature can be controlled by the preference corroborator.enabled. Setting that to false disables the feature including the data collection.

If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes, :rhelmer.

Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1, Technical. (I suppose corrupt XPI detection counts are proportional to addon install counts which could be an argument for Category 2... but a seriously weak argument)

Is the data collection request for default-on or default-off?

Default on for all channels, though the feature (including its data collection) is presently disabled in beta and release.

Does the instrumentation include the addition of any new identifiers?

No.

Is the data collection covered by the existing Firefox privacy notice?

Yes.

Does there need to be a check-in in the future to determine whether to renew the data?

No. This collection is permanent.


Result: datareview+

Attachment #9035095 - Flags: review?(chutten) → review+
Assignee

Comment 6

5 months ago

(In reply to Chris H-C :chutten from comment #5)

Comment on attachment 9035095 [details]
Data Collection Review for Bug 1515712

Preliminary note:

When it comes to Linux distributions who build their own Firefox the channels they set do not necessarily map to our nightly/beta/release set. By having releaseChannelCollection: opt-out you are enabling collection for all non-mozilla channels. By turning corroborator off for beta and release by pref you are only turning them off for those two mozilla-built channels. It is possible that Linux distros may specifically disable corroborator or build using their own firefox.js or prefs.js that supercede the one you edit, but I don't expect them to keep up with all of our changes.

This is a good point, thanks! I think that data from builds other than ours are going to be junk, since they won't be coming from us and will be unsigned.

In short, you may be collecting data from more Firefox installs than you think with the code written the way it's currently written. From a Data Collection Review POV this is acceptable: it's Category 1 data. It just may be unintentional or unexpected.

Another note: since you state in Q9 that you're interested in the current prevalence of these errors it seems as though you could start these collections as expiring collections to serve that need and "upgrade" them to eternal collection pending the results of your analysis. Something to consider.

So in Scalars.yaml I set:

expires: "71"

Is there anything else I need to do to make them expiring collections?

Flags: needinfo?(chutten)

Comment 7

5 months ago

(In reply to Robert Helmer [:rhelmer] from comment #6)

Another note: since you state in Q9 that you're interested in the current prevalence of these errors it seems as though you could start these collections as expiring collections to serve that need and "upgrade" them to eternal collection pending the results of your analysis. Something to consider.

So in Scalars.yaml I set:

expires: "71"

Is there anything else I need to do to make them expiring collections?

We should probably cut another edition of the data review request that says you only want it until the end Firefox 70 for now, and I'll reply with a marginally-different data review response. This'll help future us understand what the final result was.

But yeah, as far as code goes, that's it.

When Nightly hits 70 you should (the service that does this is somewhat reliable) get an email about it expiring in the next version. And shortly thereafter a bug might be filed when the version increase simulation builds start running if you have an automated test that doesn't account for the possibility of your collection being expired. (collecting to an expired metric is a no-op. No errors, but snapshots won't have the accumulations). This'll help us remember to look at the data and make our conclusions, and see if the metric should be removed or renewed.

Flags: needinfo?(chutten)
Assignee

Updated

3 months ago
Depends on: 1533818

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:rhelmer, could you have a look please?

Flags: needinfo?(rhelmer)
Assignee

Comment 9

2 months ago

We manually tested all the various options for signing omni JAR in bug 1533818 and everything looks good, going to get this patch in shape to land so we'll be ready once releng starts shipping signed JARs with Nightly.

Flags: needinfo?(rhelmer)
Assignee

Comment 12

2 months ago

Depends on D28246

Attachment #9059579 - Attachment is obsolete: true
Attachment #9059580 - Attachment is obsolete: true
Attachment #9059581 - Attachment is obsolete: true
Assignee

Comment 13

2 months ago

re-requesting review per comment 7

Attachment #9059584 - Flags: data-review?(chutten)
Assignee

Updated

2 months ago
Attachment #9035095 - Attachment is obsolete: true
Attachment #9032756 - Attachment is obsolete: true

Comment 16

2 months ago
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a2d24fd1fd67
check bundled JAR and XPI files for corruption r=kmag,chutten

Comment 17

2 months ago
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/acb99631797a
provide OMNIJAR_NAME constant for browser r=kmag

Comment 18

2 months ago
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3e46d5fddd21
add default pref and start corroborator if enabled r=kmag
Assignee

Comment 20

2 months ago

There is a stray line from a merge in the first patch which is fixed by the second, I'll just remove it entirely before re-landing, thanks!

Flags: needinfo?(rhelmer)

Comment 21

2 months ago
Pushed by rhelmer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/718402b108aa
provide OMNIJAR_NAME constant for browser r=kmag
https://hg.mozilla.org/integration/autoland/rev/ac6c0bc531e3
check bundled JAR and XPI files for corruption r=kmag,chutten
https://hg.mozilla.org/integration/autoland/rev/6e082b675763
add default pref and start corroborator if enabled r=kmag

Comment 22

2 months ago
bugherder
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
Assignee

Comment 23

2 months ago

I went ahead and landed this ahead of the latest data review since it's a very minor change to an already-approved data review, see comment 7.

Assignee

Updated

2 months ago
Blocks: 1515173
No longer depends on: 1515173
Comment on attachment 9059584 [details]
Data Collection Review for Bug 1515712 (v2)

DATA COLLECTION REVIEW RESPONSE:

    Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes. This collection is Telemetry so is documented in its definitions file [Scalars.yaml](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/telemetry/Scalars.yaml) and the [Probe Dictionary](https://telemetry.mozilla.org/probe-dictionary/).

    Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.

    If the request is for permanent data collection, is there someone who will monitor the data over time?

No. This collection will expire in Firefox 71.

    Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1, Technical.

    Is the data collection request for default-on or default-off?

Default on for all channels (though controlled by pref)

    Does the instrumentation include the addition of any new identifiers?

No.

    Is the data collection covered by the existing Firefox privacy notice?

Yes.

    Does there need to be a check-in in the future to determine whether to renew the data?

Yes. :rhelmer is responsible for renewing or removing the collection before it expires in Firefox 71.

---
Result: datareview+
Attachment #9059584 - Flags: data-review?(chutten) → data-review+

Doubling back on this.

Releng is close to deploying support for this, which would need uplift to be used in 68. I feel the risk is minimal but will approach that request once it is running sanely on Nightly.

Feel free to let me know if there are any product concerns over the timing.

You need to log in before you can comment on or make changes to this bug.