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

Upload fails if path contains a space #958

Open
RalphSommerer opened this issue Sep 18, 2020 · 6 comments
Open

Upload fails if path contains a space #958

RalphSommerer opened this issue Sep 18, 2020 · 6 comments
Assignees
Labels
os: windows Specific to Windows operating system topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project

Comments

@RalphSommerer
Copy link

RalphSommerer commented Sep 18, 2020

Describe the problem

Upload fails if the upload command contains a space character in the command name:

C:\Users\Some Name\Documents\Arduino\PoxA880>\stuff\ArduinoCli\arduino-cli upload -v
"C:\Users\Some Name\AppData\Local\Arduino15\packages\STM32\tools\STM32Tools\1.4.0/tools/win/stm32CubeProg.bat" 0 "C:\Users\Some Name\Documents\Arduino\PoxA880\build\STM32.stm32.GenF1/PoxA880.ino.bin" -g
'C:\Users\Some' is not recognized as an internal or external command,
operable program or batch file.
Error during Upload: uploading error: uploading error: exit status 1

Upload process fails with the complaint that a command named 'C:\Users\Some' does not exist.
It appears that the command is presumed to end at the first space character, when in actuality it continues with more stuff after the space (see full command above: "Some" is first half of the folder name "Some Name")

Expected behavior

Upload starts irrespective of whether path contains a space.

It may still fail for reasons unrelated, though, but that's not the point.

Arduino CLI version

0.13.0

Operating system

Windows

Operating system version

10, (OS Build 19041.508)

Additional context

Additional reports:

@matthijskooijman
Copy link
Collaborator

I suspect that this might not be a problem in arduino-cli, but in the "stm32CubeProg.bat" script that is called by arduino-cli. It looks like the argument to that script is properly quoted, so I suspect that that script is missing quotes. If so, this is a bug to report to the STM32 core you are using.

Looking at that script, I indeed think that there is quoting missing here: https://github.com/stm32duino/Arduino_Tools/blob/d13e554308d53f6af62071a625d1ae37a299d823/win/stm32CubeProg.bat#L74

@RalphSommerer
Copy link
Author

Actually, yes, there is a problem with my batch file so you may be right. But also note, that what you see is not what you get:

func runTool(..) in github.com/arduino/arduino-cli/blob/master/commands/upload/upload.go

cmdArgs, err := properties.SplitQuotedString(cmdLine, `"'`, false)

...

// Run Tool
if verbose {
	outStream.Write([]byte(fmt.Sprintln(cmdLine)))   <------
}
cmd, err := executils.NewProcess(cmdArgs...)  <------
if err != nil {
	return fmt.Errorf("cannot execute upload tool: %s", err)
}

cmdLine is printed
cmdArgs is executed

Still, if you're confident that NewProcess handles spaces in commands (1st arg) correctly then you needn't bother checking any further and can safely close this matter.

Thanks.

@matthijskooijman
Copy link
Collaborator

Still, if you're confident that NewProcess handles spaces in commands (1st arg) correctly then you needn't bother checking any further and can safely close this matter.

Yeah, I think that just forwards to the go utils which should know how to handle the split array. On e.g. Linux, the split version (array of arguments) is actually passed as an array to the OS, so no further space-splitting happens (this is traditionally done by the shell before passing the array to the OS, but here go does it). On Windows, the OS only accepts a command string, so NewProcess will need to join the array into a command string again, but I'm pretty confident that it will get escaping right.

There's still a chance that SplitQuotedString messes up, but I also think it's ok.

To doublecheck, you could modify the .bat file to print each argument on a separate line, then you'll be sure whether the spaces are correctly passed to the .bat file or not.

@per1234
Copy link
Contributor

per1234 commented Sep 30, 2020

It's this bug: arduino/arduino-builder#316

Which, unfortunately, is a Go bug: golang/go#17149

I can reproduce it even with stm32CubeProg.bat completely stubbed out.

This only happens under the following conditions:

  • Batch file is used
  • There is a space in the path to the batch file
  • There is a space in the parameter (e.g., "path/with space/foo.bat" "bar" "baz" is fine, but "path/with space/foo.bat" "bar" "baz asdf" breaks)

So if you can remove any of those factors, the problem is solved. You might use an executable in place of the batch file. Or you might rework the batch file so that it doesn't require the parameter with the problematic path. I'm not sure how the latter could be done with stm32CubeProg.bat, but I haven't looked at it closely.

Here's an example of someone managing to rework their batch file to get around it:
https://forum.arduino.cc/t/saving-pattern-results-in-custom-platforms-txt/674950/6
I don't think that approach is of any use here because the problematic parameter is the path to the binary.

@per1234 per1234 added component/core os: windows Specific to Windows operating system labels Sep 30, 2020
@matthijskooijman
Copy link
Collaborator

Wow, I would have expected go to get this bit right, but it seems that the problem is that there is no standard way to encode arguments, just away that "most" programs use, except that cmd.exe, which executes batch files, does it differently apparently. Or, reading more closely, executing a batch file actually runs the full commandline through the normal shell parsing rules (I think, possibly twice), including any file redirection and whatnot, which might also be problematic in addition to spaces.

Hm, no time to really dig in to what golang does exactly and what a workaround would be for the STM core...

@cmaglie
Copy link
Member

cmaglie commented Oct 1, 2020

https://docs.microsoft.com/it-it/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way

maybe we can try to do this extra-quoting while running cmd.exe...
One think that I did not understand is: does the Arduino IDE runs it correctly? I mean is this a problem only for Go?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
os: windows Specific to Windows operating system topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Development

No branches or pull requests

7 participants