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

export & rm: correctly handle paths containing whitespace and quotes #1102

Merged
merged 2 commits into from
Feb 16, 2024

Conversation

mrnerdhair
Copy link
Contributor

@mrnerdhair mrnerdhair commented Dec 18, 2023

In order to reliably handle paths with whitespace, quotes, or nonprintable characters, find, xargs, and grep should be configured to separate them using the NUL character rather than using newlines or ad-hoc escaping. (This is documented in the shellcheck linting rule SC2038, which documents a real issue and should not be disabled.)

Update:

In order to reliably process paths with whitespace or quotes to xargs without using non-POSIX features, each character must be escaped. (Quoting will handle whitespace, but not filenames with quotes in them.)

@mrnerdhair mrnerdhair changed the title export: correctly handle paths containing whitespace, quotes, or nonp… export: correctly handle paths containing whitespace, quotes, or nonprintable characters Dec 18, 2023
Copy link

@lrdass lrdass left a comment

Choose a reason for hiding this comment

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

closes #1097
nice solution!

@89luca89
Copy link
Owner

89luca89 commented Feb 1, 2024

Hi @mrnerdhair the problem I see here is that grep -Z is not POSIX supported
So this would fail for example in an alpine container:

image

@mrnerdhair
Copy link
Contributor Author

Interesting; I'll see if I can't find a workaround.

@mrnerdhair mrnerdhair force-pushed the patch-1 branch 2 times, most recently from 7e2e43b to 75af46b Compare February 2, 2024 01:21
In order to reliably process paths with whitespace or quotes to xargs without using non-POSIX features, each character must be escaped. (Quoting will handle whitespace, but not filenames with quotes in them.)
@mrnerdhair
Copy link
Contributor Author

mrnerdhair commented Feb 2, 2024

I made it work on Alpine by using xargs -d "\n" before realizing that in terms of strict POSIX compliance that's verboten as well. In fact, there's no way at all to use a different delimiter! That leaves quoting and escaping, and quoting is hard because you have to have special cases for quoting quotes. So escaping it is, and while sed is very happy to throw a backslash in front of literally every character for us it does hurt me a little inside.

Also this fancy new solution now can't handle filenames containing newline characters, so I guess I'll just label people who use those kind of filenames as social deviants who inherently deserve to be shunned.

@mrnerdhair mrnerdhair changed the title export: correctly handle paths containing whitespace, quotes, or nonprintable characters export: correctly handle paths containing whitespace and quotes Feb 2, 2024
In order to reliably process paths with whitespace or quotes to xargs without using non-POSIX features, each character must be escaped. (Quoting will handle whitespace, but not filenames with quotes in them.)
@mrnerdhair
Copy link
Contributor Author

Also found some more xargs commands with the same issues in distrobox-rm; same solution, so I threw in another commit to address those as well.

@mrnerdhair mrnerdhair changed the title export: correctly handle paths containing whitespace and quotes export & rm: correctly handle paths containing whitespace and quotes Feb 2, 2024
@89luca89 89luca89 linked an issue Feb 2, 2024 that may be closed by this pull request
@89luca89
Copy link
Owner

Thanks a lot @mrnerdhair ! ❤️

@89luca89 89luca89 merged commit d1a0f5c into 89luca89:main Feb 16, 2024
15 checks passed
AbduEhab added a commit to AbduEhab/distrobox that referenced this pull request Nov 7, 2024
I ran into an issue when exporting. I mentioned it here: 89luca89#1097 (comment)

I have reworked a small part of the code to utilize the `-0` option of `xargs`
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.

[Error] xargs parsing error at exporting app
3 participants