diff options
| -rw-r--r-- | lib/redcloth3.rb | 14 | ||||
| -rw-r--r-- | lib/redmine/syntax_highlighting.rb | 16 | ||||
| -rw-r--r-- | lib/redmine/wiki_formatting/markdown/formatter.rb | 2 | ||||
| -rw-r--r-- | lib/redmine/wiki_formatting/textile/formatter.rb | 10 | ||||
| -rw-r--r-- | public/stylesheets/application.css | 2 | ||||
| -rw-r--r-- | test/unit/helpers/application_helper_test.rb | 13 | ||||
| -rw-r--r-- | test/unit/lib/redmine/wiki_formatting/markdown_formatter_test.rb | 9 | ||||
| -rw-r--r-- | test/unit/lib/redmine/wiki_formatting/textile_formatter_test.rb | 54 |
8 files changed, 104 insertions, 16 deletions
diff --git a/lib/redcloth3.rb b/lib/redcloth3.rb index 31051fa96..d0bd217d3 100644 --- a/lib/redcloth3.rb +++ b/lib/redcloth3.rb @@ -494,7 +494,15 @@ class RedCloth3 < String style << "text-align:#{ h_align( $& ) };" if text =~ A_HLGN cls, id = $1, $2 if cls =~ /^(.*?)#(.*)$/ - + + # add wiki-class- and wiki-id- to classes and ids to prevent setting of + # arbitrary classes and ids + cls = cls.split(/\s+/).map do |c| + c.starts_with?('wiki-class-') ? c : "wiki-class-#{c}" + end.join(' ') if cls + + id = id.starts_with?('wiki-id-') ? id : "wiki-id-#{id}" if id + atts = '' atts << " style=\"#{ style.join }\"" unless style.empty? atts << " class=\"#{ cls }\"" unless cls.to_s.empty? @@ -1097,7 +1105,7 @@ class RedCloth3 < String first.match(/<#{ OFFTAGS }([^>]*)>/) tag = $1 $2.to_s.match(/(class\=("[^"]+"|'[^']+'))/i) - tag << " #{$1}" if $1 + tag << " #{$1}" if $1 && tag == 'code' @pre_list << "<#{ tag }>#{ aftertag }" end elsif $1 and codepre > 0 @@ -1202,8 +1210,8 @@ class RedCloth3 < String end end - ALLOWED_TAGS = %w(redpre pre code notextile) + ALLOWED_TAGS = %w(redpre pre code kbd notextile) def escape_html_tags(text) text.gsub!(%r{<(\/?([!\w]+)[^<>\n]*)(>?)}) {|m| ALLOWED_TAGS.include?($2) ? "<#{$1}#{$3}" : "<#{$1}#{'>' unless $3.blank?}" } end diff --git a/lib/redmine/syntax_highlighting.rb b/lib/redmine/syntax_highlighting.rb index 7480ebd16..7f4334977 100644 --- a/lib/redmine/syntax_highlighting.rb +++ b/lib/redmine/syntax_highlighting.rb @@ -40,6 +40,16 @@ module Redmine rescue ERB::Util.h(text) end + + def language_supported?(language) + if highlighter.respond_to? :language_supported? + highlighter.language_supported? language + else + true + end + rescue + false + end end module CodeRay @@ -58,6 +68,12 @@ module Redmine def highlight_by_language(text, language) ::CodeRay.scan(text, language).html(:wrap => :span) end + + def language_supported?(language) + ::CodeRay::Scanners.list.include?(language.to_s.downcase.to_sym) + rescue + false + end end end end diff --git a/lib/redmine/wiki_formatting/markdown/formatter.rb b/lib/redmine/wiki_formatting/markdown/formatter.rb index 4afbc2fdd..bfb04774c 100644 --- a/lib/redmine/wiki_formatting/markdown/formatter.rb +++ b/lib/redmine/wiki_formatting/markdown/formatter.rb @@ -35,7 +35,7 @@ module Redmine end def block_code(code, language) - if language.present? + if language.present? && Redmine::SyntaxHighlighting.language_supported?(language) "<pre><code class=\"#{CGI.escapeHTML language} syntaxhl\">" + Redmine::SyntaxHighlighting.highlight_by_language(code, language) + "</code></pre>" diff --git a/lib/redmine/wiki_formatting/textile/formatter.rb b/lib/redmine/wiki_formatting/textile/formatter.rb index 91ea14960..a698cad45 100644 --- a/lib/redmine/wiki_formatting/textile/formatter.rb +++ b/lib/redmine/wiki_formatting/textile/formatter.rb @@ -121,8 +121,14 @@ module Redmine text.gsub!(/<redpre#(\d+)>/) do content = @pre_list[$1.to_i] if content.match(/<code\s+class="(\w+)">\s?(.+)/m) - content = "<code class=\"#{$1} syntaxhl\">" + - Redmine::SyntaxHighlighting.highlight_by_language($2, $1) + language = $1 + text = $2 + if Redmine::SyntaxHighlighting.language_supported?(language) + content = "<code class=\"#{language} syntaxhl\">" + + Redmine::SyntaxHighlighting.highlight_by_language(text, language) + else + content = "<code>#{ERB::Util.h(text)}" + end end content end diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index f9a769eea..c49e97c2d 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -941,7 +941,7 @@ div.wiki table, div.wiki td, div.wiki th { padding: 4px; } -div.wiki .noborder, div.wiki .noborder td, div.wiki .noborder th {border:0;} +div.wiki .wiki-class-noborder, div.wiki .wiki-class-noborder td, div.wiki .wiki-class-noborder th {border:0;} div.wiki .external { background-position: 0% 60%; diff --git a/test/unit/helpers/application_helper_test.rb b/test/unit/helpers/application_helper_test.rb index 987668396..cbb4ce8a7 100644 --- a/test/unit/helpers/application_helper_test.rb +++ b/test/unit/helpers/application_helper_test.rb @@ -117,7 +117,8 @@ class ApplicationHelperTest < ActionView::TestCase to_test = { '!http://foo.bar/image.jpg!' => '<img src="http://foo.bar/image.jpg" alt="" />', 'floating !>http://foo.bar/image.jpg!' => 'floating <span style="float:right"><img src="http://foo.bar/image.jpg" alt="" /></span>', - 'with class !(some-class)http://foo.bar/image.jpg!' => 'with class <img src="http://foo.bar/image.jpg" class="some-class" alt="" />', + 'with class !(some-class)http://foo.bar/image.jpg!' => 'with class <img src="http://foo.bar/image.jpg" class="wiki-class-some-class" alt="" />', + 'with class !(wiki-class-foo)http://foo.bar/image.jpg!' => 'with class <img src="http://foo.bar/image.jpg" class="wiki-class-foo" alt="" />', 'with style !{width:100px;height:100px}http://foo.bar/image.jpg!' => 'with style <img src="http://foo.bar/image.jpg" style="width:100px;height:100px;" alt="" />', 'with title !http://foo.bar/image.jpg(This is a title)!' => 'with title <img src="http://foo.bar/image.jpg" title="This is a title" alt="This is a title" />', 'with title !http://foo.bar/image.jpg(This is a double-quoted "title")!' => 'with title <img src="http://foo.bar/image.jpg" title="This is a double-quoted "title"" alt="This is a double-quoted "title"" />', @@ -905,11 +906,11 @@ RAW "<pre><div>content</div></pre>" => "<pre><div>content</div></pre>", "HTML comment: <!-- no comments -->" => "<p>HTML comment: <!-- no comments --></p>", "<!-- opening comment" => "<p><!-- opening comment</p>", - # remove attributes except class - "<pre class='foo'>some text</pre>" => "<pre class='foo'>some text</pre>", - '<pre class="foo">some text</pre>' => '<pre class="foo">some text</pre>', - "<pre class='foo bar'>some text</pre>" => "<pre class='foo bar'>some text</pre>", - '<pre class="foo bar">some text</pre>' => '<pre class="foo bar">some text</pre>', + # remove attributes including class + "<pre class='foo'>some text</pre>" => "<pre>some text</pre>", + '<pre class="foo">some text</pre>' => '<pre>some text</pre>', + "<pre class='foo bar'>some text</pre>" => "<pre>some text</pre>", + '<pre class="foo bar">some text</pre>' => '<pre>some text</pre>', "<pre onmouseover='alert(1)'>some text</pre>" => "<pre>some text</pre>", # xss '<pre><code class=""onmouseover="alert(1)">text</code></pre>' => '<pre><code>text</code></pre>', diff --git a/test/unit/lib/redmine/wiki_formatting/markdown_formatter_test.rb b/test/unit/lib/redmine/wiki_formatting/markdown_formatter_test.rb index b8aa37ff2..77dd93c7d 100644 --- a/test/unit/lib/redmine/wiki_formatting/markdown_formatter_test.rb +++ b/test/unit/lib/redmine/wiki_formatting/markdown_formatter_test.rb @@ -70,6 +70,15 @@ STR end end + def test_should_not_allow_invalid_language_for_code_blocks + text = <<-STR +~~~foo +test +~~~ +STR + assert_equal "<pre>test\n</pre>", @formatter.new(text).to_html + end + def test_external_links_should_have_external_css_class text = 'This is a [link](http://example.net/)' assert_equal '<p>This is a <a href="http://example.net/" class="external">link</a></p>', @formatter.new(text).to_html.strip diff --git a/test/unit/lib/redmine/wiki_formatting/textile_formatter_test.rb b/test/unit/lib/redmine/wiki_formatting/textile_formatter_test.rb index 03d4ef5e6..7e5ef8cb2 100644 --- a/test/unit/lib/redmine/wiki_formatting/textile_formatter_test.rb +++ b/test/unit/lib/redmine/wiki_formatting/textile_formatter_test.rb @@ -44,9 +44,7 @@ class Redmine::WikiFormatting::TextileFormatterTest < ActionView::TestCase '*two*words*' => '<strong>two*words</strong>', '*two * words*' => '<strong>two * words</strong>', '*two* *words*' => '<strong>two</strong> <strong>words</strong>', - '*(two)* *(words)*' => '<strong>(two)</strong> <strong>(words)</strong>', - # with class - '*(foo)two words*' => '<strong class="foo">two words</strong>' + '*(two)* *(words)*' => '<strong>(two)</strong> <strong>(words)</strong>' ) end @@ -166,6 +164,12 @@ EXPECTED ) end + def test_kbd + assert_html_output({ + '<kbd>test</kbd>' => '<kbd>test</kbd>' + }, false) + end + def test_use_of_backslashes_followed_by_numbers_in_headers assert_html_output({ 'h1. 2009\02\09' => '<h1>2009\02\09</h1>' @@ -529,6 +533,50 @@ STR assert_match /\Ah1.\tHeading 1\s+Content 1\z/, @formatter.new(text).get_section(1).first end + def test_should_not_allow_arbitrary_class_attribute_on_offtags + %w(code pre kbd).each do |tag| + assert_html_output({"<#{tag} class=\"foo\">test</#{tag}>" => "<#{tag}>test</#{tag}>"}, false) + end + + assert_html_output({"<notextile class=\"foo\">test</notextile>" => "test"}, false) + end + + def test_should_allow_valid_language_class_attribute_on_code_tags + assert_html_output({"<code class=\"ruby\">test</code>" => "<code class=\"ruby syntaxhl\"><span class=\"CodeRay\">test</span></code>"}, false) + end + + def test_should_not_allow_valid_language_class_attribute_on_non_code_offtags + %w(pre kbd).each do |tag| + assert_html_output({"<#{tag} class=\"ruby\">test</#{tag}>" => "<#{tag}>test</#{tag}>"}, false) + end + + assert_html_output({"<notextile class=\"ruby\">test</notextile>" => "test"}, false) + end + + def test_should_prefix_class_attribute_on_tags + assert_html_output({ + '!(foo)test.png!' => "<p><img src=\"test.png\" class=\"wiki-class-foo\" alt=\"\" /></p>", + '%(foo)test%' => "<p><span class=\"wiki-class-foo\">test</span></p>", + 'p(foo). test' => "<p class=\"wiki-class-foo\">test</p>", + '|(foo). test|' => "<table>\n\t\t<tr>\n\t\t\t<td class=\"wiki-class-foo\">test</td>\n\t\t</tr>\n\t</table>", + }, false) + end + + def test_should_prefix_id_attribute_on_tags + assert_html_output({ + '!(#foo)test.png!' => "<p><img src=\"test.png\" id=\"wiki-id-foo\" alt=\"\" /></p>", + '%(#foo)test%' => "<p><span id=\"wiki-id-foo\">test</span></p>", + 'p(#foo). test' => "<p id=\"wiki-id-foo\">test</p>", + '|(#foo). test|' => "<table>\n\t\t<tr>\n\t\t\t<td id=\"wiki-id-foo\">test</td>\n\t\t</tr>\n\t</table>", + }, false) + end + + def test_should_not_prefix_class_and_id_attributes_already_prefixed + assert_html_output({ + '!(wiki-class-foo#wiki-id-bar)test.png!' => "<p><img src=\"test.png\" class=\"wiki-class-foo\" id=\"wiki-id-bar\" alt=\"\" /></p>", + }, false) + end + private def assert_html_output(to_test, expect_paragraph = true) |