#2039 Trailing slashes in URLs causing routing errors in 2.3.0 RC1 for resources declared using ":as" - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#2039 ✓committed

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

You flagged this item as spam.
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.
You flagged this item as spam.
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.
- 0001-Testcase-for-trailing-slash-bug-for-as-resources.patch 1.5 KB
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?
- failing_resouce_test.diff 923 Bytes
You flagged this item as spam.
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

You flagged this item as spam.
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.
- routing_with_slash.diff 1.9 KB
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...
You flagged this item as spam.
Wincent Colaiuta March 4th, 2009 @ 10:07 AM
Thanks for investigating and coming up with the fix, Mike.
You flagged this item as spam.
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.
Ryan Bigg October 9th, 2010 @ 09:47 PM
- Tag cleared.
Automatic cleanup of spam.
Jeff Kreeftmeijer November 7th, 2010 @ 04:52 PM
Automatic cleanup of spam.