Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #4918 (new defect)

Opened 3 years ago

Last modified 7 months ago

[PATCH] Bug in ActionView::Helpers::UrlHelper::current_page?

Reported by: ssinghi@kreeti.com Assigned to: David
Priority: high Milestone:
Component: ActionPack Version: 2.0.1
Severity: major Keywords:
Cc: ssinghi@kreeti.com, pstradomski@gmail.com

Description

Hello,

there is a bug in ActionView::Helpers::UrlHelper::current_page(options), this bug causes other methods like (link_to_current_unless) to also behave buggily.

the method definition is:

def current_page?(options)

CGI.escapeHTML(url_for(options)) == @controller.request.request_uri

end

in rails 1.0.0 the url_for method didn't use to escape the HTML, but in version 1.1.2 it does (IMO this should be documented). This causes double escaping. Also, the request_url method <b>doesn't</b> HTML escapes the url. So, the string match fails.

The current_page? method should be <b>rewritten</b> as:

url_for(options) == CGI.escapeHTML(@controller.request.request_uri)

the above should fix things.

This is my first bug report for rails, so pardon my sloppiness.

Thanks.

Attachments

url_helper_patch.txt (0.6 kB) - added by ssinghi@kreeti.com on 04/28/06 12:16:45.
Patch file
url_helper_patch.diff (0.6 kB) - added by ssinghi@kreeti.com on 05/05/06 18:30:42.
Patch file
fix_current_page_patch.diff (1.9 kB) - added by spideryoung on 12/07/07 10:41:26.

Change History

04/28/06 12:16:45 changed by ssinghi@kreeti.com

  • attachment url_helper_patch.txt added.

Patch file

05/05/06 18:24:38 changed by anonymous

  • cc set to ssinghi@kreeti.com.

05/05/06 18:30:42 changed by ssinghi@kreeti.com

  • attachment url_helper_patch.diff added.

Patch file

08/22/07 15:06:34 changed by schweikert

I just wanted to report that this bug is still present in version 1.2.3. The consequence is that link_to_unless_current doesn't work if any query string is used (when more than one parameter is in use).

12/07/07 10:41:26 changed by spideryoung

  • attachment fix_current_page_patch.diff added.

12/07/07 10:42:16 changed by spideryoung

New patch added, tested against r8328

04/23/08 15:04:45 changed by pstradomski

  • cc changed from ssinghi@kreeti.com to ssinghi@kreeti.com, pstradomski@gmail.com.

Bug again present in rails 2.0.2 This time the target url is escaped twice, while request url to which it is compared is not escaped at all.

The method should look that way to be correct: 

def current_page?(options)
    url_string = url_for(options)
    request = @controller.request
    if url_string =~ /^\w+:\/\//
        url_string == CGI.escapeHTML("#{request.protocol}#{request.host_with_port}#{request.request_uri}")
    else
        url_string == CGI.escapeHTML(request.request_uri)
    end
end

04/23/08 15:05:37 changed by pstradomski

  • version changed from 1.1.1 to 2.0.1.