Rails Ruby on Rails → 3.0.2

Comments and changes to this ticket

• You flagged this item as spam.
  

  Bruno Michel June 26th, 2010 @ 04:38 PM

  Here is the patch and its tests.

  By the way, I think that the behaviour of url_for for escaping is weird. It escapes strings by default, but not hashes, even if the documentation wasn't saying that. It seems that there is a purpose for that as there was a commit specially for that (1b3195b63ca44f0a70b61b75fcf4991cb2fbb944), so I do not have modified this. But I was tempted.

  • link_to_with_ampersand_in_query_string.diff 5 KB

• You flagged this item as spam.
  

  Bruno Michel June 26th, 2010 @ 04:51 PM

  • Tag changed from “escape, link_to, rails3” to “escape, link_to, patch, rails3”

• You flagged this item as spam.
  

  chaitanyav June 27th, 2010 @ 05:01 AM

  I checked this and it does escape the ampersand
  

  =link_to "Clickme", url_for(:controller => "posts", :action => "index")
  =link_to "Clickme", url_for(:controller => "posts", :action => "index", :foo => "1", :bar => "2")
  =link_to "Clickme", url_for(:controller => "posts", :action => "new", :foo => "1", :bar => "2")
  
  
  
  
  
  
  
  <a href="/posts">Clickme</a>
  <a href="/posts?bar=2&amp;foo=1">Clickme</a>
  <a href="/posts/new?bar=2&amp;foo=1">Clickme</a>
  

  I am using Rails 3 beta 4.

• You flagged this item as spam.
  

  chaitanyav June 27th, 2010 @ 05:27 AM

  It generated this url,

  =link_to_unless_current("Showing", {:order => "desc", :page => 2})
  <a href="/posts?order=desc&page=2">Showing</a>
  

  BTW
  

  def hash_for(opts = {})
  
  
  
  
  
  
  

  {:controller => "foo", :action => "bar"}.merge(opts)
  

  
  
  
  
  end
  
  
  
  hash_for does not escape, its a method in the url_helper_test.rb, url_for does the escaping
  

• You flagged this item as spam.
  

  chaitanyav June 27th, 2010 @ 05:36 AM

  def hash_for(opts = {})
    {:controller => "foo", :action => "bar"}.merge(opts)
  end
  

• 

  Andrew White June 27th, 2010 @ 09:36 AM

  • Assigned user set to “José Valim”
  • State changed from “new” to “open”
  • Milestone cleared.
  • Importance changed from “” to “High”

  I think that commit was to prevent double escaping of hash urls but that seems to have been corrected elsewhere. I think we need to restore the escaping by default of hash urls since that's what the HTML specs recommend.

  Attached patch includes a test to make sure hash urls are not double escaped.

  • 0001-restore-escaping-of-hash-urls.diff 3.7 KB
• 

  Repository June 28th, 2010 @ 02:37 PM

  • State changed from “open” to “resolved”

  (from [0b6ce3422370647cad3e91263a291f69b313d65b]) Restores the escaping of urls generated from hashes. [#4765 state:resolved]

  HTML specifications recommend the escaping of urls in web pages,
  which url_for does by default for string urls and consquently
  urls generated by path helpers as these return strings.

  Hashes passed to url_for are not escaped by default and this
  commit reverses this default so that they are escaped.

  Undoes the changes of this commit:
  http://github.com/rails/rails/commit/1b3195b63ca44f0a70b61b75fcf499...

  Signed-off-by: José Valim jose.valim@gmail.com
  http://github.com/rails/rails/commit/0b6ce3422370647cad3e91263a291f...

• 

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

  • Milestone set to “3.0.2”

  [bulk edit]