This project is archived and is in readonly mode.
Trailing slashes in URLs causing routing errors in 2.3.0 RC1 for resources declared using ":as"
Reported by Wincent Colaiuta | February 22nd, 2009 @ 10:56 AM
Originally posted as a question here:
http://groups.google.com/group/r...
Rails now raises routing errors when it sees a trailing slash on the end of URLs for resources defined using the ":as" option.
eg. given a resource like this:
map.resources :articles, :as => :wiki
Accessing "/wiki" works, but trying "/wiki/" does not. (Both used to work in 2.2.2.)
With a resource that doesn't use an ":as" setting:
map.resources :posts
Both "/posts" and "/posts/" work as they did before.
I tested this by adding an example like this to my spec suite:
it "should generate params { :controller => 'articles', action => 'index', :protocol => 'https' } from GET /wiki" do
# no trailing slash on "/wiki"
params_from(:get, '/wiki').should == {:controller => "articles", :action => "index", :protocol => 'https'}
end
# Rails 2.3.0 RC1 regression
it "should generate params { :controller => 'articles', action => 'index', :protocol => 'https' } from GET /wiki/" do
# note the trailing slash on "/wiki/"
params_from(:get, '/wiki/').should == {:controller => "articles", :action => "index", :protocol => 'https'}
end
Both examples pass in 2.2.2, but in 2.3.0 RC1 only the first example passes and the second raises a routing error.
I'll try to add a test case to "actionpack/test/controller/routing_test.rb" to catch this regression, but I'm not familiar with Test::Unit and I don't really know where to start in the 2,500+ line file. It may take me a while to figure this out...
There are a couple possibly related tickets already in the tracker, ticket #1753 and ticket #1967, but as this issue I'm describing here specifically applies to resources declared using ":as" I'm creating a new ticket.
Comments and changes to this ticket
-
Randy Schmidt February 23rd, 2009 @ 04:45 PM
I'm seeing a similar problem with nested resources at the top level with:
map.resources :courses do |course| course.resources :offerings end /courses/something # works /courses/something/ # doesn't work
We need this because the browser adds a trailing slash when doing something like "../.." in an anchor tag.
-
DHH February 27th, 2009 @ 01:57 PM
- Assigned user set to josh
- Milestone cleared.
-
Wincent Colaiuta February 27th, 2009 @ 04:22 PM
- Tag set to patch
Attaching a test case that demonstrates the problem. It shows the regression in the presence of an ":as" resource. I didn't include a test for the case that Randy mentions in his comment.
-
josh March 3rd, 2009 @ 08:05 PM
Hmm, this test case shows that the original unaliased doesn't work with the trailing slash either. Is this broken for all resource helpers?
-
Wincent Colaiuta March 3rd, 2009 @ 08:30 PM
I applied your test patch and confirmed that it fails in current HEAD of the master branch, but passes in v2.2.2 (just like my test patch).
I am going to see if I can "git bisect" this to find out which commit introduced the breakage.
-
Wincent Colaiuta March 3rd, 2009 @ 09:08 PM
- State changed from new to committed
According to "git bisect" the commit which introduced this regression is fef6c32afe2276dffa0347e25808a86e7a101af1.
fef6c32afe2276dffa0347e25808a86e7a101af1 is first bad commit commit fef6c32afe2276dffa0347e25808a86e7a101af1 Author: Aaron Batalion <aaron@hungrymachine.com> Date: Mon Nov 24 02:24:19 2008 -0500 Added optimal formatted routes to rails, deprecating the formatted_* methods, and reducing routes creation by 50% [#1359 state:committed] Signed-off-by: David Heinemeier Hansson <david@loudthinking.com> :040000 040000 d2ebe89abee3e8ef344578a5d878d8e3d2b5a3aa a3ae402756574735e8889671514de26a48904490 M actionpack
-
Wincent Colaiuta March 3rd, 2009 @ 09:09 PM
Sigh.
That state-change from "new" to "committed" was triggered by me pasting in the output from "git bisect".
Can somebody with the appropriate administrative rights change the state back?
-
CancelProfileIsBroken March 3rd, 2009 @ 10:05 PM
- State changed from committed to new
-
CancelProfileIsBroken March 3rd, 2009 @ 11:20 PM
The key appears to be whether the URL with the trailing slash would be properly routed by the default routes:
map.resources :messages, :as => 'reviews' map.resources :messages map.connect ':controller/:action/:id' map.connect ':controller/:action/:id.:format'
In this case, /messages and /messages/ and /reviews work, and /reviews/ fails
map.resources :messages, :as => 'reviews' map.resources :messages
(with the default catch-all routes deleted)
In this case, /messages and /reviews work, /messages/ and /reviews/ both fail.
-
CancelProfileIsBroken March 3rd, 2009 @ 11:46 PM
The problem is that before fef6c32a the route recognizer would get to the end of something like "/reviews/", have only the trailing slash left, and ignore it, so the index route would match.
Now, because there's that extra optional format segment there, it tries to match the trailing slash against the .format regex and fails.
Attached patch incorporates Josh's tests, and modifies the optional format regex chunk to '/|(.[^/?.]+)?' so that it will accept a trailing slash (but not pass it through to the controller as a format). With this in place, all ActionPack tests pass, and we're back to the old behavior of trailing slashes on all URLs (not just index URLs) being ignored.
-
Repository March 4th, 2009 @ 01:39 AM
- State changed from new to committed
(from [ce56c5daa81d61a745b88220014a846a0eea46a4]) Allow routes with a trailing slash to be recognized
Signed-off-by: Michael Koziarski michael@koziarski.com [#2039 state:committed] http://github.com/rails/rails/co...
-
Wincent Colaiuta March 4th, 2009 @ 10:07 AM
Thanks for investigating and coming up with the fix, Mike.
-
Elia Schito July 16th, 2009 @ 05:03 PM
I've an analoge problem, defining:
map.user_operations_logs 'logs/user_operations/:date/:page',
:controller => 'logs', :action => 'user_operations', :date => nil, :page => nil
This generates /logs/user_operations// (with double final slashes) which is being routed to the 404 (or catch all)
When called with user_operations_logs_path, while both user_operations_logs_path(nil) and user_operations_logs_path(nil, nil) work.
Perhaps I have to open a new bug... let me know.
Thanks.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Referenced by
- 2039 Trailing slashes in URLs causing routing errors in 2.3.0 RC1 for resources declared using ":as" Signed-off-by: Michael Koziarski michael@koziarski.com [#...
- 2094 Resources Routing not correctly generating paths Duplicate of #2039. Was fixed in ce56c5daa81d61a745b8822...
- 1753 URLs with trailing slashes behave differently in Edge than in 2.2.2 Patched on #2039