Commit ae1d9fb46aa2b07ee9836d49862ec4e2c46fbbba
Exists in
master
and in
4 other branches
Merge pull request #1512 from tsigo/escape_gfm
Better escaping of text passed into GFM
Showing
11 changed files
with
23 additions
and
14 deletions
Show diff stats
app/helpers/gitlab_markdown_helper.rb
| ... | ... | @@ -12,8 +12,8 @@ module GitlabMarkdownHelper |
| 12 | 12 | # "<a>outer text </a><a>gfm ref</a><a> more outer text</a>"). |
| 13 | 13 | def link_to_gfm(body, url, html_options = {}) |
| 14 | 14 | return "" if body.blank? |
| 15 | - | |
| 16 | - gfm_body = gfm(body, html_options) | |
| 15 | + | |
| 16 | + gfm_body = gfm(escape_once(body), html_options) | |
| 17 | 17 | |
| 18 | 18 | gfm_body.gsub!(%r{<a.*?>.*?</a>}m) do |match| |
| 19 | 19 | "</a>#{match}#{link_to("", url, html_options)[0..-5]}" # "</a>".length +1 | ... | ... |
app/views/commits/_commit_box.html.haml
| ... | ... | @@ -11,10 +11,10 @@ |
| 11 | 11 | = link_to tree_project_ref_path(@project, @commit.id), class: "browse-button primary grouped" do |
| 12 | 12 | %strong Browse Code » |
| 13 | 13 | %h3.commit-title.page_title |
| 14 | - = gfm @commit.title | |
| 14 | + = gfm escape_once(@commit.title) | |
| 15 | 15 | - if @commit.description.present? |
| 16 | 16 | %pre.commit-description |
| 17 | - = gfm @commit.description | |
| 17 | + = gfm escape_once(@commit.description) | |
| 18 | 18 | .commit-info |
| 19 | 19 | .row |
| 20 | 20 | .span4 | ... | ... |
app/views/events/_commit.html.haml
| ... | ... | @@ -5,4 +5,4 @@ |
| 5 | 5 | %strong.cdark= commit.author_name |
| 6 | 6 | – |
| 7 | 7 | = image_tag gravatar_icon(commit.author_email), class: "avatar", width: 16 |
| 8 | - = gfm truncate(commit.title, length: 50) rescue "--broken encoding" | |
| 8 | + = gfm escape_once(truncate(commit.title, length: 50)) rescue "--broken encoding" | ... | ... |
app/views/issues/show.html.haml
app/views/merge_requests/show/_mr_box.html.haml
app/views/milestones/show.html.haml
app/views/repositories/_branch.html.haml
| ... | ... | @@ -11,7 +11,7 @@ |
| 11 | 11 | %code= commit.short_id |
| 12 | 12 | |
| 13 | 13 | = image_tag gravatar_icon(commit.author_email), class: "", width: 16 |
| 14 | - = gfm truncate(commit.title, length: 40) | |
| 14 | + = gfm escape_once(truncate(commit.title, length: 40)) | |
| 15 | 15 | %span.update-author.right |
| 16 | 16 | = time_ago_in_words(commit.committed_date) |
| 17 | 17 | ago | ... | ... |
app/views/repositories/_feed.html.haml
| ... | ... | @@ -13,7 +13,7 @@ |
| 13 | 13 | = link_to project_commits_path(@project, commit.id) do |
| 14 | 14 | %code= commit.short_id |
| 15 | 15 | = image_tag gravatar_icon(commit.author_email), class: "", width: 16 |
| 16 | - = gfm truncate(commit.title, length: 40) | |
| 16 | + = gfm escape_once(truncate(commit.title, length: 40)) | |
| 17 | 17 | %td |
| 18 | 18 | %span.right.cgray |
| 19 | 19 | = time_ago_in_words(commit.committed_date) | ... | ... |
app/views/repositories/tags.html.haml
| ... | ... | @@ -17,7 +17,7 @@ |
| 17 | 17 | = link_to project_commit_path(@project, commit.id) do |
| 18 | 18 | %code= commit.short_id |
| 19 | 19 | = image_tag gravatar_icon(commit.author_email), class: "", width: 16 |
| 20 | - = gfm truncate(commit.title, length: 40) | |
| 20 | + = gfm escape_once(truncate(commit.title, length: 40)) | |
| 21 | 21 | %td |
| 22 | 22 | %span.update-author.right |
| 23 | 23 | = time_ago_in_words(commit.committed_date) | ... | ... |
lib/gitlab/markdown.rb
| ... | ... | @@ -48,8 +48,10 @@ module Gitlab |
| 48 | 48 | def gfm(text, html_options = {}) |
| 49 | 49 | return text if text.nil? |
| 50 | 50 | |
| 51 | - # prevents the string supplied through the _text_ argument to be altered | |
| 52 | - text = text.dup | |
| 51 | + # Duplicate the string so we don't alter the original, then call to_str | |
| 52 | + # to cast it back to a String instead of a SafeBuffer. This is required | |
| 53 | + # for gsub calls to work as we need them to. | |
| 54 | + text = text.dup.to_str | |
| 53 | 55 | |
| 54 | 56 | @html_options = html_options |
| 55 | 57 | ... | ... |
spec/helpers/gitlab_markdown_helper_spec.rb
| ... | ... | @@ -292,11 +292,18 @@ describe GitlabMarkdownHelper do |
| 292 | 292 | actual = link_to_gfm("Fixed in #{commit.id}", commit_path, class: 'foo') |
| 293 | 293 | actual.should have_selector 'a.gfm.gfm-commit.foo' |
| 294 | 294 | end |
| 295 | + | |
| 296 | + it "escapes HTML passed in as the body" do | |
| 297 | + actual = "This is a <h1>test</h1> - see ##{issues[0].id}" | |
| 298 | + link_to_gfm(actual, commit_path).should match('<h1>test</h1>') | |
| 299 | + end | |
| 295 | 300 | end |
| 296 | 301 | |
| 297 | 302 | describe "#markdown" do |
| 298 | 303 | it "should handle references in paragraphs" do |
| 299 | - markdown("\n\nLorem ipsum dolor sit amet, consectetur adipiscing elit. #{commit.id} Nam pulvinar sapien eget odio adipiscing at faucibus orci vestibulum.\n").should == "<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. #{link_to commit.id, project_commit_path(project, commit), title: commit.link_title, class: "gfm gfm-commit "} Nam pulvinar sapien eget odio adipiscing at faucibus orci vestibulum.</p>\n" | |
| 304 | + actual = "\n\nLorem ipsum dolor sit amet. #{commit.id} Nam pulvinar sapien eget.\n" | |
| 305 | + expected = project_commit_path(project, commit) | |
| 306 | + markdown(actual).should match(expected) | |
| 300 | 307 | end |
| 301 | 308 | |
| 302 | 309 | it "should handle references in headers" do | ... | ... |