GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Bug: Start-Trace handles file paths incorrectly by IISResetMe · Pull Request #3863 · PowerShell/PowerShell

Conversation

@IISResetMe

@IISResetMe Mathias R. Jessen (IISResetMe) commented May 25, 2017

Copy link
Copy Markdown
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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@IISResetMe
Makes Start-Trace escape file paths correctly
3b70af0 
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

Microsoft Contribution License Agreements (msftclas) commented May 25, 2017

Copy link
Copy Markdown

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-not-required label May 25, 2017
@IISResetMe Mathias R. Jessen (IISResetMe) changed the title Makes Start-Trace escape file paths correctly Bug: Start-Trace handles file paths incorrectly May 25, 2017
@daxian-dbw Dongbo Wang (daxian-dbw) assigned Travis Plunk (TravisEz13) May 26, 2017
@TravisEz13
Travis Plunk (TravisEz13) requested review from Paul Higinbotham (PaulHigin) and PowerShellTeam and removed request for PowerShellTeam May 26, 2017 19:35
PaulHigin
Paul Higinbotham (PaulHigin) suggested changes May 30, 2017
View reviewed changes

@PaulHigin Paul Higinbotham (PaulHigin) left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@IISResetMe
Merge remote-tracking branch 'refs/remotes/PowerShell/master'
cc15476
@TravisEz13 Travis Plunk (TravisEz13) added the Stale label Jul 21, 2017
@TravisEz13

Travis Plunk (TravisEz13) commented Jul 21, 2017

Copy link
Copy Markdown
Member

Will close if no response to review within a week.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@IISResetMe

Mathias R. Jessen (IISResetMe) commented Jul 24, 2017

Copy link
Copy Markdown
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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@PaulHigin

Paul Higinbotham (PaulHigin) commented Jul 24, 2017

Copy link
Copy Markdown
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.

James Truher (@JamesWTruher) Do you have any other advice such as where the test should reside?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
PaulHigin
Paul Higinbotham (PaulHigin) approved these changes Aug 4, 2017
View reviewed changes

@PaulHigin Paul Higinbotham (PaulHigin) left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@TravisEz13 Travis Plunk (TravisEz13) mentioned this pull request Aug 4, 2017
Add tests for start-trace file path handling #4504
Closed
@TravisEz13

Travis Plunk (TravisEz13) commented Aug 4, 2017

Copy link
Copy Markdown
Member

Mathias R. Jessen (@IISResetMe) Please try to write tests in the future. I've filed an issue for the missing tests. Thanks for the contribution.
IISResetMe reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@TravisEz13
Travis Plunk (TravisEz13) merged commit f15a8da into PowerShell:master Aug 4, 2017
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@IISResetMe @TravisEz13
Makes Start-Trace escape file paths correctly (PowerShell#3863)
be5c990 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@PaulHigin Paul Higinbotham (PaulHigin) PaulHigin approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@TravisEz13 Travis Plunk (TravisEz13)

Labels

Stale

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@IISResetMe @msftclas @TravisEz13 @PaulHigin
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.