Fix 60879 #8344
Fix 60879 #8344
Conversation
larkox
added
2: Dev Review
|
@enahum From what I have tested, filenames usually are not encoded, so they don't need decoding. On the other hand (and this is mostly a guess) links should be encoded, and it makes sense to decode them to get the file name. |
Can you try a pdf file with spaces attached to a post.. I'm sure there is a reason why it was done this way at some point.. I just don't remember why it was |
app/utils/url/index.ts
Outdated
| const result = decodeURIComponent(v); | ||
| return result; |
There was a problem hiding this comment.
small nit:
| const result = decodeURIComponent(v); | |
| return result; | |
| return decodeURIComponent(v); |
| @@ -88,7 +88,7 @@ const MarkdownImage = ({ | |||
| const uri = source.startsWith('/') ? serverUrl + source : source; | |||
|
|
|||
| const fileInfo = useMemo(() => { | |||
| const link = decodeURIComponent(uri); | |||
| const link = safeDecodeURIComponent(uri); | |||
There was a problem hiding this comment.
Should we test this?
An image file with hello%253Fworld.jpg. Would it get decoded into hello%3Fworld.jpg thus causing the file not to be found?
| @@ -273,7 +273,7 @@ const DownloadWithAction = ({action, item, onDownloadSuccess, setAction, gallery | |||
| actionToExecute({ | |||
| code: 200, | |||
| ok: true, | |||
| data: {path}, | |||
| data: {path: path.replace('file://', '')}, | |||
There was a problem hiding this comment.
This may not be related, but I was having issues with some files due to this ending up with a filename file://file://.
|
After my testing, I think everything should be handled correctly, but further testing is always welcomed. @yasserfaraazkhan As testing notes:
|
enahum
dismissed
their stale review
Needs tests at least for the new utility function
| @@ -252,3 +252,11 @@ export function extractFilenameFromUrl(url: string) { | |||
| const uri = urlParse(url); | |||
| return uri.pathname.split('/').pop(); | |||
| } | |||
|
|
|||
| export function safeDecodeURIComponent(v: string) { | |||
Summary
Fix 60879
Ticket Link
Fix https://mattermost.atlassian.net/browse/MM-60879
Fix #8359
Release Note