Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

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

#4870

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'.

Reported by Łukasz · June 16th, 2010 @ 11:34 AM

State: resolved
Milestone: 3.0.2
Assigned to: José Valim José Valim
Importance: Low

Activity

  1. Łukasz
    Łukasz

    The string:

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

    Edited by Rohit Arondekar to fix formating.

    June 16th, 2010 @ 11:37 AM

  2. Sebastian Gräßl
    Sebastian Gräßl

    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

    June 20th, 2010 @ 09:54 AM

  3. Rohit Arondekar
    Rohit Arondekar
    • 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.

    June 20th, 2010 @ 11:09 AM

  4. Łukasz
    Łukasz

    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

    June 21st, 2010 @ 08:43 AM

  5. Bruno Michel
    Bruno Michel
    • Tag changed from actionpack, html to actionpack, html, patch
    • Importance changed from to Low

    I've made a patch to fix this bug.

    June 26th, 2010 @ 06:02 PM

  6. Bruno Michel
    Bruno Michel
    • Assigned user set to José Valim

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

    June 28th, 2010 @ 09:41 AM

  7. José Valim
    José Valim

    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?

    June 28th, 2010 @ 09:58 AM

  8. Bruno Michel
    Bruno Michel

    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.

    June 28th, 2010 @ 10:29 AM

  9. José Valim
    José Valim
    • 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!

    June 28th, 2010 @ 10:39 AM

  10. Repository
    Repository
    • State changed from open to resolved

    June 28th, 2010 @ 10:40 AM

  11. Jeremy Kemper