Skip to content

Bug: Start-Trace handles file paths incorrectly #3863

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

Merged
merged 2 commits into from
Aug 4, 2017
Merged

Bug: Start-Trace handles file paths incorrectly #3863

merged 2 commits into from
Aug 4, 2017

Conversation

IISResetMe
Copy link
Collaborator

Start-Trace neglects qualifying arguments to the -o and -pf command line
switches, causing logman (and subsequently Start-Trace) to exit with error code
0x80070057 and return:

Error:
The argument is incorrect.

This is easy to reproduce on Windows:

PS> $dir = mkdir "a foldername with spaces" 
PS> $OutFile = Join-Path $dir trace.log
PS> Start-Trace test -OutputFilePath $OutFile -ETS
Argument 'foldername' is unknown.
Argument 'with' is unknown.
Argument 'spaces\trace.log' is unknown.
Argument 'pf' requires additional parameters.

Error:
The parameter is incorrect.

This commit moves basic input validation for output file paths to the
ProviderFilePath and OutputFilePath parameter definitions, and adds text
qualifiers (double-quotes) around input arguments

Start-Trace neglects qualifying arguments to the -o and -pf command line
switches, causing logman (and subsequently Start-Trace) to exit with
0x80070057 and return:

Error:
The argument is incorrect.

Whenever arguments to either -ProviderFilePath or -OutputFilePath
contain spaces because Start-Trace doesn't escape the path in any way.

This commit moves basic input validation for output file paths to the
ProviderFilePath and OutputFilePath parameter definitions, and adds text
qualifiers (double-quotes) around input arguments
@msftclas
Copy link

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@IISResetMe IISResetMe changed the title Makes Start-Trace escape file paths correctly Bug: Start-Trace handles file paths incorrectly May 25, 2017
@TravisEz13 TravisEz13 requested review from PowerShellTeam and PaulHigin and removed request for PowerShellTeam May 26, 2017 19:35
Copy link
Contributor

@PaulHigin PaulHigin left a comment

Choose a reason for hiding this comment

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

Please include a test for this.

@TravisEz13
Copy link
Member

Will close if no response to review within a week.

@IISResetMe
Copy link
Collaborator Author

Apologies for the delay, but I'm unsure of how to structure the test. I can't find any precedent or existing examples for tests with dependencies on Windows-specific external executables. Any guidance would be much appreciated

@PaulHigin
Copy link
Contributor

I think all you need to do is test Start-Trace with an output file path having spaces (per your repro steps). You can use "$IsWindows" to limit the test to only run on Windows platforms. Do a search for "$IsWindows" to see how other tests restrict to Windows only platforms.

@JamesWTruher Do you have any other advice such as where the test should reside?

Copy link
Contributor

@PaulHigin PaulHigin left a comment

Choose a reason for hiding this comment

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

LGTM. I don't think a test for this is all that important. This is a good fix to get in.

@TravisEz13
Copy link
Member

@IISResetMe Please try to write tests in the future. I've filed an issue for the missing tests. Thanks for the contribution.

@TravisEz13 TravisEz13 merged commit f15a8da into PowerShell:master Aug 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants