This project is archived and is in readonly mode.
.:format feature breaks URL compliance (rfc1738) - should be optional
-
Tyler Jenningslighthouse ate my rfc url and I can't seem to edit the ticket. Here is the link: http://www.ietf.org/rfc/rfc1738.txt
-
Dave Hoover+1
-
Joe Banks+1
I've found this particularly annoying when trying to define routes for a versioned api. /api/1.2/foos.json
-
Ryan Platte
+1 -- Rails shouldn't stumble over routes involving/user.names/with/dots, for example.
-
Jake Scruggs+1
-
Ryan Kinderman+1
Even if the '.' was restricted to the last segment before the query string for format determination, I'd be happy.
What are other peoples' thoughts on the form that the "fix" for this should take?
-
CancelProfileIsBroken
- Tag changed from format, routes, routing to bugmash, format, routes, routing
-
Elise Huard+1
i wouldn't add dots in resources, so this leaves:
* namespaces * named routes * identifiers (ex. name.lastname) where the controller can still be given without the use of dots.the named routes containing dots are no problem
the namespaces are, because there's metaprogramming magic: when the routeset is created, a method hash_for_x_route_path is dynamically defined, and if x contains a dot, this makes for an error. Maybe exchanging the '.' for something else would help, though this would obviously need to be researched carefully.
identifiers, if they are at the end of the url, also are a problem, since they can be mixed up with formats. This is difficult to solve.
-
Elad Meidar+1 was beating around that bush for a long time with api version urls.
-
Josh Nichols+1, on account of having head-desk moments trying to figure out why a route wasn't working, only to have there be a '.' in a param somewhere.
-
Blue Box Jesse+1
Worthwhile fix.
-
sr.iniv.t
+1
I've had a few surprises trying to use version numbers in resource urls in the past.
-
Gaius Centus Novus
+1 Particularly for version numbers and usernames
-
Gaius Centus Novus
It does, however, raise some hard questions. What are you going to do with the following?
/people/I.really.love.xml /gems/shoulda/versions/1.2 /tags/ASP.Net -
Mike Enriquez+1 It would be nice for this to work by default without configuration.
The .:format feature should be configurable by setting a list of formats. I don't see any advantages to having an unlimited and dynamic list of formats. Most people will only use a few formats.
For example the following route:
map.connect "/users/:full_name", :controller => "users", :action => "show"By default, the URL "/users/mike.enriquez" would be recognized and would set params[:full_name] to "mike.enriquez". If the .:format feature has the format "enriquez" configured in the list of formats, then the same URL would set params[:format] to "enriquez".
-
Christopher Hlubek
+1
I had issues with that, too.
Gaius: I think Mikes idea of restricting the matched formats to the configured ones could be a good default. But a malicious user could set his username to "johndoe.xml" which would break some routes. This should be handled by validation of the username.
-
Rizwan Reza- Tag changed from bugmash, format, routes, routing to format, routes, routing
-
Ian+1 because the behavior of :requirements actually makes my scenario impossible. Admittedly, my scenario is a little weird. Basically, I want to allow urls like:
/foos/bar.html # => :id => "bar", :format => "html" /foos/bar.html.html # => :id => "bar.html", :format => "html" /foos/bar.html.xml # => :id => "bar.html", :format => "xml"You can achieve routing of URLs with an
:idrequirements regexp like/([^\?\/](?!(xml|html)(?!\.(xml|html))))+/, but because route generation uses the same regexp, it can't tell that args like:id => "foobar.html", :format => "xml"are perfectly valid.Ideally, you could say something like
map.resources :foos, :parse => {:id => /([^\?\/](?!(xml|html)(?!\.(xml|html))))+/}, :requirements => {:id => /[^\?\/]+/} -
-
Andrew White- State changed from new to open
I've had a good go at fixing this today and I've determined that's it's not possible to do it in such a way that works well for the general case. The main stumbling block is that if the :id parameter can take a period then there's no easy way without hinting to tell the format apart from the id.
However if I can get this commit applied to rack-mount you can work around it easily enough, e.g:
namespace :api do scope(':version', :version => /.+/) do resources :users, :id => /.+?/, :format => /json|xml/ end endIan, you'd be able to do the following to get what you want:
resources :foos, :id => /.+?/, :format => /html|xml/I think this would be a satisfactory solution
-
Andrew White
- Milestone cleared.
- Assigned user set to José Valim
Tests to make sure the above solution can work
Patch inline as uploads aren't workingFrom 01510e1a8edae61955c6112a33fb103f0507395e Mon Sep 17 00:00:00 2001 From: Andrew White <andyw@pixeltrix.co.uk> Date: Fri, 25 Jun 2010 12:16:43 +0100 Subject: [PATCH] Add failing test case for parameters with periods --- actionpack/test/dispatch/routing_test.rb | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-) diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 8f43b5f..915c51a 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -213,6 +213,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get "profile" => "customers#profile", :as => :profile, :on => :member post "preview" => "customers#preview", :as => :preview, :on => :new end + scope(':version', :version => /.+/) do + resources :users, :id => /.+?/, :format => /json|xml/ + end end match 'sprockets.js' => ::TestRoutingMapper::SprocketsApp @@ -1421,6 +1424,30 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_non_greedy_regexp + with_test_routes do + get '/api/1.0/users' + assert_equal 'api/users#index', @response.body + assert_equal '/api/1.0/users', api_users_path(:version => '1.0') + + get '/api/1.0/users.json' + assert_equal 'api/users#index', @response.body + assert_equal true, @request.format.json? + assert_equal '/api/1.0/users.json', api_users_path(:version => '1.0', :format => :json) + + get '/api/1.0/users/first.last' + assert_equal 'api/users#show', @response.body + assert_equal 'first.last', @request.params[:id] + assert_equal '/api/1.0/users/first.last', api_user_path(:version => '1.0', :id => 'first.last') + + get '/api/1.0/users/first.last.xml' + assert_equal 'api/users#show', @response.body + assert_equal 'first.last', @request.params[:id] + assert_equal true, @request.format.xml? + assert_equal '/api/1.0/users/first.last.xml', api_user_path(:version => '1.0', :id => 'first.last', :format => :xml) + end + end + private def with_test_routes yield -- 1.7.1 -
José Valim- State changed from open to committed
-