453545 - InstallTrigger Broken when doing multiple signed XPI installs

Status: VERIFIED FIXED

(Core Graveyard :: Installer: XPInstall Engine, defect)

Description

[Pete Collins (MDG)]

Attachment: screen-shot of alert error

Steps to reproduce:

  1. go to this page: https://www.mozdevgroup.com/clients/bm/
  2. click on the "Combined Install" link
  3. two signed XPI's are installed
  4. second XPI fails w/ a "Signing could not be verified" error.

Expected result:
  4. both XPI's are successfully installed

This is only an issue w/ FF3. The above works correctly w/ FF2.

Comment 1

[Pete Collins (MDG)]

You can install the two XPI's individually though ...

Comment 2

[Dave Townsend [:mossop]]

Attachment: patch rev 1

This is a pretty simple problem. The manifest hash isn't emptied when the zipreader is closed. Since xpinstall uses a single instance of zipreader to parse all installs the second one fails because it appears that an entry is duplicated in the manifest.

Comment 3

[Dave Townsend [:mossop]]

Comment on attachment 347871 [details] [diff] [review]
patch rev 1

I'd like to get this landed a.s.a.p. as this issue is causing problems for AMO and I would like to get a branch fix done.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 347871 [details] [diff] [review]
patch rev 1

a & <3 = beltzner

Comment 5

[Dave Townsend [:mossop]]

Pushed: http://hg.mozilla.org/mozilla-central/rev/4abab3456109

Comment 6

[Fred Wenzel [:wenzel]]

Hot!

Comment 7

[Dave Townsend [:mossop]]

Comment on attachment 347871 [details] [diff] [review]
patch rev 1

Looking for approval to land this on the branch before code freeze. This is a trivial fix that makes the zip reader properly reusable when testing signed zips. This has been on trunk for a few days with no reported issues.

Comment 8

[Fred Wenzel [:wenzel]]

(In reply to comment #7)
> This has been on trunk for a few days with no reported issues.

Agreed: I just tried installing two signed xpis together with Minefield, and unlike on Fx 3.0.4, it works great.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 347871 [details] [diff] [review]
patch rev 1

Approved for 1.9.0.5, a=dveditz for release-drivers

Comment 10

[Dave Townsend [:mossop]]

Checked in to the 1.9 tree:

Checking in modules/libjar/nsJAR.cpp;
/cvsroot/mozilla/modules/libjar/nsJAR.cpp,v  <--  nsJAR.cpp
new revision: 1.132; previous revision: 1.131
done

Comment 11

[Al Billings [:abillings - ex-MoCo]]

Verified that this now works with 1.9.0.5 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5pre) Gecko/2008120105 GranParadiso/3.0.5pre.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5 ID:20090624012136

Dave, do we really still need a Litmus test or is it already covered by the automated test?

Comment 13

[Dave Townsend [:mossop]]

(In reply to comment #12)
> Dave, do we really still need a Litmus test or is it already covered by the
> automated test?

We have no automated test for the 1.9.0.x branch so a litmus test may be helpful there.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #13)
> (In reply to comment #12)
> > Dave, do we really still need a Litmus test or is it already covered by the
> > automated test?
> 
> We have no automated test for the 1.9.0.x branch so a litmus test may be
> helpful there.

But we have for 1.9.1? Have I understood your last comment correctly?

Comment 15

[Dave Townsend [:mossop]]

(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > Dave, do we really still need a Litmus test or is it already covered by the
> > > automated test?
> > 
> > We have no automated test for the 1.9.0.x branch so a litmus test may be
> > helpful there.
> 
> But we have for 1.9.1? Have I understood your last comment correctly?

That is correct. To land the automated changes on 1.9.0 would have required backporting a significant number of changes to the test harnesses.