Skip to content
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

*nix: dotnet subcommands incorrectly splitting quoted args containing spaces #4139

Closed
natemcmaster opened this issue Sep 1, 2016 · 16 comments · Fixed by #8753

Comments

@natemcmaster
Copy link
Contributor

commented Sep 1, 2016

Steps to reproduce

To see problem, use this:

using System;

class Program
{
    static void Main(string[] args)
    {
        for(var i = 0; i < args.Length; i++)
        {
            Console.WriteLine($"{i} = {args[i]}");
        }
    }
}

Execute dotnet run a b '" c "' d

Expected behavior

Output should be:

0 = a
1 = b
2 = " c "
3 = d

Actual behavior

0 = a
1 = b
2 = "
3 = c
4 = "
5 = d

Environment data

Repros in preview2 and preview3 nightlies, bash on Linux and OSX

dotnet --info output:

.NET Command Line Tools (1.0.0-preview3-003546)

Product Information:
 Version:            1.0.0-preview3-003546
 Commit SHA-1 hash:  c0c07ed959

Runtime Environment:
 OS Name:     ubuntu
 OS Version:  14.04
 OS Platform: Linux
 RID:         ubuntu.14.04-x64

More info

The problem appears to be somewhere in CLI. Using dotnet exec does not have this problem.

namc@ubuntu:/tmp/quoted$ dotnet exec --depsfile bin/Debug/netcoreapp1.0/quoted.deps.json --runtimeconfig bin/Debug/netcoreapp1.0/quoted.runtimeconfig.json bin/Debug/netcoreapp1.0/quoted.dll a b '" c "' d
0 = a
1 = b
2 = " c "
3 = d
@natemcmaster

This comment has been minimized.

Copy link
Contributor Author

commented Sep 1, 2016

cc @brthor

@brthor brthor self-assigned this Sep 1, 2016
@brthor

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2016

@natemcmaster What shell are you using, bash?

@natemcmaster

This comment has been minimized.

Copy link
Contributor Author

commented Sep 1, 2016

Repros on bash and zsh, Linux and macOS.

@brthor brthor assigned TheRealPiotrP and unassigned brthor Oct 21, 2016
@TheRealPiotrP TheRealPiotrP added this to the Backlog milestone Nov 12, 2016
@TheRealPiotrP TheRealPiotrP added the bug label Nov 12, 2016
@TheRealPiotrP

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2016

Moving to the backlog for the moment as we can work around this using dotnet path/to.dll. I'd love to fix it but we need to focus on issues with no workarounds :-/

@TheRealPiotrP

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2016

@natemcmaster if this IS blocking something, please let me know and we can repriorotize

natemcmaster pushed a commit to natemcmaster/dotnet-cli that referenced this issue Mar 8, 2018
@livarcocc livarcocc modified the milestones: Backlog, 2.1.3xx Mar 9, 2018
peterhuene added a commit that referenced this issue Mar 19, 2018
Fix #4139 - escape quoted strings for process start
peterhuene added a commit to peterhuene/cli that referenced this issue Mar 20, 2018
* master:
  Update translations following merge with conflicts.
  LOC CHECKIN | cli master | 20180315
  Implement the --tool-path option for the list tool command.
  Ensure tool package store root is a full path.
  Better using facing string (dotnet#8809)
  Fix list tool command tests to be localizable.
  Fix dotnet#4139 - escape quoted strings for process start
peterhuene added a commit to peterhuene/cli that referenced this issue Mar 20, 2018
peterhuene added a commit that referenced this issue Mar 23, 2018
…elease/2.1.3xx-to-master

* upstream/release/2.1.3xx:
  Try shorter test names
  Fix project type GUIDs when adding projects to solution files.
  Updating the runtime to 2.1.0-preview2-26314-02
  Remove runtime identifiers from the test project
  Update launch settings for ApplicationUrl handling
  Change command order for tools (#8862)
  Update translations following merge with conflicts.
  LOC CHECKIN | cli master | 20180315
  Implement the --tool-path option for the list tool command.
  Ensure tool package store root is a full path.
  Better using facing string (#8809)
  Fix list tool command tests to be localizable.
  Fix #4139 - escape quoted strings for process start
  Generate Microsoft.NETCoreSdk.BundledCliTools.props
peterhuene added a commit that referenced this issue Mar 23, 2018
* upstream/release/2.1.3xx:
  Try shorter test names
  Fix project type GUIDs when adding projects to solution files.
  Updating the runtime to 2.1.0-preview2-26314-02
  Remove runtime identifiers from the test project
  Update launch settings for ApplicationUrl handling
  Change command order for tools (#8862)
  Update translations following merge with conflicts.
  LOC CHECKIN | cli master | 20180315
  Implement the --tool-path option for the list tool command.
  Ensure tool package store root is a full path.
  Better using facing string (#8809)
  Fix list tool command tests to be localizable.
  Fix #4139 - escape quoted strings for process start
  Generate Microsoft.NETCoreSdk.BundledCliTools.props
peterhuene added a commit that referenced this issue Mar 23, 2018
* upstream/release/2.1.3xx:
  Try shorter test names
  Fix project type GUIDs when adding projects to solution files.
  Updating the runtime to 2.1.0-preview2-26314-02
  Remove runtime identifiers from the test project
  Update launch settings for ApplicationUrl handling
  Change command order for tools (#8862)
  Update translations following merge with conflicts.
  LOC CHECKIN | cli master | 20180315
  Implement the --tool-path option for the list tool command.
  Ensure tool package store root is a full path.
  Better using facing string (#8809)
  Fix list tool command tests to be localizable.
  Fix #4139 - escape quoted strings for process start
  Generate Microsoft.NETCoreSdk.BundledCliTools.props
@Vogel612

This comment has been minimized.

Copy link

commented Dec 9, 2018

I'm currently encountering this issue on ArchLinux using zsh, bash and sh. dotnet --info puts me at version 2.2.100 for the sdk and 2.2.0 for the host version.

 dotnet --info
.NET Core SDK (reflecting any global.json):
 Version:   2.2.100
 Commit:    b9f2fa0ca8

Runtime Environment:
 OS Name:     arch
 OS Version:  
 OS Platform: Linux
 RID:         arch-x64
 Base Path:   /opt/dotnet/sdk/2.2.100/

Host (useful for support):
  Version: 2.2.0
  Commit:  1249f08fed

.NET Core SDKs installed:
  2.2.100 [/opt/dotnet/sdk]

.NET Core runtimes installed:
  Microsoft.NETCore.App 2.2.0 [/opt/dotnet/shared/Microsoft.NETCore.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download

Using the 3.0.100-preview-009752 fixes the issue.

@peterhuene

This comment has been minimized.

Copy link
Contributor

commented Dec 26, 2018

Hi @Vogel612,

I'm unable to reproduce this issue with either 2.2.100 or 2.2.101 (the most recent SDK version), at least on macOS. Unfortunately I'm far away from my Arch system (which is powered down), but I can try to reproduce it in a VM if you like.

The fix should have made it into 2.1.300 and I see it tagged all the way through to the current release.

@externl

This comment has been minimized.

Copy link

commented Jan 15, 2019

Also experiencing this issue on Arch linux. Works correctly on macOS, Ubuntu, Debian with 2.2.100. Failing only on Arch.

@peterhuene

This comment has been minimized.

Copy link
Contributor

commented Jan 15, 2019

@externl What shell are you using?

@externl

This comment has been minimized.

Copy link

commented Jan 15, 2019

@peterhuene Using Bash 5.0.0

GNU bash, version 5.0.0(1)-release (x86_64-pc-linux-gnu)
Copyright (C) 2019 Free Software Foundation, Inc.

I tried Bash 5.0.0 on macOS and everything works correctly.

ZSH (5.6.2) on Arch fails as well.

@pepone

This comment has been minimized.

Copy link

commented Jan 15, 2019

@peterhuene testing with a simple programs that just print the argugments

archlinux% dotnet bin/Debug/netcoreapp2.2/test.dll "h x a" 
ARG 0 h
ARG 1 x
ARG 2 a
archlinux% cat Program.cs 
using System;

namespace test
{
    class Program
    {
        static void Main(string[] args)
        {
            for(int i = 0; i < args.Length; i++)
            {
                System.Console.WriteLine("ARG {0} {1}", i, args[i]);
            }
        }
    }
}
@peterhuene

This comment has been minimized.

Copy link
Contributor

commented Jan 16, 2019

Thanks for the test case. I'll take a look on my arch system now.

@peterhuene

This comment has been minimized.

Copy link
Contributor

commented Jan 16, 2019

The issue on Arch Linux is how dotnet is being packaged.

It's installing .NET Core into a non-standard location of /opt/dotnet/dotnet and then using a shell script at /usr/bin/dotnet to invoke /opt/dotnet/dotnet after setting DOTNET_ROOT to point at /opt/dotnet/dotnet. This shell script does not preserve the quotes when forwarding the arguments to /opt/dotnet/dotnet.

Possible workarounds:

  • Use /opt/dotnet/dotnet to invoke (e.g. /opt/dotnet/dotnet run foo "bar baz").
  • Edit /usr/bin/dotnet and change $@ to "$@" to preserve the quotes.

An issue should be filed with the Arch Linux bug tracker for the community/dotnet-host package regarding fixing the /usr/bin/dotnet wrapper script.

@Vogel612

This comment has been minimized.

Copy link

commented Jan 16, 2019

reported against the arch bugtracker as https://bugs.archlinux.org/task/61427

@eli-schwartz

This comment has been minimized.

Copy link

commented Jan 16, 2019

Is that wrapper script even needed? According to your suggestion to run /opt/dotnet/dotnet directly, I would expect a symlink to work without the need to set a variable indicating the root of the runtime installation. For relocatable programs, detecting the root on its own is the ideal solution.

@peterhuene

This comment has been minimized.

Copy link
Contributor

commented Jan 16, 2019

Ideally DOTNET_ROOT should be set to allow applications that launch via an apphost to locate the .NET Core runtime that is installed to a non-default location. In 2.2 you can publish a .NET Core application as RID-specific (i.e. "platform-specific") "framework-dependent" (as opposed to "self-contained") and get a native executable that can start the .NET application so that you don't need to use dotnet. The environment variable would let that framework-dependent executable know where to find the runtime.

Technically setting the environment variable isn't required to start dotnet itself since it can find the runtime relative to its location. However, imagine a scenario where dotnet build execs another process that needs to find the non-default location of the runtime. Without DOTNET_ROOT set it would not start correctly.

There was some debate as to whether or not the SDK should set DOTNET_ROOT inside the dotnet process for child processes to respect, but there were some concerns there that ultimately resulted in the SDK not doing so.

That said, I'd probably recommend the script in this case to ensure that every process started by dotnet can properly locate the runtime at its non-default location.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.