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

Disallow backslashes in file paths #2083

Merged
merged 1 commit into from Oct 15, 2017

Conversation

Projects
None yet
2 participants
@jyrkive
Member

jyrkive commented Oct 8, 2017

The intent is to avoid UMC authors on Windows from accidentally using backslashes and making add-ons that won't work correctly on GNU/Linux and macOS.

IMO, this change is risky enough that we should merge it for 1.13.11, not 1.13.10.

@jyrkive jyrkive added this to the 1.13.11 milestone Oct 8, 2017

@Arcanister

This comment has been minimized.

Show comment
Hide comment
@Arcanister

Arcanister Oct 10, 2017

Contributor

The code has a small drawback though: \ is a valid character in a filename in most file systems (unlike /), and it will no longer work, even though the code means to drop \ only while it's used on Windows.

If client side filtering of filenames is implemented then \ should not be double-filtered.

I suggest wrapping it in #ifdef so it won't be compiled in on platform where / is only path separator.

Contributor

Arcanister commented Oct 10, 2017

The code has a small drawback though: \ is a valid character in a filename in most file systems (unlike /), and it will no longer work, even though the code means to drop \ only while it's used on Windows.

If client side filtering of filenames is implemented then \ should not be double-filtered.

I suggest wrapping it in #ifdef so it won't be compiled in on platform where / is only path separator.

@jyrkive

This comment has been minimized.

Show comment
Hide comment
@jyrkive

jyrkive Oct 10, 2017

Member

I consider blocking backslashes on other platforms a bonus. Windows doesn't allow backslashes in file names, and therefore it's a good thing that UMC authors on other platforms can't accidentally use file names with backslashes.

Member

jyrkive commented Oct 10, 2017

I consider blocking backslashes on other platforms a bonus. Windows doesn't allow backslashes in file names, and therefore it's a good thing that UMC authors on other platforms can't accidentally use file names with backslashes.

Disallow specifying file paths with backslashes
Backslashes only work on Windows. A UMC author may accidentally use them in
their add-on that won't then work properly on GNU/Linux and macOS.
Just disallow them on all platforms to avoid problems.

@jyrkive jyrkive merged commit 1b9c07e into master Oct 15, 2017

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
@jyrkive

This comment has been minimized.

Show comment
Hide comment
@jyrkive

jyrkive Oct 15, 2017

Member

Merged.

Member

jyrkive commented Oct 15, 2017

Merged.

@soliton- soliton- deleted the no_backslashes branch Jan 14, 2018

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