Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't download attachments with russian letters in name (from issue) in Redmine 3.4.0 with Rails 4.2.9, but work corect with Rails 4.2.8 #29669

Closed
VVD opened this issue Jul 4, 2017 · 1 comment

Comments

@VVD
Copy link

VVD commented Jul 4, 2017

Steps to reproduce

  1. Install Redmine 3.4.0.
  2. Update Rails to 4.2.9.
  3. Add issue in installed Redmine.
  4. Attach file with russian letters in name to this issue.
  5. Try do download this file.

Expected behavior

Browser start downloading file.

Actual behavior

Error 404.

System configuration

Rails version: 4.2.9

Ruby version: 2.4.1

But it work fine with Rails 4.2.8 in same environment.

More information is here: https://www.redmine.org/issues/26337#note-9

With Rails 4.2.9 production.log:

Started GET "/attachments/5450/%d0%a2%d0%97.pdf" for 10.0.0.1 at 2017-07-04 08:12:53 +0300
Processing by AttachmentsController#show as HTML
  Parameters: {"id"=>"5450", "filename"=>"\xD0\xA2\xD0\x97.pdf"}
  Current user: VVD (id=3)
  Rendered common/error.html.erb within layouts/base (0.2ms)
Filter chain halted as :find_attachment rendered or redirected
Completed 404 Not Found in 76ms (Views: 28.6ms | ActiveRecord: 42.5ms)

With Rails 4.2.8 production.log:

Started GET "/attachments/5450/%d0%a2%d0%97.pdf" for 10.0.0.1 at 2017-07-04 11:06:56 +0300
Processing by AttachmentsController#show as HTML
  Parameters: {"id"=>"5450", "filename"=>"ТЗ.pdf"}
  Current user: VVD (id=3)
  Rendered common/_other.html.erb (0.8ms)
  Rendered layouts/_file.html.erb (17.1ms)
  Rendered attachments/other.html.erb within layouts/base (26.4ms)
Completed 200 OK in 123ms (Views: 76.3ms | ActiveRecord: 21.1ms)

Look at difference:

"filename"=>"\xD0\xA2\xD0\x97.pdf"
"filename"=>"ТЗ.pdf"
@VVD
Copy link
Author

VVD commented Jul 4, 2017

I made workaround (dirty hack):

--- rails-4.2.9.orig/actionpack/lib/action_dispatch/journey/router/utils.rb
+++ rails-4.2.9/actionpack/lib/action_dispatch/journey/router/utils.rb
@@ -13,13 +13,13 @@
         #   normalize_path("")      # => "/"
         #   normalize_path("/%ab")  # => "/%AB"
         def self.normalize_path(path)
-          encoding = path.encoding
+          encoding = Encoding::UTF_8
           path = "/#{path}"
           path.squeeze!('/')
           path.sub!(%r{/+\Z}, '')
           path.gsub!(/(%[a-f0-9]{2})/) { $1.upcase }
           path = "/" if path == "".freeze
           path.force_encoding(encoding)
           path
         end

@eileencodes eileencodes added this to the 5.1.3 milestone Jul 4, 2017
@eileencodes eileencodes self-assigned this Jul 4, 2017
eileencodes added a commit that referenced this issue Aug 1, 2017
This commit changes the behavior such the path_params now default to
UTF8 just like regular parameters. This also changes the behavior such
that if a path parameter contains invalid UTF8 it returns a 400 bad
request. Previously the behavior was to encode the path params as binary
but that's not the same as query params.

So this commit makes path params behave the same as query params.

It's important to test with a path that's encoded as binary because
that's how paths are encoded from the socket. The test that was altered
was changed to make the behavior for bad encoding the same as query
params. We want to treat path params the same as query params. The params
in the test are invalid UTF8 so they should return a bad request.

Fixes #29669

*Eileen M. Uchitelle, Aaron Patterson, & Tsukuru Tanimichi*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants