Skip to content

(#1986) Use unique temp file names in RefreshEnv.cmd #1987

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jvuori
Copy link

@jvuori jvuori commented Jan 12, 2020

Fix the issue with generating a GUID (with Powershell [guid]::NewGuid() function) and using it as a part of temporary file names.

Tried first %RANDOM% but it's not random enough. It didn't pass the test script which launches two refreshenv terminals almost at the same time. %TIME% worked better but but it's not 100% reliable either. Besides the format is locale specific so in certain countries it includes characters (colon) that can't be used in file names.

@@ -11,53 +11,63 @@
::echo "RefreshEnv.cmd only works from cmd.exe, please install the Chocolatey Profile to take advantage of refreshenv from PowerShell"
echo | set /p dummy="Refreshing environment variables from registry for cmd.exe. Please wait..."

:: Generate unique file names
for /F "tokens=* usebackq" %%F IN (`powershell -command "[guid]::NewGuid().ToString()"`) do set REFRESHENV_GUID=%%F
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would require at least a 2 second start up time for PowerShell. Not that I'm against that, but that is longer than the entire thing takes to run now. So this is more than doubling the time. Are there any other methods?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid pure CMD doesn't provide anything reliable we could really use. Sounds an odd idea but what if we just implemented some tiny win32 application (with C) that would only print its process ID and that would be used as the random part of the temporary file names? The executable would be so small that it could be even put into version control beside RefreshEnv.cmd.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL! 13 824 bytes :)

#include <stdio.h>
#include <processthreadsapi.h>

void main()
{
    printf("%d", GetCurrentProcessId());
}

gcc pid.c -o pid.exe -s -Os

for /F "tokens=* usebackq" %%F IN (`powershell -command "[guid]::NewGuid().ToString()"`) do set REFRESHENV_GUID=%%F
set REFRESHENV_ENVSET_TMP="%TEMP%\refreshenv_envset_%REFRESHENV_GUID%.tmp"
set REFRESHENV_ENVGET_TMP="%TEMP%\refreshenv_envget_%REFRESHENV_GUID%.tmp"
set REFRESHENV_ENV_CMD="%TEMP%\refreshenv_env_%REFRESHENV_GUID%.cmd"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's put this somewhere else besides temp. Somewhere more locked down, like a temp folder under the Chocolatey directories that is locked down.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@gep13 gep13 changed the title (GH-1986) Use unique temp file names in RefreshEnv.cmd (#1986) Use unique temp file names in RefreshEnv.cmd Oct 14, 2021
@gep13 gep13 linked an issue Oct 15, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refreshenv.cmd uses hard-coded temp files
3 participants