-
Notifications
You must be signed in to change notification settings - Fork 93
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
tmpNameSync: Bad result if the TMP/TEMP env. variables are enclosed in quotes #246
tmpNameSync: Bad result if the TMP/TEMP env. variables are enclosed in quotes #246
Comments
…so sanitize dir option and template option
This also applies to the dir option, the template option and the name option. |
…so sanitize dir option, the template option and the name option
The downside of this is that the user may no longer include a single single quote or a single double quote in the path. But this seems to be a rather fringe use case to me, so we will not support it any more. |
Please, please don't make any changes; I was WRONG. Quotes are part of the string I suppose.
I am so so sorry to report this issue and wasted your time. |
Way too late 😄. This has already been merged and I think that it is ok handling this rather fringe case. The user might also be passing in a string for template or name or dir that might contain single quotes or double quotes. |
…gle quotes from os.tmpdir also sanitize dir option, the template option and the name option" This reverts commit c8823e5. Single quotes must not be removed from paths because they are valid (even if hard to use) on all OSes. Double quotes are only disallowed on Windows, but tmp should not change any arg it gets; instead, it should rely on the underlying fs API to fail with an error that the user needs to fix.
Operating System
NodeJS Version
Tmp Version
TBD:0.1.0
Expected Behavior
TBD: What have you expected tmp to do?
Create proper tmp-name without quotes
Experienced Behavior
TBD: What did actually happen?
Created path names with embedded quotes if the env. vars TMP and TEMP has spaces in it. I have a space in my user name and we have to use our own TMP directories rather than using the system ones. To reproduce the problem in a shell/cmd set the TMP/TEMP variables to any path with spaces in it
set TMP="C:\users\xxx\T M P"
set TEMP="C:\users\xxx\T M P"
node test.js
The above commands creates the following output. Yes, I can filter the path-name but it was unexpected
And my test.js looks like
The text was updated successfully, but these errors were encountered: