Rails Ruby on Rails → 3.x

Comments and changes to this ticket

• 

  CancelProfileIsBroken March 10th, 2009 @ 12:55 PM

  • Assigned user set to “josh”
  • State changed from “new” to “verified”
  • Title changed from “"optimal formatted routes" breaks format requirements” to “Bad routing recognition with globbing/format/requirements”

  There are a couple of general issues tangled up together here. You can see both by looking at the route recognizer we build in this case:

  
  def recognize(path, env = {})
  if (match = /\A\/files(?:\/?\Z|\/(.*)(?:\/|(\.[^\/?\.]+)?)??)\Z/.match(path))
  params = parameter_shell.dup
  params[:path] = PathSegment::Result.new_escaped((match[1] || "").split('/'))
  
            if (m = match[2])
              params[:format] = URI.unescape(m.from(1))
            end
  
  params
  end
  end
  

  Two problems:

  1) Because we use (.*) to match the globbed path, there will never be a match[2] here to match against the format

  2) Even if we fixed that, this would still fail because the regex segment we generate for the format includes [^\/?.], causing it to only recognize formats without a dot in them - so .tar.gz will never been seen as a format in any route.

  Both of these issues seem fairly intractable for the 2.3 cycle. It looks like changing the format recognition regexp segment to include [^\/?] would fix the second issue, but that causes test_irregular_id_with_no_requirements_should_raise_error(ResourcesTest) to fail - personally I'm not too happy about that test, but it documents existing behavior that we probably don't want to change.

  For what it's worth, here's a patch with a bunch of tests to document the failures. In the current edge code, test_path_without_format_should_include_format_in_path passes and all the rest fail. I think in an ideal world all should pass.

  • failing_routing_tests.diff 3.5 KB
• 

  josh March 11th, 2009 @ 05:32 PM

  • Assigned user cleared.
• 

  Pratik March 12th, 2009 @ 02:49 PM

  • State changed from “verified” to “open”

• You flagged this item as spam.
  

  Robert Glaser July 15th, 2009 @ 05:16 PM

  For me, globbed routes with an attached format parameter generally don't work:

  map.article ':locale/*path.:format', :controller => "articles", :action => "show"
  

  The format parameter is always being concatenated to the last element of the path array.

• 

  Michael Koziarski August 3rd, 2009 @ 06:14 AM

  • Tag set to “bugmash”

• You flagged this item as spam.
  

  Kieran P August 9th, 2009 @ 03:05 AM

  +1 The patch applies cleanly to Rails 2.3 (2-3-stable) and Rails 3.0 (master).

• You flagged this item as spam.
  

  Elise Huard August 15th, 2009 @ 05:50 PM

  Mike's patch applies to master, and indeed, only test_path_without_format_should_include_format_in_path passes from the 5 proposed tests.

  IMHO there's a link with #2478

• You flagged this item as spam.
  

  Peter Abrahamsen December 18th, 2009 @ 07:52 PM

  Bump for great justice; this is really annoying.

• 

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

  • Tag cleared.

  [bulk edit]

• 

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

  • Milestone changed from “2.x” to “3.x”

  [bulk edit]

• 

  Andrew White June 25th, 2010 @ 02:44 PM

  • State changed from “open” to “resolved”
  • Assigned user set to “Andrew White”

  This seems to be working now:

    match '/:locale/*path.:format', :to => 'articles#show'
  
    # GET /en/this/is/a/long/path.html
    # => {"format"=>"html", "action"=>"show", "path"=>"this/is/a/long/path", "controller"=>"articles", "locale"=>"en"}
  

  Please update this ticket if it isn't working for you.

• You flagged this item as spam.
  

  Dawid Fatyga November 11th, 2010 @ 12:56 PM

  • Importance changed from “” to “”

  Hello,

  what about cases where :format is omitted? The route to /en/this/is/a/long/path will not match articles#show action with :format => 'html'.

  Adding:
  

  :defaults => { :format => 'html' }
  

  does not solve the problem. Also changing the match to:

  match '/:locale/*path(.:format)', :to => 'articles#show'
  

  The routes that work as expected (allows the optional format) should look like this:

  match '/:locale/*path.:format', :to => 'articles#show'
  match '/:locale/*path',         :to => 'articles#show', :as => "article"
  

  I am submitting a diff with failing tests to clarify my point.

  Regards,
  Dawid

  • globbing_routes_failing_test.diff 2 KB
• 

  Andrew White November 11th, 2010 @ 01:58 PM

  The problem with trying to make the format parameter optional is that glob parameter will eat up the format even if you set the parameter regexp to be non-greedy. You need a fixed item in the path to stop the glob matching - the following works:

    match '/:locale/*category/:article(.:format)' => 'articles#show', :as => :article
    # Matches the following:
    # - /en/computers/apple
    # - /en/computers/apple.html
    # - /en/computers/apple.xml
    # - /en/computers/apple/macbook
    # - /en/computers/apple/macbook.html
    # - /en/computers/apple/macbook.xml
  

  You can make the category optional as well

    match '/:locale/(*category)/:article(.:format)' => 'articles#show', :as => :article
  

  You don't need the format parameter - Rails will add that by default unless you pass :format => false.