Stop! Tickets are now managed at GitHub.

Please enter new tickets, and find and edit existing tickets there:


Ticket #25 (closed defect: fixed)

Opened 5 years ago

Last modified 5 years ago

Installer sets(!) the path instead of properly appending

Reported by: torsten.kemps-benedix@… Owned by: refold
Priority: critical Milestone:
Component: Windows installer Keywords:
Cc: the.dead.shall.rise@…

Description

The windows PATH environment variable is set instead of appended to. This makes the complete system unusable.

Change History

  Changed 5 years ago by refold

  • status changed from new to assigned

What version of Windows are you using?

  Changed 5 years ago by refold

Quoting the above link:

Warning this code will replace paths rather than append if the existing path exceeds the maximum string length in the NSIS build you are using.

And by default, the maximum string length is 1024.

  Changed 5 years ago by claus

Permanently messing up the PATH is a serious issue, yet the download page still doesn't show even a warning! Please fix that first, unless the updated installer already exists.

Could you read PATH directly and check for error status before setting it?

The comments I see in that NSIS page make me doubt that NSIS is a wise choice, btw (certainly those scripts are dubious). They are aware of the issue, but only offer a warning in the docs, rather than a guard in the code that would avoid the erroneous setting. They've decided that any attempt to set an environment variable should also lead to some cleanup (suggesting that their code would fail on valid, but non-cleaned PATHs), as well as moving the added path to the front or back of PATH (instead of preserving the order if the entry already exists).

Btw, it might be useful if the download page automatically listed the active ticket numbers for each installer.

  Changed 5 years ago by refold

  • cc the.dead.shall.rise@… added

We can use a special build of NSIS that has maximum string length set to 8192:

http://nsis.sourceforge.net/Special_Builds

Could you read PATH directly and check for error status before setting it?

Probably; I'll look into it.

  Changed 5 years ago by anonymous

8192

yes, please do it

  Changed 5 years ago by refold

The installer was rebuilt using the special NSIS build mentioned above:

http://code.haskell.org/~refold/HaskellPlatform-2009.2.0-setup.exe

  Changed 5 years ago by dons

  • priority changed from blocker to critical

Platform site now distributing the new RC:

http://hackage.haskell.org/platform/2009.2.0/HaskellPlatform-2009.2.0-r1-setup.exe

Please close if the issue is fixed.

  Changed 5 years ago by refold

  • status changed from assigned to closed
  • resolution set to fixed

The new installer has been uploaded to the platform web site.

follow-up: ↓ 11   Changed 5 years ago by refold

And it looks like 8192 is the maximum allowed length for an environment variable in Windows (http://support.microsoft.com/kb/830473), so we don't have to do any additional checks.

in reply to: ↑ 10   Changed 5 years ago by claus

And it looks like 8192 is the maximum allowed length for an environment variable in Windows (http://support.microsoft.com/kb/830473), so we don't have to do any additional checks.

If I understand the warning correctly, EnvVarUpdate will always do the wrong thing when the limit is reached, no matter what the limit is (and apparently, 2k would have been enough for earlier versions of windows).

So you've got a workaround that makes the bug less likely to appear, but no fix yet. I still recommend doing the check, if at all possible, to ensure that, at worst, PATH isn't meddled with if the limit is reached, but not emptied.

  Changed 5 years ago by refold

EnvVarUpdate? does the wrong thing only when length(PATH) > length(STR_MAX), where STR_MAX is the maximum string length in the NSIS build we're using (chosen at the configure stage). So we shouldn't need additional checks, at least in current Windows versions.

Note: See TracTickets for help on using tickets.