From 90e577c811c154fe900d57a884f8a2eb96a2c329 Mon Sep 17 00:00:00 2001 From: kikito Date: Tue, 4 Aug 2015 18:40:58 +0200 Subject: [PATCH 1/3] Adds feature for debate description --- spec/features/debates_spec.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/spec/features/debates_spec.rb b/spec/features/debates_spec.rb index 034b1d504..df9a4a92c 100644 --- a/spec/features/debates_spec.rb +++ b/spec/features/debates_spec.rb @@ -45,21 +45,22 @@ feature 'Debates' do expect(page).to have_content I18n.l(Date.today) end - scenario 'JS injection is sanitized' do + scenario 'JS injection is prevented but safe html is respected' do author = create(:user) login_as(author) visit new_debate_path fill_in 'debate_title', with: 'A test' - fill_in 'debate_description', with: 'This is ' + fill_in 'debate_description', with: '

This is

' check 'debate_terms_of_service' click_button 'Create Debate' expect(page).to have_content 'Debate was successfully created.' expect(page).to have_content 'A test' - expect(page).to have_content 'This is alert("an attack");' + expect(page.html).to include '

This is alert("an attack");

' expect(page.html).to_not include '' + expect(page.html).to_not include '<p>This is' end scenario 'tagging using dangerous strings' do From 1fa4087befa0be1138135f04010b5d72375077b5 Mon Sep 17 00:00:00 2001 From: kikito Date: Tue, 4 Aug 2015 18:42:05 +0200 Subject: [PATCH 2/3] Removes all the extra sanitisation from debates This makes the feature red --- app/views/debates/_debate.html.erb | 2 +- app/views/debates/show.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/debates/_debate.html.erb b/app/views/debates/_debate.html.erb index 4253143a6..6cc218e7a 100644 --- a/app/views/debates/_debate.html.erb +++ b/app/views/debates/_debate.html.erb @@ -5,7 +5,7 @@

<%= link_to debate.title, debate %>

Por <%= debate.author.name %> el <%= l debate.created_at.to_date %>

-

<%= sanitize(debate.description.html_safe) %>

+

<%= debate.description %>

<%= render 'shared/tags', debate: debate %>

diff --git a/app/views/debates/show.html.erb b/app/views/debates/show.html.erb index c9ce7abf1..21f23cdb7 100644 --- a/app/views/debates/show.html.erb +++ b/app/views/debates/show.html.erb @@ -14,7 +14,7 @@ <%= @debate.author.name %>  •  <%= l @debate.created_at.to_date %>  •  <%= pluralize(@debate.comment_threads.count, t("debates.show.comment"), t("debates.show.comments")) %>
- <%= @debate.description.html_safe %> + <%= @debate.description %>

<%= render 'shared/tags', debate: @debate %>

From 87dd655d70dfd13284ac74d62e00446e16c53184 Mon Sep 17 00:00:00 2001 From: kikito Date: Tue, 4 Aug 2015 18:42:49 +0200 Subject: [PATCH 3/3] Make debate.description always html_safe --- app/models/debate.rb | 4 ++++ spec/models/debate_spec.rb | 23 +++++++++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/app/models/debate.rb b/app/models/debate.rb index d82cfb8ac..836e3bf34 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -35,6 +35,10 @@ class Debate < ActiveRecord::Base editable? && author == user end + def description + super.try :html_safe + end + protected def sanitize_description diff --git a/spec/models/debate_spec.rb b/spec/models/debate_spec.rb index f2af2cf0a..00f6217bd 100644 --- a/spec/models/debate_spec.rb +++ b/spec/models/debate_spec.rb @@ -20,15 +20,22 @@ describe Debate do expect(@debate).to_not be_valid end - it "should not be valid without a description" do - @debate.description = nil - expect(@debate).to_not be_valid - end + describe "#description" do + it "should be mandatory" do + @debate.description = nil + expect(@debate).to_not be_valid + end - it "should sanitize the description" do - @debate.description = "" - @debate.valid? - expect(@debate.description).to eq("alert('danger');") + it "should be sanitized" do + @debate.description = "" + @debate.valid? + expect(@debate.description).to eq("alert('danger');") + end + + it "should be html_safe" do + @debate.description = "" + expect(@debate.description).to be_html_safe + end end it "should sanitize the tag list" do