From 5ca0760f7c978783e1e54050602906bd52337805 Mon Sep 17 00:00:00 2001 From: Jacques Distler Date: Thu, 15 May 2008 01:22:13 -0500 Subject: [PATCH] Efficiency: Sanitize Once Envoke the HTML5lib Sanitizer just once (when the content is finally rendered), rather than each time it passes through the chunk-handler. --- lib/chunks/engines.rb | 19 +-- lib/wiki_content.rb | 4 +- test/functional/file_controller_test.rb | 2 +- test/unit/page_renderer_test.rb | 187 ++++++++++++------------ 4 files changed, 100 insertions(+), 112 deletions(-) diff --git a/lib/chunks/engines.rb b/lib/chunks/engines.rb index 013c424a..2576ad14 100644 --- a/lib/chunks/engines.rb +++ b/lib/chunks/engines.rb @@ -25,19 +25,16 @@ module Engines class Textile < AbstractEngine def mask - require 'sanitize' require 'redcloth' redcloth = RedCloth.new(@content, [:hard_breaks] + @content.options[:engine_opts]) redcloth.filter_html = false redcloth.no_span_caps = false html = redcloth.to_html(:textile) - sanitize_xhtml(html) end end class Markdown < AbstractEngine def mask - require 'sanitize' require 'maruku' require 'maruku/ext/math' @@ -48,10 +45,8 @@ module Engines :author => @content.options[:engine_opts][:author], :title => @content.options[:engine_opts][:title]}) @content.options[:renderer].s5_theme = my_content.s5_theme - sanitize_xhtml(my_content.to_s5) else - html = sanitize_rexml(Maruku.new(@content.delete("\r"), - {:math_enabled => false}).to_html_tree) + html = Maruku.new(@content.delete("\r"), {:math_enabled => false}).to_html html.gsub(/\A
\n?(.*?)\n?<\/div>\Z/m, '\1') end @@ -60,7 +55,6 @@ module Engines class MarkdownMML < AbstractEngine def mask - require 'sanitize' require 'maruku' require 'maruku/ext/math' @@ -72,35 +66,30 @@ module Engines :author => @content.options[:engine_opts][:author], :title => @content.options[:engine_opts][:title]}) @content.options[:renderer].s5_theme = my_content.s5_theme - sanitize_xhtml(my_content.to_s5) + my_content.to_s5 else - html = sanitize_rexml(Maruku.new(@content.delete("\r"), + html = Maruku.new(@content.delete("\r"), {:math_enabled => true, - :math_numbered => ['\\[','\\begin{equation}']}).to_html_tree) + :math_numbered => ['\\[','\\begin{equation}']}).to_html html.gsub(/\A
\n?(.*?)\n?<\/div>\Z/m, '\1') end - end end class Mixed < AbstractEngine def mask - require 'sanitize' require 'redcloth' redcloth = RedCloth.new(@content, @content.options[:engine_opts]) redcloth.filter_html = false redcloth.no_span_caps = false html = redcloth.to_html - sanitize_xhtml(html) end end class RDoc < AbstractEngine def mask - require 'sanitize' require_dependency 'rdocsupport' html = RDocSupport::RDocFormatter.new(@content).to_html - sanitize_xhtml(html) end end diff --git a/lib/wiki_content.rb b/lib/wiki_content.rb index 94e3e3f3..c3e9de1e 100644 --- a/lib/wiki_content.rb +++ b/lib/wiki_content.rb @@ -4,8 +4,8 @@ require 'chunks/category' require_dependency 'chunks/include' require_dependency 'chunks/wiki' require_dependency 'chunks/literal' -#require_dependency 'chunks/uri' require 'chunks/nowiki' +require 'sanitize' # Wiki content is just a string that can process itself with a chain of # actions. The actions can modify wiki content so that certain parts of @@ -192,7 +192,7 @@ class WikiContent < String chunk.unmask_text end end - self + self.replace sanitize_xhtml(self) end def page_name diff --git a/test/functional/file_controller_test.rb b/test/functional/file_controller_test.rb index 6a05b0d8..ac18d831 100755 --- a/test/functional/file_controller_test.rb +++ b/test/functional/file_controller_test.rb @@ -80,7 +80,7 @@ class FileControllerTest < Test::Unit::TestCase renderer = PageRenderer.new @wiki.revise_page('wiki1', 'HomePage', '[[rails-e2e.gif:pic]]', Time.now, 'AnonymousBrave', renderer) - assert_equal "

rails-e2e.gif" + + assert_equal "

rails-e2e.gif" + "?

", renderer.display_content diff --git a/test/unit/page_renderer_test.rb b/test/unit/page_renderer_test.rb index 23d6056a..943f698d 100644 --- a/test/unit/page_renderer_test.rb +++ b/test/unit/page_renderer_test.rb @@ -14,14 +14,14 @@ class PageRendererTest < Test::Unit::TestCase @web.add_page('SecondPage', 'Yo, yo. Have you EverBeenHated', Time.now, 'DavidHeinemeierHansson', test_renderer) - assert_equal('

Yo, yo. Have you Ever Been Hated' + - '?

', + assert_equal("

Yo, yo. Have you Ever Been Hated" + + "?

", rendered_content(@web.page("SecondPage"))) @web.add_page('EverBeenHated', 'Yo, yo. Have you EverBeenHated', Time.now, 'DavidHeinemeierHansson', test_renderer) - assert_equal('

Yo, yo. Have you Ever Been Hated

', + assert_equal("

Yo, yo. Have you Ever Been Hated

", rendered_content(@web.page("SecondPage"))) end @@ -42,17 +42,17 @@ class PageRendererTest < Test::Unit::TestCase end def test_content_with_wiki_links - assert_equal '

His Way? ' + - 'would be My Way ' + - '' + - 'sin(x)' + - ' in kinda ' + - 'That Way in ' + - 'His Way? ' + - %{though My Way OverThere \xE2\x80\x93 see } + - 'Smart Engine in that ' + - 'Smart Engine GUI' + - '?

', + assert_equal "

His Way? " + + "would be My Way " + + "" + + "sin(x)" + + " in kinda " + + "That Way in " + + "His Way? " + + %{though My Way OverThere \xE2\x80\x93 see } + + "Smart Engine in that " + + "Smart Engine GUI" + + "?

", test_renderer(@revision).display_content end @@ -60,31 +60,31 @@ class PageRendererTest < Test::Unit::TestCase set_web_property :markup, :markdownMML assert_markup_parsed_as( - %{

equation } + - %{sin(x)

}, + %{

equation } + + %{sin(x)

}, "equation $\\sin(x)$") assert_markup_parsed_as( - %{

My Headline

\n\n

that } + - %{Smart Engine GUI?

}, + %{

My Headline

\n\n

that } + + %{Smart Engine GUI?

}, "My Headline\n===========\n\nthat SmartEngineGUI") assert_markup_parsed_as( - %{

My Headline

\n\n

that } + - %{Smart Engine GUI?

}, + %{

My Headline

\n\n

that } + + %{Smart Engine GUI?

}, "#My Headline#\n\nthat SmartEngineGUI") assert_markup_parsed_as( - %{

SVG } + - %{Math ML?

}, + %{

SVG } + + %{Math ML?

}, "SVG MathML") assert_markup_parsed_as( - %{
sin} + - %{(x)} + - %{
} + + %{
sin} + + %{(x)} + + %{
} + %{\\sin(x) \\begin{svg}\\end{svg}
}, "$$\\sin(x) \\begin{svg}\\end{svg}$$") @@ -103,29 +103,28 @@ class PageRendererTest < Test::Unit::TestCase code_block) assert_markup_parsed_as( - %{

} + - %{sin(x) ecuasi\303\263n

}, + %{

} + + %{sin(x) ecuasi\303\263n

}, "$\\sin(x)$ ecuasi\303\263n") assert_markup_parsed_as( - %{

ecuasi\303\263n

\n
sin} + - %{(x)} + - %{
\\sin(x)
}, + %{

ecuasi\303\263n

\n
sin} + + %{(x)} + + %{
\\sin(x)
}, "ecuasi\303\263n\n$$\\sin(x)$$") assert_markup_parsed_as( - %{

ecuasi\303\263n

\n\n

} + - %{sin(x)

}, + %{

ecuasi\303\263n

\n\n

} + + %{sin(x)

}, "ecuasi\303\263n \n\n$\\sin(x)$") -#FIXME assert_markup_parsed_as( - %{

ecuasi\303\263n } + - %{sin(x)

}, + %{

ecuasi\303\263n } + + %{sin(x)

}, "ecuasi\303\263n $\\sin(x)$") end @@ -135,7 +134,7 @@ class PageRendererTest < Test::Unit::TestCase set_web_property :markup, :markdown assert_markup_parsed_as( - '

text

', + "

text

", '[text](http://example/with/slash)') end @@ -154,7 +153,7 @@ class PageRendererTest < Test::Unit::TestCase set_web_property :markup, :markdown assert_markup_parsed_as( - "

Markdown heading

\n\n" + + "

Markdown heading

\n\n" + "

h2. Textile heading

\n\n" + "

some text with -styles-

\n\n" + "
    \n
  • list 1
  • \n\n
  • list 2
  • \n
", @@ -181,8 +180,8 @@ class PageRendererTest < Test::Unit::TestCase @revision = Revision.new(:page => @page, :content => '+hello+ that SmartEngineGUI', :author => Author.new('DavidHeinemeierHansson')) - assert_equal "hello that Smart Engine GUI" + - "?\n\n", + assert_equal "hello that Smart Engine GUI" + + "?\n\n", test_renderer(@revision).display_content end @@ -197,15 +196,15 @@ class PageRendererTest < Test::Unit::TestCase def test_content_with_aliased_links assert_markup_parsed_as( - '

Would a clever motor' + + "

Would a clever motor" + ' go by any other name?

', 'Would a [[SmartEngine|clever motor]] go by any other name?') end def test_content_with_wikiword_in_em assert_markup_parsed_as( - '

should we go ' + - 'That Way or This Way?' + + "

should we go " + + "That Way or This Way?" + '

', '_should we go ThatWay or ThisWay _') end @@ -213,15 +212,15 @@ class PageRendererTest < Test::Unit::TestCase # wikiwords are invalid as styles, must be in "name: value" form def test_content_with_wikiword_in_style_tag assert_markup_parsed_as( - '

That is some Stylish Emphasis

', - 'That is some Stylish Emphasis') + "

That is some Stylish Emphasis

", + "That is some Stylish Emphasis") end # validates format of style.. def test_content_with_valid_style_in_style_tag assert_markup_parsed_as( - '

That is some Stylish Emphasis

', - 'That is some Stylish Emphasis') + "

That is some Stylish Emphasis

", + "That is some Stylish Emphasis") end def test_content_with_escaped_wikiword @@ -248,24 +247,24 @@ class PageRendererTest < Test::Unit::TestCase def test_content_with_link_in_parentheses assert_markup_parsed_as( - '

(What is a wiki?)

', + "

(What is a wiki?)

", '([What is a wiki?](http://wiki.org/wiki.cgi?WhatIsWiki))') end def test_content_with_image_link assert_markup_parsed_as( - '

This is a Markdown image link.

', + "

This is a Markdown image link.

", 'This ![](http://hobix.com/sample.jpg) is a Markdown image link.') end def test_content_with_inlined_img_tag assert_markup_parsed_as( - '

This is an inline image link.

', + "

This is an inline image link.

", 'This is an inline image link.') # currently, upper case HTML elements are not allowed assert_markup_parsed_as( - '

This <IMG SRC="http://hobix.com/sample.jpg" alt=""></IMG> is an inline image link.

', + "

This <IMG SRC=\"http://hobix.com/sample.jpg\" alt=\"\"/> is an inline image link.

", 'This is an inline image link.') end @@ -279,7 +278,7 @@ class PageRendererTest < Test::Unit::TestCase def test_multiline_nowiki_tag assert_markup_parsed_as( "

Do not mark \n up [[this text]] \nand http://this.url.com but markup " + - 'this?

', + "this?

", "Do not mark \n up [[this text]] \n" + "and http://this.url.com but markup [[this]]") end @@ -293,45 +292,45 @@ class PageRendererTest < Test::Unit::TestCase def test_content_with_bracketted_wiki_word set_web_property :brackets_only, true assert_markup_parsed_as( - '

This is a WikiWord and a tricky name ' + - 'Sperberg-McQueen?.

', + "

This is a WikiWord and a tricky name " + + "Sperberg-McQueen?.

", 'This is a WikiWord and a tricky name [[Sperberg-McQueen]].') end def test_content_for_export - assert_equal '

His Way would be ' + - 'My Way ' + - '' + - 'sin(x)' + - ' in kinda ' + - 'That Way in ' + - 'His Way though ' + - %{My Way OverThere \xE2\x80\x93 see } + - 'Smart Engine in that ' + - 'Smart Engine GUI

', + assert_equal "

His Way would be " + + "My Way " + + "" + + "sin(x)" + + " in kinda " + + "That Way in " + + "His Way though " + + %{My Way OverThere \xE2\x80\x93 see } + + "Smart Engine in that " + + "Smart Engine GUI

", test_renderer(@revision).display_content_for_export end def test_double_replacing @revision.content = "VersionHistory\r\n\r\ncry VersionHistory" - assert_equal '

Version History' + - "?

\n\n

cry " + - 'Version History?' + + assert_equal "

Version History" + + "?

\n\n

cry " + + "Version History?" + '

', test_renderer(@revision).display_content @revision.content = "f\r\nVersionHistory\r\n\r\ncry VersionHistory" - assert_equal "

f Version History" + - "?

\n\n

cry " + - "Version History?" + + assert_equal "

f Version History" + + "?

\n\n

cry " + + "Version History?" + "

", test_renderer(@revision).display_content end def test_difficult_wiki_words @revision.content = "[[It's just awesome GUI!]]" - assert_equal "

It's just awesome GUI!" + - "?

", + assert_equal "

It's just awesome GUI!" + + "?

", test_renderer(@revision).display_content end @@ -347,7 +346,7 @@ class PageRendererTest < Test::Unit::TestCase def test_link_to_file assert_markup_parsed_as( - '

doc.pdf?

', + "

doc.pdf?

", '[[doc.pdf:file]]') end @@ -357,16 +356,16 @@ class PageRendererTest < Test::Unit::TestCase FileUtils.rm_rf("#{RAILS_ROOT}/public/wiki1/files/*") @web.wiki_files.create(:file_name => 'square.jpg', :description => 'Square', :content => 'never mind') assert_markup_parsed_as( - '

Blue Square

', + "

Blue Square

", '[[square.jpg|Blue Square:pic]]') assert_markup_parsed_as( - '

Square

', + "

Square

", '[[square.jpg:pic]]') assert_markup_parsed_as( - '

Blue Square

', + "

Blue Square

", '[[square.jpg|Blue Square:file]]') assert_markup_parsed_as( - '

Square

', + "

Square

", '[[square.jpg:file]]') end @@ -376,33 +375,33 @@ class PageRendererTest < Test::Unit::TestCase FileUtils.rm_rf("#{RAILS_ROOT}/public/wiki1/files/*") @web.wiki_files.create(:file_name => 'square.jpg', :description => '', :content => 'never mind') assert_markup_parsed_as( - '

Blue Square

', + "

Blue Square

", '[[square.jpg|Blue Square:pic]]') assert_markup_parsed_as( - '

', + "

", '[[square.jpg:pic]]') assert_markup_parsed_as( - '

Blue Square

', + "

Blue Square

", '[[square.jpg|Blue Square:file]]') assert_markup_parsed_as( - '

', + "

", '[[square.jpg:file]]') end def test_link_to_non_existant_pic assert_markup_parsed_as( - '

NonExistant?' + + "

NonExistant?" + '

', '[[NonExistant.jpg|NonExistant:pic]]') assert_markup_parsed_as( - '

NonExistant.jpg?' + + "

NonExistant.jpg?" + '

', '[[NonExistant.jpg:pic]]') end def test_wiki_link_with_colon assert_markup_parsed_as( - '

Instiki:Colon?

', + "

Instiki:Colon?

", '[[Instiki:Colon]]') end @@ -413,7 +412,7 @@ class PageRendererTest < Test::Unit::TestCase EOL assert_markup_parsed_as( - "
    \n
  • a
  • \n\n
  • c~ d
  • \n
", + "
    \n
  • a
  • \n\n
  • c~ d
  • \n
", list_with_tildas) end @@ -471,11 +470,11 @@ class PageRendererTest < Test::Unit::TestCase included = @web.add_page('Included', 'link to HomePage', Time.now, 'AnAuthor', test_renderer) main = @web.add_page('Main', '[[!include Included]]', Time.now, 'AnAuthor', test_renderer) - assert_equal '

link to Home Page

', + assert_equal "

link to Home Page

", test_renderer(main).display_content - assert_equal '

link to Home Page

', + assert_equal "

link to Home Page

", test_renderer(main).display_published - assert_equal '

link to Home Page

', + assert_equal "

link to Home Page

", test_renderer(main).display_content_for_export end