Closed Bug 1485484 Opened Last year Closed Last year

Improper use of ApplicationID plugin causing unbalanced NSIS stack

Categories

(Firefox :: Installer, enhancement)

enhancement
Not set

Tracking

()

VERIFIED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: mhowell, Assigned: mhowell)

References

Details

Attachments

(1 file)

In the function UpdateShortcutAppModelIDs [1] we make 5 calls to ApplicationID::Set where we pass it only two parameters, but it expects a third (or an empty stack otherwise). The result is an extra item being popped off the NSIS stack.

This was originally discussed and patched in bug 425167, but this is a different root cause from the issue that bug was originally filed for.

[1] https://searchfox.org/mozilla-central/rev/3fa761ade83ed0b8ab463acb057c2cf0b104689e/toolkit/mozapps/installer/windows/nsis/common.nsh#6999
In theory, the third parameter (dualMode) to the NSIS ApplicationID plugin's
Set function is optional, but the plugin assumes that either it's there or
there's nothing else on the NSIS stack, so it pops three items from the stack
unconditionally. This means that supplying only two parameters results in
one item silently being dropped from the NSIS stack. Fortunately this one
function is the only place where we were doing that, so it only became a
problem if there were an awful lot of shortcuts (around 7) to the same
installation.
Comment on attachment 9003264 [details]
Bug 1485484 - Always fill in the optional third parameter to ApplicationID::Set. r=agashlin

Adam Gashlin [:agashlin] has approved the revision.
Attachment #9003264 - Flags: review+
Pushed by mhowell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/202f0986f459
Always fill in the optional third parameter to ApplicationID::Set. r=agashlin
https://hg.mozilla.org/mozilla-central/rev/202f0986f459
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Most recent nightly update no longer has the issue for me \o/ - thanks Matt.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.