Skip to content
This repository has been archived by the owner on Dec 18, 2017. It is now read-only.

Script blocks with spaces in arguments will fail. #876

Closed
troydai opened this issue Nov 10, 2014 · 6 comments
Closed

Script blocks with spaces in arguments will fail. #876

troydai opened this issue Nov 10, 2014 · 6 comments
Assignees
@dougbu
Labels
3 - Done bug
Milestone
1.0.0-beta6

Comments

@troydai
Copy link
Contributor

troydai commented Nov 10, 2014

ScriptExecutor will remove the quotes in script block unconditionally, as a result user can't wrap argument in quotes.
@troydai troydai changed the title ScriptExecutor will remove the quotes in script block unconditionally Script blocks with spaces in arguments will fail. Nov 10, 2014
@dougbu dougbu self-assigned this Dec 1, 2014
dougbu added a commit that referenced this issue Dec 1, 2014
@dougbu
Fix #876, Correct handling of quotation marks in commands
c9e7ef8 
- remove extra quotes around `cmd /c` arguments
 - prevented && and similar special cases
- don't strip quotes surrounding command-line arguments
 - prevented command paths and arguments containing spaces
- `CommandGrammar` may need additional generalizations
 - e.g. unquoted term can't contain more than one escape sequence
 - but it's probably good enough for now
@dougbu dougbu mentioned this issue Dec 1, 2014
Closed
@troydai
Copy link
Contributor Author

troydai commented Mar 5, 2015

@dougbu is this issue fixed?

@dougbu
Copy link
Member

dougbu commented Mar 5, 2015

the issue is not fixed. for example, if I add

    "scripts": {
        "prebuild": [
            "\"C:/Program Files (x86)/Git/bin/cat\" project.json"
        ]
    }

to a project.json and kpm build --quiet in the containing folder, the output is

'C:\Program' is not recognized as an internal or external command,
operable program or batch file.
The 'prebuild' script failed with status code 1.

has this bug been triaged and assigned to a milestone?

@glennc glennc added this to the 1.0.0 milestone Mar 13, 2015
@muratg muratg modified the milestones: 1.0.0, 1.0.0-beta5 Mar 20, 2015
dougbu added a commit that referenced this issue Mar 29, 2015
@dougbu
Fix #876, Correct handling of quotation marks in commands
bc37ead 
- remove extra quotes around `cmd /c` arguments
 - prevented && and similar special cases
- don't strip quotes surrounding command-line arguments
 - prevented command paths and arguments containing spaces
- `CommandGrammar` may need additional generalizations
 - e.g. unquoted term can't contain more than one escape sequence
 - but it's probably good enough for now
@muratg
Copy link
Contributor

muratg commented May 18, 2015

@dougbu Beta5

@dougbu
Copy link
Member

dougbu commented May 18, 2015

@muratg OK I'll update my old branches...

dougbu added a commit that referenced this issue May 22, 2015
@dougbu
Fix #876, Correct handling of quotation marks in commands
2cfc316 
- remove extra quotes around `cmd /c` arguments
 - prevented && and similar special cases
- don't strip quotes surrounding command-line arguments
 - prevented command paths and arguments containing spaces
- `CommandGrammar` may need additional generalizations
 - e.g. unquoted term can't contain more than one escape sequence
 - but it's probably good enough for now

nit:
- let VS do its thing with test services
dougbu added a commit that referenced this issue May 22, 2015
@dougbu
Fix #876, Correct handling of quotation marks in commands
4c0daa8 
- remove extra quotes around `cmd /c` arguments
 - prevented && and similar special cases
- don't strip quotes surrounding command-line arguments
 - prevented command paths and arguments containing spaces

note:
- `CommandGrammar` may need additional generalizations
 - e.g. unquoted term can't contain more than one escape sequence
 - but it's probably good enough for now
- _not_ adding quotes around variables replacements inserted into commands
 - no way of determining if replacement is already quoted
 - basically, leave it to project.json author to perform appropriate quoting for their platform

nit:
- let VS do its thing with test services
dougbu added a commit that referenced this issue May 23, 2015
@dougbu
Fix #876, Correct handling of quotation marks in commands
e2f9241 
- remove extra quotes around `cmd /c` arguments
 - prevented && and similar special cases
- don't strip quotes surrounding command-line arguments
 - prevented command paths and arguments containing spaces

note:
- `CommandGrammar` may need additional generalizations
 - e.g. unquoted term can't contain more than one escape sequence
 - but it's probably good enough for now
- _not_ adding quotes around variables replacements inserted into commands
 - no way of determining if replacement is already quoted
 - basically, leave it to project.json author to perform appropriate quoting for their platform

nit:
- let VS do its thing with test services
dougbu added a commit that referenced this issue May 24, 2015
@dougbu
Fix #876, Correct handling of quotation marks in commands
d8cb409 
- remove extra quotes around `cmd /c` arguments
 - prevented && and similar special cases
- don't strip quotes surrounding command-line arguments
 - prevented command paths and arguments containing spaces

note:
- `CommandGrammar` may need additional generalizations
 - e.g. unquoted term can't contain more than one escape sequence
 - but it's probably good enough for now
- _not_ adding quotes around variables replacements inserted into commands
 - no way of determining if replacement is already quoted
 - basically, leave it to project.json author to perform appropriate quoting for their platform

nit:
- let VS do its thing with test services
@davidfowl davidfowl added the bug label May 24, 2015
dougbu added a commit that referenced this issue May 24, 2015
@dougbu
Fix #876, Correct handling of quotation marks in commands
8dda194 
- remove extra quotes around `cmd /c` arguments
 - prevented && and similar special cases
- don't strip quotes surrounding command-line arguments
 - prevented command paths and arguments containing spaces

note:
- `CommandGrammar` may need additional generalizations
 - e.g. unquoted term can't contain more than one escape sequence
 - but it's probably good enough for now
- _not_ adding quotes around variables replacements inserted into commands
 - no way of determining if replacement is already quoted
 - basically, leave it to project.json author to perform appropriate quoting for their platform

nit:
- let VS do its thing with test services
@dougbu
Copy link
Member

dougbu commented May 26, 2015

Per offline conversation with @davidfowl, postponing to Beta 6. Need to research Linux / Mac failures with fix (PR #1931) in place.

@dougbu dougbu modified the milestones: 1.0.0-beta6, 1.0.0-beta5 May 26, 2015
dougbu added a commit that referenced this issue Jun 1, 2015
@dougbu
Fix #876, Correct handling of quotation marks in commands
812e080 
- remove extra quotes around `cmd /c` arguments
 - prevented && and similar special cases
- don't strip quotes surrounding command-line arguments
 - prevented command paths and arguments containing spaces

note:
- `CommandGrammar` may need additional generalizations
 - e.g. unquoted term can't contain more than one escape sequence
 - but it's probably good enough for now
- _not_ adding quotes around variables replacements inserted into commands
 - no way of determining if replacement is already quoted
 - basically, leave it to project.json author to perform appropriate quoting for their platform

nit:
- let VS do its thing with test services
dougbu added a commit that referenced this issue Jun 2, 2015
@dougbu
Fix #876, Correct handling of quotation marks in commands
3b1e733 
- remove extra quotes around `cmd /c` arguments
 - prevented && and similar special cases
- don't strip quotes surrounding command-line arguments
 - prevented command paths and arguments containing spaces

note:
- `CommandGrammar` may need additional generalizations
 - e.g. unquoted term can't contain more than one escape sequence
 - but it's probably good enough for now
- _not_ adding quotes around variables replacements inserted into commands
 - no way of determining if replacement is already quoted
 - basically, leave it to project.json author to perform appropriate quoting for their platform

nit:
- let VS do its thing with test services
dougbu added a commit that referenced this issue Jun 2, 2015
@dougbu
Fix #876, Correct handling of quotation marks in commands
b2bb99d 
- don't strip quotes surrounding command-line arguments when used in `scripts`
 - prevented command paths and grouping arguments containing spaces
 - pass `preserveSurroundingQuotes` into `CommandGrammar` to control stripping
- use `cmd /s /c` when executing `scripts` commands on Windows
 - allows `&&` and similar
- add functional tests of commands and scripts (together with `dnu restore`)
 - no `dnu restore` for .NET Core due to extra time required

note:
- `CommandGrammar` may need additional generalizations
 - e.g. unquoted term can't contain more than one escape sequence
 - but it's probably good enough for now
- _not_ adding quotes around variables replacements inserted into commands
 - no way of determining if replacement is already quoted
 - basically, leave it to project.json author to perform appropriate quoting for their platform

nits:
- let VS do its thing with test services
- make `CommandGrammar` constructor private; used only in `Process()` method
dougbu added a commit that referenced this issue Jun 2, 2015
@dougbu
Fix #876, Correct handling of quotation marks in commands
1f0f120 
- don't strip quotes surrounding command-line arguments when used in `scripts`
 - prevented command paths and grouping arguments containing spaces
 - pass `preserveSurroundingQuotes` into `CommandGrammar` to control stripping
- use `cmd /s /c` when executing `scripts` commands on Windows
 - allows `&&` and similar
- add functional tests of commands and scripts (together with `dnu restore`)
 - no `dnu restore` for .NET Core due to extra time required

note:
- `CommandGrammar` may need additional generalizations
 - e.g. unquoted term can't contain more than one escape sequence
 - but it's probably good enough for now
- _not_ adding quotes around variables replacements inserted into commands
 - no way of determining if replacement is already quoted
 - basically, leave it to project.json author to perform appropriate quoting for their platform

nits:
- let VS do its thing with test services
- make `CommandGrammar` constructor private; used only in `Process()` method
dougbu added a commit that referenced this issue Jun 3, 2015
@dougbu
Fix #876, Correct handling of quotation marks in commands
3ab4f36 
- don't strip quotes surrounding command-line arguments when used in `scripts`
 - prevented command paths and grouping arguments containing spaces
 - pass `preserveSurroundingQuotes` into `CommandGrammar` to control stripping
- use `cmd /s /c` when executing `scripts` commands on Windows
 - allows `&&` and similar
- add functional tests of commands and scripts (together with `dnu restore`)
 - no `dnu restore` for .NET Core due to extra time required

note:
- `CommandGrammar` may need additional generalizations
 - e.g. unquoted term can't contain more than one escape sequence
 - but it's probably good enough for now
- _not_ adding quotes around variables replacements inserted into commands
 - no way of determining if replacement is already quoted
 - basically, leave it to project.json author to perform appropriate quoting for their platform

nits:
- let VS do its thing with test services
- make `CommandGrammar` constructor private; used only in `Process()` method
dougbu added a commit that referenced this issue Jun 3, 2015
@dougbu
Fix #876, Correct handling of quotation marks in commands
96f85ca 
- don't strip quotes surrounding command-line arguments when used in `scripts`
 - prevented command paths and grouping arguments containing spaces
 - pass `preserveSurroundingQuotes` into `CommandGrammar` to control stripping
- support expected shell capabilities such as quoting command and redirections
 - use `cmd /s /c` when executing `scripts` on Windows
 - use `/bin/bash -c` when executing `scripts` on Linux
- add functional tests of commands and scripts (together with `dnu restore`)
 - no `dnu restore` for .NET Core due to extra time required

note:
- `CommandGrammar` may need additional generalizations
 - e.g. unquoted term can't contain more than one escape sequence
 - but it's probably good enough for now
- _not_ adding quotes around variables replacements inserted into commands
 - no way of determining if replacement is already quoted
 - basically, leave it to project.json author to perform appropriate quoting for their platform

nits:
- let VS do its thing with test services
- make `CommandGrammar` constructor private; used only in `Process()` method
- update a few `ScriptExecutor` comments, long lines, et cetera
@dougbu dougbu closed this as completed in 90f1f03 Jun 9, 2015
@dougbu
Copy link
Member

dougbu commented Jun 9, 2015

90f1f03

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@dougbu dougbu

Labels
3 - Done bug
Projects
None yet
Milestone
1.0.0-beta6
Development

No branches or pull requests
5 participants
@davidfowl @glennc @troydai @muratg @dougbu