Issues with upload files with complex names in Safari

Bug
1

There appears to be an issue with uploaded files in Safari when the file names include quotes (particularly bad) or non-ASCII characters. The file names in question are:

Create New "Open File" Macro and Copy Macro URL 1.1.kmmacros
Create New “Open File” Macro and Copy Macro URL 1.1.kmmacros

The first has regular double quotes and behaves very badly, the second has curly quotes and they get mangled.

See this forum topic:

In Chrome/Mac, both files download normally as:

Create New _Open File_ Macro and Copy Macro URL 1.1.kmmacros
Create New “Open File” Macro and Copy Macro URL 1.1.kmmacros

The double quotes in the first file are replaced with underscores, but that is acceptable, presumably a deliberate choice somewhere.

However in Safari, the files downloads as:

Create New .dms
Create New “Open File” Macro and Copy Macro URL 1.1.kmmacros

The first is particularly concerning, since the file name has been completely mangled, including the extension. There are potentially security implications with this as who knows how the quotes are being processed.

The second one appears to be a UTF conversion issue.

I suspect this is a regression, either in Discourse (my server is on 2.3.2) or in Safari, as I think we would have seen it earlier, but I could not find any specific older cases where the macros clearly had quotes in their names to check. There are definitely older files that have non-ASCII characters (eg here), and I have never noticed them to not work before.

It is particularly strange that it works in Chrome and not in Safari, so presumably it is some sort of client-side code issue.

3 Likes
2

I can repro this locally on latest uploading and downloading a file via Firefox. Safari also truncates the download name at the quotes. Chrome must handle quotes in file names differently by replacing them with underscores.

3 Likes
3

@gerhard can you have a look?

2 Likes
5

send_file from Rails currently doesn’t use the filename* parameter in the Content-Disposition header. That’s why those filenames are mangled in most browsers.

Rails 6 will fix it: https://github.com/rails/rails/commit/890485cfce4c361c03a41ec23b0ba187007818cc

Should I apply a monkey patch for now? And it looks like the S3 store should set the filename* as well.

https://github.com/discourse/discourse/blob/fe7f0982af9577314d87001f35b7ea15e044f8b7/lib/file_store/s3_store.rb#L115-L115

2 Likes
6

Probably, not sure when we’ll get to Rails 6?

7

Yes please. Looks like this code is already 1 year old so it should be pretty safe (make sure you look for bugfixes though)

3 Likes
8

I just commited a fix. You can give it a try by upgrading / switching to the tests-passed branch.

3 Likes