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

cleanup: Filter invalid file names for inbound file transfers #112

Merged
merged 1 commit into from
Nov 5, 2020

Conversation

JFreegman
Copy link
Member

@JFreegman JFreegman commented Nov 5, 2020

Addressing #111


This change is Reviewable

@JFreegman JFreegman added the cleanup Internal code cleanup, possibly affecting semantics, e.g. deleting a deprecated feature. label Nov 5, 2020
@JFreegman JFreegman added this to the 0.8.x milestone Nov 5, 2020
Copy link

@zoff99 zoff99 left a comment

Choose a reason for hiding this comment

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

:lgtm_strong:

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained

@pull-request-attention pull-request-attention bot assigned JFreegman and unassigned zoff99 Nov 5, 2020
@zoff99
Copy link

zoff99 commented Nov 5, 2020

maybe use a whilelist of characters instead of a blacklist.
because not sure which OS has what special bad chars.

maybe '<' '>' '|' and whatnot ...

also windos filename can not contain certain chars anyway.

@JFreegman
Copy link
Member Author

JFreegman commented Nov 5, 2020

maybe use a whilelist of characters instead of a blacklist.
because not sure which OS has what special bad chars.

maybe '<' '>' '|' and whatnot ...

also windos filename can not contain certain chars anyway.

Toxic is only concerned with Posix compatibility. As far as Posix/unix-like goes, only the - character at the start of a file name and the '/' character and are disallowed. Fully portable Posix only supports ASCII alphanumeric characters and -_., but we still want to allow unicode for other languages, and there are way too many unicode characters for a whitelist. There's also probably nothing wrong with allowing punctuation. I think the focus should be on preventing malicious behaviour.

@JFreegman JFreegman merged commit 32eb7d3 into TokTok:master Nov 5, 2020
@JFreegman JFreegman deleted the file_name_filter branch November 5, 2020 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Internal code cleanup, possibly affecting semantics, e.g. deleting a deprecated feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants