- Created a function that replaces all invalid characters in filename with underscores. If the filename is not present, it is randomly generated and extension is deduced from MIME type.
- Made the filename optional in MediaLibrarySelection type.
- Used the above function to make sure that filename is valid when processed in processMedia().
Details
Details
- Reviewers
ashoat atul - Commits
- rCOMMeb9aa10454da: [lib][native] Sanitize media upload filenames
On both platforms various different photos/videos including these containing spaces. Confirmed that all of them work and thumbnail generation doesn't crash.
Also manually tested the regex: https://regex101.com/r/1FCRbJ/2
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
So it appears that we're passing the filename down to processVideo which in turn passes it to getVideoProcessingPlan which is where we try to construct outputPaths for transcoded video + thumbnail and things appear to fail when we try to write to disk at the outputPaths? Wonder if we should push the sanitization down to where we're trying to construct a path so filename is "accurate" as it's being passed along to various functions?
I understand that the filename "doesn't really matter" because we're using uri to actually access the file, but I'm a little concerned about setting it to a value that doesn't reflect the actual name of the file? Like down the line maybe someone assumes that there's a file at ${uri.getParentDirectory}/${filename} but there's actually no file with that name?
Could we pass mime down to getVideoProcessingPlan and then call sanitizeFilename where we're constructing filenames?
Like turn
into something like
Definitely feel free to let me know if there's something I'm missing/misunderstanding here
Comment Actions
Yes, that's my suspicion and it seems to be correct
Wonder if we should push the sanitization down to where we're trying to construct a path so filename is "accurate" as it's being passed along to various functions?
Don't have a strong opinion on that...
I understand that the filename "doesn't really matter" because we're using uri to actually access the file, but I'm a little concerned about setting it to a value that doesn't reflect the actual name of the file? Like down the line maybe someone assumes that there's a file at ${uri.getParentDirectory}/${filename} but there's actually no file with that name?
My main concern and the reason why I made these inputs optional is that on modern mobile OS we shouldn't assume that the filename is always available - due to security reasons, we usually receive a content URI with minimal knowledge about its origin. For instance, modern Android usually gives you a content:// URI with very few permissions, like content://com.android.providers.media.documents/document/image:56. The app can then query a content resolver for more info like filename etc, but it isn't guaranteed to be available. Currently, it usually is (e.g. through expo-media-library), but each android version increases privacy even more.
On iOS, expo-image-picker uses the suggestedName to get a filename, but it's a nullable type.
So in conclusion, we shouldn't rely on origin filenames too much.
Could we pass mime down to getVideoProcessingPlan and then call sanitizeFilename where we're constructing filenames?
Like turn
into something like
Sure, that shouldn't be a problem.
Comment Actions
Ahhh that makes sense, thanks for explaining and pointing out the specific Foundation API where filename is nullable
Sure, that shouldn't be a problem.
Awesome, thanks