grape icon indicating copy to clipboard operation
grape copied to clipboard

Type FIle (Rack::Multipart::UploadedFile) replace all non-English letters in file name with underscores ( _ )

Open alexgonor opened this issue 5 years ago • 8 comments

Hello, I'm notice, that grape File type validation replace all non-English letters with an underscore symbol I send:

------WebKitFormBoundarybo7OclPI12IMowot Content-Disposition: form-data; name="file"; filename="привет-hello.png" Content-Type: image/png

and on endpoint I have next validation:

params do
  requires :file, type: File, allow_blank: false, desc: 'Image'
end

but in params I see:

{"file"=> {"filename"=>"\xD0\xBF\xD1\x80\xD0\xB8\xD0\xB2\xD0\xB5\xD1\x82-hello.png", "type"=>"image/png", "name"=>"file", "tempfile"=>#<File:/tmp/RackMultipart20200710-8-11mcjd3.png>, "head"=>"Content-Disposition: form-data; name=\"file\"; filename=\"\xD0\xBF\xD1\x80\xD0\xB8\xD0\xB2\xD0\xB5\xD1\x82-hello.png\"\r\nContent-Type: image/png\r\n"} }

And this "\xD0\xBF\xD1\x80\xD0\xB8\xD0\xB2\xD0\xB5\xD1\x82-hello.png" it same as "____________-hello.png"

alexgonor avatar Jul 10 '20 14:07 alexgonor

Looks suspicious. Can you please try turning this into a spec, @alexgonor ?

dblock avatar Jul 10 '20 14:07 dblock

@dblock I'm not quite sure what you want to see, can you clarify the request, please?

alexgonor avatar Jul 10 '20 15:07 alexgonor

You're saying that Grape file validation is mangling data. I am pretty sure it does not. Write a spec that demonstrates your problem - AFAIK what you're seeing is some kind of encoding issue and maybe Grape is converting unicode characters into some other kind of encoding and that could be a bug.

dblock avatar Jul 10 '20 15:07 dblock

Ok, this is simple Rails app with grape, you can send file with non-English letters in file name to PUT localhost:3000/ and in params you will be see something like this: Started PUT "/" for ::1 at 2020-07-13 11:14:54 +0300 {"привет-hello.png"=>{"filename"=>"\xD0\xBF\xD1\x80\xD0\xB8\xD0\xB2\xD0\xB5\xD1\x82-hello.png", "type"=>"image/png", "name"=>"привет-hello.png", "tempfile"=>#<Tempfile:/tmp/RackMultipart20200713-32573-14kbwtr.png>, "head"=>"Content-Disposition: form-data; name=\"\"; filename=\"\xD0\xBF\xD1\x80\xD0\xB8\xD0\xB2\xD0\xB5\xD1\x82-hello.png\"; filename*=\"UTF-8''%D0%BF%D1%80%D0%B8%D0%B2%D0%B5%D1%82-hello.png\"\r\nContent-Type: image/png\r\n"}} @dblock

alexgonor avatar Jul 13 '20 08:07 alexgonor

I can take a look when I have a chance. To fix the issue faster:

  1. Check out this project.
  2. Add a spec somewhere in spec/** that reproduces this behavior.
  3. Make a pull request with the failing spec.

dblock avatar Jul 13 '20 12:07 dblock

@alexgonor I'm having the same issue, have you managed to fix it?

khataev avatar Apr 14 '22 20:04 khataev

I figured out what was the reason: it is a combination of Grape + Carriervawe. Curious moment that we can explore on this test. When we use кириллица.png file, then params[:file][:filename] on line 1345 returns кириллица.png

When we use кириллица.xls (real excel file) it returns escaped filename (I suppose using CGI.escape):

"\xD0\xBA\xD0\xB8\xD1\x80\xD0\xB8\xD0\xBB\xD0\xBB\xD0\xB8\xD1\x86\xD0\xB0.xls"

This is the first part. The second part is actually sanitization performed by Carrierwave. It simply replaces all evil characters as (_)

The only question for me, why behavior is different for different files

khataev avatar Apr 17 '22 17:04 khataev

@khataev Do you have a repro of this different behavior in a spec?

dblock avatar Apr 18 '22 01:04 dblock