This project is archived and is in readonly mode.

#4870 ✓resolved
Łukasz

A bug in ActionView::Helpers::SanitizeHelper.strip_tags

Reported by Łukasz | June 16th, 2010 @ 11:34 AM | in 3.0.2

There is a bug in ActionView::Helpers::SanitizeHelper.strip_tags.

For example:

When I tried remove all tags from string :

"

using ActionView::Helpers::SanitizeHelper.strip_tags, I got TypeError: 'can't convert nil into String'.

Comments and changes to this ticket

  • Łukasz

    Łukasz June 16th, 2010 @ 11:37 AM

    The string:

    "\<p>\<span class=\\``\\\"\\\\\"\\\\\\\\\"\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"grejborder2\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\"\\\\\\\\\"\\\\\"\\\"\\``> \<p>\<span class=\\``\\\"\\\\\"\\\\\\\\\"\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"grejborder2\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\"\\\\\\\\\"\\\\\"\\\"\\``> \<p>\<strong>Charakterystyka produktu:\</strong>\<span class=\\``\\\"\\\\\"\\\\\\\\\"\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"
    

    Edited by Rohit Arondekar to fix formating.

  • Sebastian Gräßl

    Sebastian Gräßl June 20th, 2010 @ 09:54 AM

    Hej Łukasz,

    can you provide more information about your environment and rails version.
    Also it would be helpful if you provide a code example in which the problem occurs.

    Note: Please always wrap code in a ticket in a code-block. see
    You can use the "Preview"-button on the right hand side to see if the code is displayed right in your ticket.

    Best,
    Sebastian

  • Rohit Arondekar

    Rohit Arondekar June 20th, 2010 @ 11:09 AM

    • Tag set to actionpack, html
    • State changed from “new” to “open”

    Sebastian, Łukasz: I have fixed the formating of the string.

    Confirmed on Rails master.

    ruby-1.9.2-head > helper.strip_tags("\<p>\<span class=\\``\\\"\\\\\"\\\\\\\\\"\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"grejborder2\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\"\\\\\\\\\"\\\\\"\\\"\\``> \<p>\<span class=\\``\\\"\\\\\"\\\\\\\\\"\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"grejborder2\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\"\\\\\\\\\"\\\\\"\\\"\\``> \<p>\<strong>Charakterystyka produktu:\</strong>\<span class=\\``\\\"\\\\\"\\\\\\\\\"\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\"\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\")
    TypeError: can't convert nil into String
        from /home/rohit/remote-repos/rails_patches/working4/actionpack/lib/action_controller/vendor/html-scanner/html/tokenizer.rb:99:in `block in consume_quoted_regions'
        from /home/rohit/remote-repos/rails_patches/working4/actionpack/lib/action_controller/vendor/html-scanner/html/tokenizer.rb:83:in `loop'
        from /home/rohit/remote-repos/rails_patches/working4/actionpack/lib/action_controller/vendor/html-scanner/html/tokenizer.rb:83:in `consume_quoted_regions'
        from /home/rohit/remote-repos/rails_patches/working4/actionpack/lib/action_controller/vendor/html-scanner/html/tokenizer.rb:63:in `scan_tag'
        from /home/rohit/remote-repos/rails_patches/working4/actionpack/lib/action_controller/vendor/html-scanner/html/tokenizer.rb:40:in `next'
        from /home/rohit/remote-repos/rails_patches/working4/actionpack/lib/action_controller/vendor/html-scanner/html/sanitizer.rb:19:in `tokenize'
        from /home/rohit/remote-repos/rails_patches/working4/actionpack/lib/action_controller/vendor/html-scanner/html/sanitizer.rb:8:in `sanitize'
        from /home/rohit/remote-repos/rails_patches/working4/actionpack/lib/action_controller/vendor/html-scanner/html/sanitizer.rb:33:in `sanitize'
        from /home/rohit/remote-repos/rails_patches/working4/actionpack/lib/action_view/helpers/sanitize_helper.rb:76:in `strip_tags'
        from (irb):11
        from /home/rohit/remote-repos/rails_patches/working4/railties/lib/rails/commands/console.rb:47:in `start'
        from /home/rohit/remote-repos/rails_patches/working4/railties/lib/rails/commands/console.rb:8:in `start'
        from /home/rohit/remote-repos/rails_patches/working4/railties/lib/rails/commands.rb:23:in `<top (required)>'
        from script/rails:6:in `require'
        from script/rails:6:in `<main>'
    

    This looks like a html parsing problem.

  • Łukasz

    Łukasz June 21st, 2010 @ 08:43 AM

    Sorry for invalid code sample, Im newbie here, next tickets will be better :).
    Sebastian, Rohit thanks for notes and replies.
    Fixing this bug would help me finish sth important in my project.

    Best,

    Lukasz

  • Bruno Michel

    Bruno Michel June 26th, 2010 @ 06:02 PM

    • Tag changed from actionpack, html to actionpack, html, patch
    • Importance changed from “” to “Low”

    I've made a patch to fix this bug.

  • Bruno Michel

    Bruno Michel June 28th, 2010 @ 09:41 AM

    • Assigned user set to “José Valim”

    José, do you have the time to look at this patch before the release candidate?

  • José Valim

    José Valim June 28th, 2010 @ 09:58 AM

    Changing vendored gems is not a good practice. So there are a few questions:

    1) Is the html-scanner available somewhere as a gem?

    2) If so, can't we fix it in the gem? Shouldn't we add test cases straight to the html-scanner code?

  • Bruno Michel

    Bruno Michel June 28th, 2010 @ 10:29 AM

    I don't think that html-scanner is available as a gem. In fact, I didn't find html-scanner, except in Rails. Maybe, it's time to move it out of vendor.

  • José Valim

    José Valim June 28th, 2010 @ 10:39 AM

    • Milestone cleared.

    According to JK: "html-scanner was added as a utility lib just for rails". That said, I'm going to apply the patch soon!

  • Repository

    Repository June 28th, 2010 @ 10:40 AM

    • State changed from “open” to “resolved”

    (from [2002e5877efa40b336b70b707670e734c6389958]) Strip_tags never ending attribute should not raise a TypeError [#4870 state:resolved]

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

  • Jeremy Kemper

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

    • Milestone set to 3.0.2

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>

Attachments

Referenced by

Pages