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

regex routes are breaking redirection and 'rake routes' #672

Closed
marcboeker opened this issue Sep 16, 2011 · 10 comments
Closed

regex routes are breaking redirection and 'rake routes' #672

marcboeker opened this issue Sep 16, 2011 · 10 comments
Assignees
Milestone

Comments

@marcboeker
Copy link

Hi,

under 0.10.2, Ruby 1.9.3 running

rake routes

results in

rake aborted!
can't convert Regexp into String
/Users/marc/.rvm/gems/ruby-1.9.3-preview1/gems/padrino-core-0.10.2/lib/padrino-core/mounter.rb:90:in `join'
/Users/marc/.rvm/gems/ruby-1.9.3-preview1/gems/padrino-core-0.10.2/lib/padrino-core/mounter.rb:90:in `block in named_routes'
/Users/marc/.rvm/gems/ruby-1.9.3-preview1/gems/padrino-core-0.10.2/lib/padrino-core/mounter.rb:86:in `map'
/Users/marc/.rvm/gems/ruby-1.9.3-preview1/gems/padrino-core-0.10.2/lib/padrino-core/mounter.rb:86:in `named_routes'
/Users/marc/.rvm/gems/ruby-1.9.3-preview1/gems/padrino-core-0.10.2/lib/padrino-core/cli/rake_tasks.rb:21:in `list_app_routes'
/Users/marc/.rvm/gems/ruby-1.9.3-preview1/gems/padrino-core-0.10.2/lib/padrino-core/cli/rake_tasks.rb:38:in `block (2 levels) in <top (required)>'
/Users/marc/.rvm/gems/ruby-1.9.3-preview1/gems/padrino-core-0.10.2/lib/padrino-core/cli/rake_tasks.rb:37:in `each'
/Users/marc/.rvm/gems/ruby-1.9.3-preview1/gems/padrino-core-0.10.2/lib/padrino-core/cli/rake_tasks.rb:37:in `block in <top (required)>'

I have a route that uses a regex:

get :index, map: /^\/(?!robots\.txt|sitemap\.xml).*/im do
  render_page
end

To fix the problem, I had to change

#L109 of mounter.rb

to

full_path = File.join(uri_root.to_s, route.original_path.to_s)

Another problem is redirection with regex routes. Let's take this controller:

Site.controllers :base do
  get :index, map: /^\/(?!robots\.txt|sitemap\.xml).*/im do
    render_page
  end
end

Running rake routes gives me:

Application: Site
    URL                                 REQUEST  PATH
    (:index)                              GET    /(?mi-x:^\/(?!robots\.txt|sitemap\.xml).*)

So redirection with:

url(:base, :index)

fails, as the name base of the controller is omitted.

What am I doing wrong?

Thanks Marc

@marcboeker
Copy link
Author

Hm just thought, redirecting to routes that are defined as regex makes no sense :)
Maybe we can add a fallback to /if someone (likes me) wants to redirect to a regex route?

Thanks Marc

@ghost ghost assigned joshbuddy Sep 16, 2011
@DAddYE
Copy link
Member

DAddYE commented Sep 16, 2011

@joshbuddy, what about?

@nesquena
Copy link
Member

I am not sure I can recommend in good conscience using the named route syntax with a regular expression. What would you need to refer to the regex using a symbol (considering you cant redirect to it). I feel like it is better off with:

get /^\/(?!robots\.txt|sitemap\.xml).*/im do
  render_page
end

What do you guys think?

@marcboeker
Copy link
Author

@nesquena Sounds good to throw the route name on regex routes away, as it is stupid to redirect to these. If this should be the common behaviour, we do not need to fix rake routes.

Thanks so far.

@DAddYE
Copy link
Member

DAddYE commented Sep 16, 2011

Mmm, if Im not wrong (we need to make a spec) shouldn't work, because you are inside a controller, and it prepend "/base/" to their route. Maybe should work if defined directly inside app. This can be the best way to prevent problems. I think @joshbuddy has the right answer.

@nesquena
Copy link
Member

Moving to 0.10.4 because seems like a minor issue (and I want to try and prepare for 0.10.3)

@joshbuddy
Copy link
Contributor

I'll take a look at this.

@joshbuddy
Copy link
Contributor

Regexp route generation in 98ce668

@joshbuddy
Copy link
Contributor

Rake routes fixed in aaad729

@nesquena
Copy link
Member

Awesome, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants