This project is archived and is in readonly mode.

#2536 ✓committed
Tyler Jennings

.:format feature breaks URL compliance (rfc1738) - should be optional

Reported by Tyler Jennings | April 21st, 2009 @ 03:47 PM | in 3.0.2

Section 3.3 http://www.ietf.org/rfc/rfc1738.txt:

"Within the path and searchpart components, "/", ";", "?" are reserved. The "/" character may be used within HTTP to designate a hierarchical structure."

"." is not a reserved character in the path component of an http schemed url, however Rails treats it as such by default.

I have a requirement to include "." in url path segments that is compliant with the rfc, but doesn't work in Rails.

A mechanism to turn this feature off both globally and on a per-route basis to achieve compliance when needed would be useful.

You can work around this problem by specifying your own :requirements hash for the path segments in a url that will contain ".", but this has other implications. Example:


map.manual_models '/owner_manuals/:brand/:type', :controller => 'owner_manuals', :action => 'list', :requirements => { :brand => /([^\/?]+)/, :type => /([^\/?]+)/ }

Specifying :requirements for a route does two things: First, it checks arguments to url generating functions and throws an exception if the arguments do not match the regex. Second, it changes the route recognition to use the provided regex for a path segment instead of the default. The second behavior is the "fix", but the first causes issues in this context.

The smallest regex you can use with :requirements that will not break normal parsing for other path segments is /[^\/?]/. Now you can use "." freely, but the special characters "/" and "?" are not allowed.

I took a look under the hood and it appears this could mean some significant changes to the routing code. I wanted to get everyone's thoughts before I attempted to work up a solution.

Comments and changes to this ticket

  • Tyler Jennings

    Tyler Jennings April 21st, 2009 @ 03:49 PM

    lighthouse 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
  • Joe Banks

    Joe Banks April 21st, 2009 @ 06:17 PM

    +1

    I've found this particularly annoying when trying to define routes for a versioned api. /api/1.2/foos.json

  • Ryan Platte

    Ryan Platte April 21st, 2009 @ 07:36 PM

    +1 -- Rails shouldn't stumble over routes involving/user.names/with/dots, for example.

  • Jake Scruggs
  • Ryan Kinderman

    Ryan Kinderman April 22nd, 2009 @ 02:55 PM

    +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

    CancelProfileIsBroken August 6th, 2009 @ 02:06 PM

    • Tag changed from format, routes, routing to bugmash, format, routes, routing
  • Elise Huard

    Elise Huard August 9th, 2009 @ 03:13 PM

    +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

    Elad Meidar August 9th, 2009 @ 03:31 PM

    +1 was beating around that bush for a long time with api version urls.

  • Josh Nichols

    Josh Nichols August 10th, 2009 @ 04:08 AM

    +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
  • sr.iniv.t

    sr.iniv.t September 27th, 2009 @ 07:15 PM

    +1

    I've had a few surprises trying to use version numbers in resource urls in the past.

  • Gaius Centus Novus

    Gaius Centus Novus September 28th, 2009 @ 12:58 AM

    +1 Particularly for version numbers and usernames

  • Gaius Centus Novus

    Gaius Centus Novus September 28th, 2009 @ 01:01 AM

    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

    Mike Enriquez September 28th, 2009 @ 08:23 AM

    +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

    Christopher Hlubek October 9th, 2009 @ 12:27 PM

    +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

    Rizwan Reza February 12th, 2010 @ 12:46 PM

    • Tag changed from bugmash, format, routes, routing to format, routes, routing
  • Ian

    Ian April 14th, 2010 @ 06:54 PM

    +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 :id requirements 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 => /[^\?\/]+/}
    
  • Jeremy Kemper

    Jeremy Kemper May 4th, 2010 @ 06:48 PM

    • Milestone changed from 2.x to 3.x
  • Andrew White

    Andrew White June 25th, 2010 @ 12:31 PM

    • 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
      end
    

    Ian, 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

    Andrew White June 25th, 2010 @ 05:31 PM

    • Milestone cleared.
    • Assigned user set to “José Valim”

    Tests to make sure the above solution can work
    Patch inline as uploads aren't working

    From 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

    José Valim June 25th, 2010 @ 11:29 PM

    • State changed from “open” to “committed”
  • Jeremy Kemper

    Jeremy Kemper October 15th, 2010 @ 11:01 PM

    • Milestone set to 3.0.2
    • Importance changed from “” to “”

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>

Referenced by

Pages