Skip to content

Conversation

@kevin-bates
Copy link
Member

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.

When an invalid character is encountered during the rename operation, a message similar to the following will occur:
Screen Shot 2020-01-28 at 1 55 50 PM

Fixes #5190

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 jupyter#5190

for char in invalid_chars:
if char in path:
raise web.HTTPError(400, u"Path '{}' contains invalid characters {} relative to its platform. "
Copy link
Member

Choose a reason for hiding this comment

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

How about Path '{}' contains characters that are invalid for the filesystem. Path names on this filesystem cannot contain any of the following characters: {}.? I'm trying to make the message simpler, though it seems a little less actionable too (though in the validation method, it's not clear exactly who is calling us and what the recovery procedure should be...)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you - that reads much more cleanly. Here's a screen shot with the update:
Screen Shot 2020-01-28 at 3 25 12 PM

Previously, I was taking advantage of the parenthesis that sets print with. That could now be argued as unnecessary. If there's an easy pythonic way to remove those, I can do that - otherwise it seems okay to me.

Copy link
Member

Choose a reason for hiding this comment

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

You could just define it as a string instead of a tuple.

Copy link
Member

Choose a reason for hiding this comment

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

invalid_chars = ':><*?|"'

Copy link
Member

@jasongrout jasongrout left a comment

Choose a reason for hiding this comment

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

The code looks good, though I can't test on a mac very well. I wrote an inline comment about the error message for consideration.

@lresende lresende added this to the 6.0.4 milestone Jan 31, 2020
@kevin-bates kevin-bates merged commit df887b2 into jupyter:master May 5, 2020
@kevin-bates kevin-bates deleted the validate-rename branch January 29, 2021 23:53
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All notebook data is lost after renaming

3 participants