Skip to content

Conversation

@hrumhurum
Copy link
Contributor

When the DotNet.Script solution resides in a folder containing a space character in its name, an avalanche of test failures occurs due to the absence of escaping of command line arguments.

The enclosed pull request fixes all those issues.

Besides tests, there was a similar issue in DotNetRestorer class that was fixed as well by employing a batte-tested string escaping algorithm from Gapotchenko.FX.Diagnostics.CommandLine package.

An intricate detail of NuGet's -s argument is that it may end with \ character (e.g. the last separator in a folder path), so such a naive escaping approach as $"-s \"{s}\"" would not work here, as it will cause an unanticipated escape of the escape character ". Hence the usage of a proper escape function here to isolate the customer-supplied data.

@filipw
Copy link
Member

filipw commented Aug 17, 2021

Thanks for catching this!

Besides tests, there was a similar issue in DotNetRestorer class that was fixed as well by employing a batte-tested string escaping algorithm from Gapotchenko.FX.Diagnostics.CommandLine package.

One note here, I would prefer to inline the escaping logic because the dependency model package ships with OmniSharp and we intentionally do not want to create extra dependencies for OmniSharp.

@hrumhurum
Copy link
Contributor Author

I would prefer to inline the escaping logic because the dependency model package ships with OmniSharp and we intentionally do not want to create extra dependencies for OmniSharp.

Done. The corresponding code is now inlined.

@filipw filipw merged commit bf52486 into dotnet-script:master Aug 18, 2021
@filipw
Copy link
Member

filipw commented Aug 18, 2021

thanks!

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.

3 participants