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"

#2039

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.

Reported by Wincent Colaiuta · February 22nd, 2009 @ 10:56 AM

Activity

Randy Schmidt
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.
February 23rd, 2009 @ 04:45 PM
DHH
- Assigned user set to josh
- Milestone cleared.
February 27th, 2009 @ 01:57 PM
Wincent Colaiuta
- 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.54 KB
February 27th, 2009 @ 04:22 PM
josh
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
March 3rd, 2009 @ 08:05 PM
Wincent Colaiuta
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.

March 3rd, 2009 @ 08:30 PM

Wincent Colaiuta

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

March 3rd, 2009 @ 09:08 PM

Wincent Colaiuta
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?

March 3rd, 2009 @ 09:09 PM
CancelProfileIsBroken
- State changed from committed to new
March 3rd, 2009 @ 10:05 PM
CancelProfileIsBroken
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.

March 3rd, 2009 @ 11:20 PM
CancelProfileIsBroken
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.91 KB
March 3rd, 2009 @ 11:46 PM
Repository
- 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...

March 4th, 2009 @ 01:39 AM
Wincent Colaiuta
Thanks for investigating and coming up with the fix, Mike.

March 4th, 2009 @ 10:07 AM
Elia Schito
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.
July 16th, 2009 @ 05:03 PM
Ryan Bigg
- Tag cleared.
Automatic cleanup of spam.

October 9th, 2010 @ 09:47 PM
Jeff Kreeftmeijer
Automatic cleanup of spam.

November 7th, 2010 @ 04:52 PM