i18n routes can't be recognized #3470

Closed
kuraga opened this Issue Oct 30, 2011 · 8 comments

Comments

Projects
None yet
6 participants
@kuraga

kuraga commented Oct 30, 2011

It seems the situation described here: https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/4896 is still aactual.

So, Rails correctly creates and stores utf-8 based routes. But when it comes to route recognition, it fails to match the route name:

get 'начало', controller: :home, action: :index

then http://localhost/%D0%BD%D0%B0%D1%87%D0%B0%D0%BB%D0%BE gives
No route matches "/%D0%BD%D0%B0%D1%87%D0%B0%D0%BB%D0%BE" with {:method=>:get}

The reason is that recognize_path does html unescape only after routes are mapped, and because of this it fails to map raw utf8 stored name against its %XX%XX equivalent.

Is this a bug? Can somebody help with this?

@bublik

This comment has been minimized.

Show comment
Hide comment
@bublik

bublik Nov 14, 2011

I think it should be CGI.unescape before roure recognize
+1

bublik commented Nov 14, 2011

I think it should be CGI.unescape before roure recognize
+1

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Apr 29, 2012

Member

Confirmed that this is still an issue on 3.2.2.

Member

steveklabnik commented Apr 29, 2012

Confirmed that this is still an issue on 3.2.2.

@Najaf

This comment has been minimized.

Show comment
Hide comment
@Najaf

Najaf Jun 4, 2012

Still an issue in 3.2.5

Had a look into this today, and it seems that even if you had the paths unescaped this wouldn't work in rails at the moment.

Started by writing this failing test in actionpack/test/controller/routing_test.rb:

get 'начало', :controller => 'home', :action => 'index'
# and then later...
assert_equal(
  {:controller => 'home', :action => 'index'}, 
  @routes.recognize_path('/начало', :method => :get)
)

First error you get is:

ActionController::RoutingError: bad URI(is not URI?): /начало

Which is actually a URI::InvalidURIError thrown by Rack::MockRequest.env_for calling URI('/начало').

Played around in IRB to sanity check:

$ irb -r 'uri'
> URI('начало')
URI::InvalidURIError: bad URI(is not URI?): /начало

As expected URI('ルービが好きです。') yielded the same error.

Najaf commented Jun 4, 2012

Still an issue in 3.2.5

Had a look into this today, and it seems that even if you had the paths unescaped this wouldn't work in rails at the moment.

Started by writing this failing test in actionpack/test/controller/routing_test.rb:

get 'начало', :controller => 'home', :action => 'index'
# and then later...
assert_equal(
  {:controller => 'home', :action => 'index'}, 
  @routes.recognize_path('/начало', :method => :get)
)

First error you get is:

ActionController::RoutingError: bad URI(is not URI?): /начало

Which is actually a URI::InvalidURIError thrown by Rack::MockRequest.env_for calling URI('/начало').

Played around in IRB to sanity check:

$ irb -r 'uri'
> URI('начало')
URI::InvalidURIError: bad URI(is not URI?): /начало

As expected URI('ルービが好きです。') yielded the same error.

@kennyj

This comment has been minimized.

Show comment
Hide comment
@kennyj

kennyj Jun 5, 2012

Contributor

If we draw the following route,

get Rack::Utils.escape('こんにちは'), :controller => 'news', :action => 'index'

the following test is passed (on master).

def test_unicode_path
  assert_equal({:controller => 'news', :action => 'index'},
    @routes.recognize_path("/#{Rack::Utils.escape('こんにちは')}", :method => :get))
end

But I want to draw a route as

get 'こんにちは', :controller => 'news', :action => 'index'

WDYT ? :-)

Contributor

kennyj commented Jun 5, 2012

If we draw the following route,

get Rack::Utils.escape('こんにちは'), :controller => 'news', :action => 'index'

the following test is passed (on master).

def test_unicode_path
  assert_equal({:controller => 'news', :action => 'index'},
    @routes.recognize_path("/#{Rack::Utils.escape('こんにちは')}", :method => :get))
end

But I want to draw a route as

get 'こんにちは', :controller => 'news', :action => 'index'

WDYT ? :-)

@kennyj

This comment has been minimized.

Show comment
Hide comment
@kennyj

kennyj Jun 5, 2012

Contributor

How about kennyj/rails@09fad24 ?
It's work fine, and all tests are green :-)

Contributor

kennyj commented Jun 5, 2012

How about kennyj/rails@09fad24 ?
It's work fine, and all tests are green :-)

@kennyj

This comment has been minimized.

Show comment
Hide comment
@kennyj

kennyj Jun 6, 2012

Contributor

Anyway, I'll submit PR. Thanks !

Contributor

kennyj commented Jun 6, 2012

Anyway, I'll submit PR. Thanks !

@Najaf

This comment has been minimized.

Show comment
Hide comment
@Najaf

Najaf Jun 6, 2012

Looks all good to me, let's see what core think.

Najaf commented Jun 6, 2012

Looks all good to me, let's see what core think.

@kuraga

This comment has been minimized.

Show comment
Hide comment
@kuraga

kuraga Jun 6, 2012

Thank you! I can't write tests yet and haven't write this simple patch for seven month :) Will learn tests after exams ;)

kuraga commented Jun 6, 2012

Thank you! I can't write tests yet and haven't write this simple patch for seven month :) Will learn tests after exams ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment