bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

make partial updates contain checksums for all files, so that if a checksum fails, we'll roll over into a complete update [frankenfox fixing]

RESOLVED WONTFIX

Status

()

Toolkit
Application Update
RESOLVED WONTFIX
11 years ago
11 months ago

People

(Reporter: (not reading, please use seth@sspitzer.org instead), Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

this idea comes from rhelmer.

to address frankenfox (see bug #360127 and bug #376407 for more about franken-fox) we should fix make_incremental_update.sh generate mars that include no-op .patch files for all files.  The reason for this is that we can rely on the crc and size checks that we do in updater.exe to verify that the "source" version of all the application files for firefox are legit, even if we aren't going to change it.

if the source files are not legit, our crc or size checks would fail, and we'd get the complete update, which is what we want.  a complete update would fix frankefox.  (unless you were unable to apply the complete because a .dll was in use, but that is another issue)

note, I'd still like AUS to log when we encounter franken-fox, and there is a bug about that already.  we should be able to detect franken-fox by bogus aus urls.

the one thing I don't know is we can generate no-op patches, or if the updater would do the right thing with them (check crc and size, and then do nothing, and gracefully continune on.)
rob helmer writes:

Here's my reasoning - a partial mar is intended to patch a user from one known state to another. In other words, from one complete mar to another complete mar.

If the user is not in a known state (esp. in a frankenfox state), they should not get a partial mar, but instead the complete, which brings them to the desired state but with the penalty of having longer download times.

So the crux of my argument is, giving users a complete mar, all things being equal, would be the most desirable thing to do, and we do use them if a partial fails for any reason. However since there is a cost in download time, the partial is a shortcut to this goal.

The partial should not apply just because it happens to, it should be bringing the user from one known state to another.

For 1.5 partner builds we modify the files that you'd find in a standard build, so we generate custom mars. For 2.0 builds we only add files, never modify existing ones, so we can use the standard mars.

The only exception I can think of that I have seen is that we purposely delivered only a partial mar to broken (read-only default profile) german locale users. Even in that case, checksumming everything would have not had an adverse effect, but I would say that when we are doing surgical mars then all bets are off.

Back to the standard case, here is how this would help frankenfox users :

* we build a set of partial mars that are intended to take users from one release (a) to another (b)
* frankenfox users are not release (a) but something else, but they are asking us to be upgraded.
* frankenfox users will fail over to a complete mar, bringing them to a known state

I will be at the office in a few minutes, let me know if you have time to discuss. This would be a pretty easy change as far as I can tell, and I think it'd not only deal with the frankenfox problem but that it's the right thing to do. 

Updated

11 years ago
Summary: to address frankenfox, make_incremental_update.sh generate no-op .patch files for all files → make_incremental_update.sh should generate no-op .patch files for unchanged files (address existing "frankenfox"es by making the partial update fail for them)
gentle ping - any update on this?
> any update on this?

no updates on my end.
no-op .patch files seems like a hacky way to do this, not sure if it'd be better to just add support more directly. Is it worth fixing this bug? I can take a stab at it if it's likely to be accepted, but I'm not sure that doing no-op .patch files is the right way.

mconnor suggests that it would be useful to support more than one checksum for "from" files, because there have been issues more than once where a local file is modified and we don't want to blow away that modification.

This seems like a pathological case, but it affects doing this kind of consistency check, so mentioning it :)
(In reply to comment #4)
> no-op .patch files seems like a hacky way to do this, not sure if it'd be
> better to just add support more directly. Is it worth fixing this bug? I can
> take a stab at it if it's likely to be accepted, but I'm not sure that doing
> no-op .patch files is the right way.
> 
> mconnor suggests that it would be useful to support more than one checksum for
> "from" files, because there have been issues more than once where a local file
> is modified and we don't want to blow away that modification.
> 
> This seems like a pathological case, but it affects doing this kind of
> consistency check, so mentioning it :)
> 

Actually this does not make sense, because there's only one complete to fail over to, and partial may fail for reasons outside of our control (as I understand it).

I think we should just enforce that partials can only be applied if certain files with certain checksums are present (e.g. the full contents of "from" MAR), and fail over to complete if that is not the case. Otherwise the end result will be an inconsistent state.
Remember to account for bug 404340 :)
(In reply to comment #6)
> Remember to account for bug 404340 :)

Ugh, yeah. This would certainly need to be optional on a per-file basis, to deal with issues like this (and it'd work around issues like mconnor is concerned with to). 

I'd be much happier if it was being specified explicitly that those files were not being checked, due to some specific bug, as opposed to the current situation.

The current solution to this is actually pretty poor, it just so happens that the .chk files generate a large enough diff that forcing the complete file into the partial is worth it, which is not at all deterministic.
Duplicate of this bug: 387153
(In reply to comment #8)
> *** Bug 387153 has been marked as a duplicate of this bug. ***

Duped, but stealing the summary from 387153.
Summary: make_incremental_update.sh should generate no-op .patch files for unchanged files (address existing "frankenfox"es by making the partial update fail for them) → make partial updates contain checksums for all files, so that if a checksum fails, we'll roll over into a complete update [frankenfox prevention]
(Assignee)

Updated

10 years ago
Product: Firefox → Toolkit
If the checksum is already different then we already have a frankenfox.

From the numbers below, I don't think there is much value in doing this when compared to other work that prevents frankenfoxes. Anyone disagree?

Total crashes processed : 31761889

Firefox 3.5 Mismatched File Version Report
20101204 through 20110411

Summary
=============================
    Lines processed : 1711982
  Crashes processed : 1721575
      Lines skipped :     361
   Total mismatches :  137788
 Percent mismatches :   8.00%

Skipped Line Breakdown
===============================
 Invalid firefox.exe name : 309
     Missing version info :  52

Mismatched Versions
==============================================================================+
                        |      Total       | DLLVer < FFVer  | DLLVer > FFVer |
                        +------------------+-----------------+----------------+
       Total mismatches : 137788 (100.00%) | 135850 (98.59%) |  1938 (1.41%)  |
 browserdirprovider.dll :  13507   (9.80%) |  11575  (8.40%) |  1932 (1.40%)  |
           brwsrcmp.dll :   6087   (4.42%) |   4894  (3.55%) |  1193 (0.87%)  |
              xpcom.dll : 101843  (73.91%) | 101810 (73.89%) |    33 (0.02%)  |
                xul.dll : 135854  (98.60%) | 135823 (98.57%) |    31 (0.02%)  |
                        +------------------+-----------------+----------------+


Firefox 3.6 Mismatched File Version Report
20101204 through 20110411

Summary
==============================
    Lines processed : 20277061
  Crashes processed : 20278718
      Lines skipped :     3797
   Total mismatches :    49995
 Percent mismatches :    0.25%

Skipped Line Breakdown
================================
      Invalid line format :   34
 Invalid firefox.exe name : 2907
     Missing version info :  857

Mismatched Versions
==============================================================================+
                        |      Total       | DLLVer < FFVer  | DLLVer > FFVer |
                        +------------------+-----------------+----------------+
       Total mismatches :  49995 (100.00%) |   1039 (2.08%)  | 48956 (97.92%) |
 browserdirprovider.dll :  48685  (97.38%) |    370 (0.74%)  | 48315 (96.64%) |
           brwsrcmp.dll :  48465  (96.94%) |    351 (0.70%)  | 48114 (96.24%) |
              xpcom.dll :  16297  (32.60%) |    426 (0.85%)  | 15871 (31.75%) |
                xul.dll :   7903  (15.81%) |    877 (1.75%)  |  7026 (14.05%) |
                        +------------------+-----------------+----------------+


Firefox 4.0 Mismatched File Version Report
20101111 through 20110411

Summary
=============================
    Lines processed : 9750994
  Crashes processed : 9761596
      Lines skipped :     954
   Total mismatches :    3679
 Percent mismatches :   0.04%

Skipped Line Breakdown
===============================
      Invalid line format :  46
 Invalid firefox.exe name : 246
     Missing version info : 664

Mismatched Versions
==============================================================================+
                        |      Total       | DLLVer < FFVer  | DLLVer > FFVer |
                        +------------------+-----------------+----------------+
       Total mismatches :  3679 (100.00%)  |    95 (2.58%)   |  3584 (97.42%) |
       browsercomps.dll :  3501  (95.16%)  |    82 (2.23%)   |  3419 (92.93%) |
           mozalloc.dll :  2110  (57.35%)  |    77 (2.09%)   |  2033 (55.26%) |
              xpcom.dll :  2067  (56.18%)  |    75 (2.04%)   |  1992 (54.15%) |
                xul.dll :  2531  (68.80%)  |    77 (2.09%)   |  2454 (66.70%) |
                        +------------------+-----------------+----------------+

[reply] [-]
Private
Comment 26 Robert Strong [:rstrong] (do not email) 2011-04-13 22:50:16 PDT

Going back just two weeks of Firefox 4 the numbers are tad better coming in at
0.02% vs 0.05% and the same is true going back 4 weeks.

Firefox 4.0 Mismatched File Version Report
20110329 through 20110411

Summary
============================
    Lines processed : 841619
  Crashes processed : 841894
      Lines skipped :     84
   Total mismatches :    175
 Percent mismatches :  0.02%

Skipped Line Breakdown
==============================
      Invalid line format :  1
 Invalid firefox.exe name : 16
     Missing version info : 67

Mismatched Versions
==============================================================================+
                        |      Total       | DLLVer < FFVer  | DLLVer > FFVer |
                        +------------------+-----------------+----------------+
       Total mismatches :   175 (100.00%)  |    5 (2.86%)    |  170 (97.14%)  |
       browsercomps.dll :   169  (96.57%)  |    4 (2.29%)    |  165 (94.29%)  |
           mozalloc.dll :    94  (53.71%)  |    4 (2.29%)    |   90 (51.43%)  |
              xpcom.dll :    98  (56.00%)  |    4 (2.29%)    |   94 (53.71%)  |
                xul.dll :   147  (84.00%)  |    5 (2.86%)    |  142 (81.14%)  |
                        +------------------+-----------------+----------------+
Summary: make partial updates contain checksums for all files, so that if a checksum fails, we'll roll over into a complete update [frankenfox prevention] → make partial updates contain checksums for all files, so that if a checksum fails, we'll roll over into a complete update [frankenfox fixing]
Per comment #10 I don't think this bug is needed.
Status: NEW → RESOLVED
Last Resolved: 11 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.