Skip to content

Commit df887b2

Browse files
authored
Check for invalid characters prior to rename (#5193)
* Check for invalid characters prior to rename On Windows platforms, rename operations containing ':' are problematic since the operation succeeds, but creates an unintended "alternate data stream" that gives the user the impression their file contents have been deleted. This change performs cursory validation of the destination name on platforms in which issues can occur. Currently, non-Windows systems don't require this level of validation. Fixes #5190 * Apply changes per review comments
1 parent ec5131c commit df887b2

File tree

2 files changed

+29
-0
lines changed

2 files changed

+29
-0
lines changed

notebook/services/contents/filemanager.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,10 @@ def rename_file(self, old_path, new_path):
558558
if new_path == old_path:
559559
return
560560

561+
# Perform path validation prior to converting to os-specific value since this
562+
# is still relative to root_dir.
563+
self._validate_path(new_path)
564+
561565
new_os_path = self._get_os_path(new_path)
562566
old_os_path = self._get_os_path(old_path)
563567

@@ -586,3 +590,23 @@ def get_kernel_path(self, path, model=None):
586590
else:
587591
parent_dir = ''
588592
return parent_dir
593+
594+
@staticmethod
595+
def _validate_path(path):
596+
"""Checks if the path contains invalid characters relative to the current platform"""
597+
598+
if sys.platform == 'win32':
599+
# On Windows systems, we MUST disallow colons otherwise an Alternative Data Stream will
600+
# be created and confusion will reign! (See https://github.com/jupyter/notebook/issues/5190)
601+
# Go ahead and add other invalid (and non-path-separator) characters here as well so there's
602+
# consistent behavior - although all others will result in '[Errno 22]Invalid Argument' errors.
603+
invalid_chars = '?:><*"|'
604+
else:
605+
# On non-windows systems, allow the underlying file creation to perform enforcement when appropriate
606+
invalid_chars = ''
607+
608+
for char in invalid_chars:
609+
if char in path:
610+
raise web.HTTPError(400, "Path '{}' contains characters that are invalid for the filesystem. "
611+
"Path names on this filesystem cannot contain any of the following "
612+
"characters: {}".format(path, invalid_chars))

notebook/services/contents/tests/test_manager.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,11 @@ def test_rename(self):
522522
# Fetching the notebook under the new name is successful
523523
assert isinstance(cm.get("changed_path"), dict)
524524

525+
# Test validation. Currently, only Windows has a non-empty set of invalid characters
526+
if sys.platform == 'win32' and isinstance(cm, FileContentsManager):
527+
with self.assertRaisesHTTPError(400):
528+
cm.rename("changed_path", "prevent: in name")
529+
525530
# Ported tests on nested directory renaming from pgcontents
526531
all_dirs = ['foo', 'bar', 'foo/bar', 'foo/bar/foo', 'foo/bar/foo/bar']
527532
unchanged_dirs = all_dirs[:2]

0 commit comments

Comments
 (0)