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.

Sign up for GitHub

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

Handle files with spaces in the name #44

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Handle files with spaces in the name #44

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions tools/plugin-release
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ def _do_build(repo, ver):
append = False
break
if append:
if " " in ls_file: ls_file = "\""+ls_file+"\""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't other characters than space cause issues?
Looking at a more "generic" solution, I've found the following SO answer: https://stackoverflow.com/a/35857 - the best ay seems to use https://docs.python.org/3/library/shlex.html#shlex.quote. Can you test that on a Windows with a space and see if issues really gets fixed? They say it's "not guaranteed to be correct" on windows, but it may works.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe can we act on the subprocess call, see that other answer to the same original question on SO: https://stackoverflow.com/a/35858

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not really know how python is bundled in OS distributions, but as the 3.3. version has been released on 2012, I guess the best solution is to use shlex.quote.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the best ay seems to use https://docs.python.org/3/library/shlex.html#shlex.quote. Can you test that on a Windows with a space and see if issues really gets fixed? They say it's "not guaranteed to be correct" on windows, but it may works.

"Executing commands quoted by this module on such shells can open up the possibility of a command injection vulnerability."

Seems like a pretty big risk to use it if it could have security issues on Windows.

Or maybe can we act on the subprocess call, see that other answer to the same original question on SO: https://stackoverflow.com/a/35858

Seems better. I don't know Python at all, but I'll look at it later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'm not sure either solve the issue. I'm not trying to sanitize the entire argument, I just need to ensure files with spaces in their name are treated like a single file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess using the process is the way to go. Could you give that a quick try?

paths.append(ls_file)

archive_cmd_pattern = 'git archive --prefix=%s/ %s %s | bzip2 > %s'
Expand Down