401608 - updater.exe will crash on windows if you only specify 2 args

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect, P4)

Description

[(not reading, please use [e-mail] instead)]

updater.exe will crash on windows if you only specify 2 args

here's why:

from http://lxr.mozilla.org/seamonkey/source/toolkit/mozapps/update/src/updater/updater.cpp#1207

1206   if (gSucceeded)
1207     LaunchWinPostProcess(argv[4]);

note, we already have code before calling LaunchCallbackApp to make sure we have enough args, we just need to do something similar:

1213   if (argc > 4)
1214     LaunchCallbackApp(argv[3], argc - 4, argv + 4);

Comment 1

[(not reading, please use [e-mail] instead)]

this was the root cause of bug #401327

this could be a simple fix for both branch / trunk to avoid having to fix the check_updates.sh script for 2009 -> 2010 (and for the trunk update)

Comment 2

[Robert Strong (they/them - no direct email)]

Attachment: patch

Comment 3

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

From comments in bug#401327, do we also need to have extra files present? Specifically, updater.ini and helper.exe?

Comment 4

[Robert Strong (they/them - no direct email)]

You shouldn't need helper.exe even without this patch since the code already checks and returns early if it isn't present. With this patch we just return early if updater.exe isn't launched by the application.

Comment 5

[Robert Strong (they/them - no direct email)]

bah... I was thinking about other code and it does appear to require it. Have you tried without updater.ini being present?

Comment 6

[(not reading, please use [e-mail] instead)]

Comment on attachment 286612 [details] [diff] [review]
patch

r=sspitzer, thanks robert.

Comment 7

[Robert Strong (they/them - no direct email)]

Comment on attachment 286612 [details] [diff] [review]
patch

Drivers, this is a simple / safe fix to restore the previous behavior of updater.exe when using it from the command line.

Comment 8

[Robert Strong (they/them - no direct email)]

Checked in to trunk

Checking in mozilla/toolkit/mozapps/update/src/updater/updater.cpp;
/cvsroot/mozilla/toolkit/mozapps/update/src/updater/updater.cpp,v  <--  updater.cpp
new revision: 1.30; previous revision: 1.29
done

Comment 9

[Robert Strong (they/them - no direct email)]

Comment on attachment 286612 [details] [diff] [review]
patch

Requesting 1.8.1.10. This is a simple / safe fix to restore the previous behavior of
updater.exe when using it from the command line.

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 286612 [details] [diff] [review]
patch

approved for 1.8.1.12, a=dveditz for release-drivers

Comment 11

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

curious - did this land yet on 1.8.1.12? 

Comment 12

[Samuel Sidler (old account; do not CC)]

(In reply to comment #11)
> curious - did this land yet on 1.8.1.12? 

No. If it had, it would have the fixed1.8.1.12 or verified1.8.1.12 keyword. Please don't re-request blocking on bugs that already have blocking.

Comment 13

[Daniel Veditz [:dveditz]]

joduinn: changing a blocking+ flag to a blocking? flag is generally interpreted as saying "I think this should _not_ block", otherwise it would have been left in the state you wanted.

Comment 14

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

(In reply to comment #13)
> joduinn: changing a blocking+ flag to a blocking? flag is generally interpreted
> as saying "I think this should _not_ block", otherwise it would have been left
> in the state you wanted.
> 
Sorry about that; I see it is now set back as blocking1.8.1.12+ which seems right to me.

Comment 15

[Robert Strong (they/them - no direct email)]

Checked in to MOZILLA_1_8_BRANCH

Checking in mozilla/toolkit/mozapps/update/src/updater/updater.cpp;
/cvsroot/mozilla/toolkit/mozapps/update/src/updater/updater.cpp,v  <--  updater.cpp
new revision: 1.11.4.15; previous revision: 1.11.4.14
done