Redirect routes being selected when no valid route is available.

[iHiD]

Hi guys,

I've been struggling for a few hours with a wrong route being generated when using url_for. I've solved why that's happening, but I believe the the symptom I was getting is a different bug.

Minimal App

To create a minimal app that highlights the problem (you can just copy paste this in)

rails new redirect_bug
cd redirect_bug
rails g scaffold user
rails g scaffold user/media_files
n
rake db:migrate

Change the routes.rb to:

RedirectBug::Application.routes.draw do
  resources :users do
    resources :media_files, controller: "User::MediaFiles"
  end
  get '/google', to: redirect("http://www.google.com")  
end

And add the following to the end of both app/views/users/index.html.erb and app/views/user/media_files/index.html.erb

<%= url_for({:foo => :bar}) %>

• Start the server, and go to /users/.
• Note the url at the bottom is /users?foo=bar
• Go to /users/new.
• Click "Create User"
• Go to /users/1/media_files
• Note the url at the bottom is /google?controller=user%2Fmedia_files&foo=bar

  Where it occurs

As far as I can tell, this occurs on any page that has a URL that cannot itself be reverse engineered. If you change the route from: resources :media_files, class_name: "User::MediaFiles" to: resources :media_files, class_name: "user/media_files" then the error will go away, because url_for will correctly find the route.

The bigger problem

The bigger issue is that match_routes seems to always contain all routes that have redirect as part of them, even if they don't in any way match - this means I never get a routing error.

Specifically this is my output of the hash after it has been 'scored':

{0=>[
    [0, #<Journey::Route:0x007fd6a0c23340 @name=nil, @app=#<Sprockets::Environment:0x3feb4df59204 root="/Users/iHiD/Projects/tmp/redirect_bug", paths=["/Users/iHiD/Projects/tmp/redirect_bug/app/assets/images", "/Users/iHiD/Projects/tmp/redirect_bug/app/assets/javascripts", "/Users/iHiD/Projects/tmp/redirect_bug/app/assets/stylesheets", "/Users/iHiD/Projects/tmp/redirect_bug/vendor/assets/javascripts", "/Users/iHiD/Projects/tmp/redirect_bug/vendor/assets/stylesheets", "/Users/iHiD/.rvm/gems/ruby-1.9.3-p0/gems/jquery-rails-2.1.1/vendor/assets/javascripts", "/Users/iHiD/.rvm/gems/ruby-1.9.3-p0/gems/coffee-rails-3.2.2/lib/assets/javascripts"],
         digest="9f3b95dd7ea3030dc35985c0a8020862">, @path=#<Journey::Path::Pattern:0x007fd6a0c23cf0 @anchored=false, @spec=/assets, @requirements={}, @separators="/.?", @names=[],
         @optional_names=[],
         @required_names=[],
         @re=/\A\/assets/, @offsets=[0]>, @verb=//, @ip=//, @constraints={}, @defaults={}, @required_defaults={}, @required_parts=[],
         @parts=[],
         @decorated_ast=nil, @precedence=0>
    ],
    [15, #<Journey::Route:0x007fd6a0bf2858 @name="google", @app=#<ActionDispatch::Routing::Redirect:0x007fd6a0bf49c8 @status=301, @block=#<Proc:0x007fd6a0bf49f0@/Users/iHiD/.rvm/gems/ruby-1.9.3-p0/gems/actionpack-3.2.6/lib/action_dispatch/routing/redirection.rb:106 (lambda)>>, @path=#<Journey::Path::Pattern:0x007fd6a0bf38e8 @anchored=true, @spec=/google(.:format), @requirements={}, @separators="/.?", @names=["format"],
         @optional_names=["format"],
         @required_names=[],
         @re=nil, @offsets=nil>, @verb=/^GET$/, @ip=//, @constraints={:request_method=>/^GET$/}, @defaults={}, @required_defaults={}, @required_parts=[],
         @parts=[:format],
         @decorated_ast=/google(.:format), @precedence=15>
    ]
]}

I've just lost most of my day tracking down this (and specifically the change in the routes.rb), so I can't look into this any more at the moment, but I hope someone who has more knowledge of the code might be able to see quickly why this happens.

Also, could someone please advise me where best to post the issue with user/media_files vs User::MediaFiles not working in the same way (IMO, they should both work two-ways or one should not work at all). I'm not sure whether that falls under ActiveDispatch or here.

Thanks for all your hard work guys - I hope it's a quick one to solve! Jez (iHiD)

[pixeltrix]

The bigger issue is that match_routes seems to always contain all routes that have redirect as part of them, even if they don't in any way match - this means I never get a routing error.

The never getting a routing error should be fixed by c70e548, which was part of the Journey 1.0.4. It's not just redirect routes - all mounted Rack applications are also included.

Also, could someone please advise me where best to post the issue with user/media_files vs User::MediaFiles not working in the same way (IMO, they should both work two-ways or one should not work at all). I'm not sure whether that falls under ActiveDispatch or here.

Controllers should always be written as 'user/media_files' and not 'User::MediaFiles'. All the examples in the Rails documention are in snakecase so I'm not sure where you'd get the idea that using camelcase is appropriate. However I'm sure that a simple regexp to validate the controller name could be added to raise an error where camelcase is used. If you'd feel up to it please create a PR on the Rails project or create an issue and I'll take a look at it.

[iHiD]

The never getting a routing error should be fixed by c70e548, which was part of the Journey 1.0.4. It's not just redirect routes - all mounted Rack applications are also included.

Brilliant, thanks. Sorry for the duplicate in that case.

All the examples in the Rails documention are in snakecase so I'm not sure where you'd get the idea that using camelcase is appropriate.

Yeah, it's pretty old code. I honestly have no idea :) It's worked fine up till now, until I used url_for() in some pagination, which then highlighted the other problem. I'll put a PR up in Rails when I have chance.

Thanks for responding so promptly - I'll close this now.

[gggritso]

This issue persists in Journey 1.0.4. It accounts correctly for Rack applications, but the redirect routes are still being matched.