fix(files): mitigate issues with special chars in file names #10673
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(tests were failing because base64 was putting "+" in the URL. Base64URL fixes this.) |
mrclay
commented
Dec 24, 2016
| // Using special characters to test against files that have been | ||
| // uploaded prior to implementation of filename sanitization | ||
| // See #10608 | ||
| $file->setFilename("foo'baž.txt"); |
There was a problem hiding this comment.
I think filesystem name mapping issues are causing the tests to not find this file, so I'm going to try reading the name from disk.
mrclay
force-pushed
the
serve_filename_22
branch
from
6dd4aaf
to
e89ec07
Compare
hypeJunction
approved these changes
Dec 25, 2016
mrclay
commented
Dec 25, 2016
| $file->setFilename('foo'); | ||
| $dir = dirname($file->getFilenameOnFilestore()); | ||
| $files = glob("$dir/bing*"); | ||
| $filename = basename($files[0]); |
There was a problem hiding this comment.
I should just create the file here and clean up after.
mrclay
force-pushed
the
serve_filename_22
branch
from
e89ec07
to
8a9dd15
Compare
Relative paths to files that contain special characters in the name will now be encoded with base64 to avoid malformatted URLs and HMAC mismatches resulting from unescaped characters. URLs generated prior to this change will continue working. Refs Elgg#10608
mrclay
force-pushed
the
serve_filename_22
branch
from
8a9dd15
to
4a7b74e
Compare
mrclay
commented
Dec 25, 2016
| @@ -106,6 +108,12 @@ public function getURL() { | |||
| return false; | |||
| } | |||
|
|
|||
| if (preg_match('~[^a-zA-Z0-9_\./ ]~', $relative_path)) { | |||
There was a problem hiding this comment.
Changed this because \w may match multibyte chars depending on locale
There was a problem hiding this comment.
I am sure we will be dealing with some hickups in the future, so as long as we get most of the craziness covered, I can live with it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relative paths to files that contain special characters in the name will now be encoded with base64 to avoid malformatted URLs and HMAC mismatches resulting from unescaped characters. URLs generated prior to this change will continue working.
Refs #10608