From 5640070915e3b18a29e3a941dc96a603529fb40c Mon Sep 17 00:00:00 2001
From: Rodrigo Souto <rodrigo@colivre.coop.br>
Date: Tue, 29 Sep 2015 18:16:29 -0300
Subject: [PATCH] avoid html double escape

---
 test/unit/article_test.rb                         |  8 --------
 test/unit/comment_test.rb                         | 23 ++++++++++++-----------
 test/unit/community_test.rb                       | 14 --------------
 test/unit/environment_test.rb                     |  8 --------
 test/unit/event_test.rb                           | 28 ----------------------------
 test/unit/folder_test.rb                          |  8 --------
 test/unit/gallery_test.rb                         |  8 --------
 test/unit/organization_test.rb                    | 19 -------------------
 test/unit/product_test.rb                         | 10 ----------
 test/unit/profile_test.rb                         | 28 ----------------------------
 test/unit/text_article_test.rb                    |  9 ---------
 test/unit/validation_info_test.rb                 | 10 ----------
 vendor/plugins/xss_terminate/lib/xss_terminate.rb | 52 ++++------------------------------------------------
 13 files changed, 16 insertions(+), 209 deletions(-)

diff --git a/test/unit/article_test.rb b/test/unit/article_test.rb
index 2d337ac..0787cb3 100644
--- a/test/unit/article_test.rb
+++ b/test/unit/article_test.rb
@@ -931,14 +931,6 @@ class ArticleTest < ActiveSupport::TestCase
     assert_no_match(/<script>/, a.name)
   end
 
-  should 'escape malformed html tags' do
-    article = Article.new
-    article.name = "<h1 Malformed >> html >< tag"
-    article.valid?
-
-    assert_no_match /[<>]/, article.name
-  end
-
   should 'return truncated title in short_title' do
     article = Article.new
     article.name = 'a123456789abcdefghij'
diff --git a/test/unit/comment_test.rb b/test/unit/comment_test.rb
index 4ac6758..cdd63f0 100644
--- a/test/unit/comment_test.rb
+++ b/test/unit/comment_test.rb
@@ -201,17 +201,6 @@ class CommentTest < ActiveSupport::TestCase
     assert comment.errors[:body.to_s].present?
   end
 
-  should 'escape malformed html tags' do
-    owner = create_user('testuser').person
-    article = owner.articles.create(:name => 'test', :body => '...')
-    comment = build(Comment, :article => article, :title => '<h1 title </h1>>> sd f <<', :body => '<h1>> sdf><asd>< body </h1>', :name => '<h1 name </h1>>><<dfsf<sd', :email => 'cracker@test.org')
-    comment.valid?
-
-    assert_no_match /[<>]/, comment.title
-    assert_no_match /[<>]/, comment.body
-    assert_no_match /[<>]/, comment.name
-  end
-
   should 'use an existing image for deleted comments' do
     image = Comment.new.removed_user_image[1..-1]
     assert File.exists?(Rails.root.join('public', image)), "#{image} does not exist."
@@ -749,6 +738,18 @@ class CommentTest < ActiveSupport::TestCase
     comment.destroy
   end
 
+  should 'not double escape html content after validation' do
+    comment = create_comment
+    body = 'Comment with "quotes"'
+    comment.body = body
+
+    comment.valid?
+    assert_equal body, comment.body
+
+    comment.valid?
+    assert_equal body, comment.body
+  end
+
   private
 
   def create_comment(args = {})
diff --git a/test/unit/community_test.rb b/test/unit/community_test.rb
index 5887d9f..e0edf22 100644
--- a/test/unit/community_test.rb
+++ b/test/unit/community_test.rb
@@ -242,20 +242,6 @@ class CommunityTest < ActiveSupport::TestCase
     end
   end
 
-  should 'escape malformed html tags' do
-    community = Community.new
-    community.name = "<h1 Malformed >> html >< tag"
-    community.address = "<h1 Malformed >,<<<asfdf> html >< tag"
-    community.contact_phone = "<h1 Malformed<<> >> html >><>< tag"
-    community.description = "<h1 Malformed /h1>>><<> html ><>h1< tag"
-    community.valid?
-
-    assert_no_match /[<>]/, community.name
-    assert_no_match /[<>]/, community.address
-    assert_no_match /[<>]/, community.contact_phone
-    assert_no_match /[<>]/, community.description
-  end
-
   should "the followed_by method be protected and true to the community members by default" do
     c = fast_create(Community)
     p1 = fast_create(Person)
diff --git a/test/unit/environment_test.rb b/test/unit/environment_test.rb
index 55a91ca..b4f30f1 100644
--- a/test/unit/environment_test.rb
+++ b/test/unit/environment_test.rb
@@ -1140,14 +1140,6 @@ class EnvironmentTest < ActiveSupport::TestCase
     assert_equal "<h1> Disabled Enterprise </h1>", environment.message_for_disabled_enterprise
   end
 
-  should 'escape malformed html tags' do
-    environment = Environment.new
-    environment.message_for_disabled_enterprise = "<h1> Disabled Enterprise /h1>"
-    environment.valid?
-
-    assert_no_match /[<>]/, environment.message_for_disabled_enterprise
-  end
-
   should 'not sanitize html comments' do
     environment = Environment.new
     environment.message_for_disabled_enterprise = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>'
diff --git a/test/unit/event_test.rb b/test/unit/event_test.rb
index 04ae82b..4356f71 100644
--- a/test/unit/event_test.rb
+++ b/test/unit/event_test.rb
@@ -263,24 +263,6 @@ class EventTest < ActiveSupport::TestCase
     assert_not_includes profile.events.by_day(today), event_out_of_range
   end
 
-  should 'filter fields with full filter' do
-    event = Event.new
-    event.link = "<h1 Malformed >> html >< tag"
-    event.valid?
-
-    assert_no_match /[<>]/, event.link
-  end
-
-  should 'filter fields with white_list filter' do
-    event = Event.new
-    event.body = "<h1> Description </h1>"
-    event.address = "<strong> Address <strong>"
-    event.valid?
-
-    assert_equal "<h1> Description </h1>", event.body
-    assert_equal "<strong> Address <strong>", event.address
-  end
-
   should 'not filter & on link field' do
     event = Event.new
     event.link = 'myevent.com/?param1=value&param2=value2'
@@ -289,16 +271,6 @@ class EventTest < ActiveSupport::TestCase
     assert_equal "http://myevent.com/?param1=value&param2=value2", event.link
   end
 
-  should 'escape malformed html tags' do
-    event = Event.new
-    event.body = "<h1<< Description >>/h1>"
-    event.address = "<strong>><< Address <strong>"
-    event.valid?
-
-    assert_no_match /[<>]/, event.body
-    assert_no_match /[<>]/, event.address
-  end
-
   should 'not sanitize html comments' do
     event = Event.new
     event.body = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>'
diff --git a/test/unit/folder_test.rb b/test/unit/folder_test.rb
index 247c4d5..58d44a5 100644
--- a/test/unit/folder_test.rb
+++ b/test/unit/folder_test.rb
@@ -133,14 +133,6 @@ class FolderTest < ActiveSupport::TestCase
     assert_match  /<!-- .* --> <h1> Wellformed html code <\/h1>/, folder.body
   end
 
-  should 'escape malformed html tags' do
-    folder = Folder.new
-    folder.body = "<h1<< Description >>/h1>"
-    folder.valid?
-
-    assert_no_match /[<>]/, folder.body
-  end
-
   should 'not have a blog as parent' do
     folder = Folder.new
     folder.parent = Blog.new
diff --git a/test/unit/gallery_test.rb b/test/unit/gallery_test.rb
index 8e2d5b8..9ec7a04 100644
--- a/test/unit/gallery_test.rb
+++ b/test/unit/gallery_test.rb
@@ -134,14 +134,6 @@ class GalleryTest < ActiveSupport::TestCase
     assert_match  /<!-- .* --> <h1> Wellformed html code <\/h1>/, gallery.body
   end
 
-  should 'escape malformed html tags' do
-    gallery = Gallery.new
-    gallery.body = "<h1<< Description >>/h1>"
-    gallery.valid?
-
-    assert_no_match /[<>]/, gallery.body
-  end
-
   should 'accept uploads' do
     folder = fast_create(Gallery)
     assert folder.accept_uploads?
diff --git a/test/unit/organization_test.rb b/test/unit/organization_test.rb
index a71d5ff..4d2ce72 100644
--- a/test/unit/organization_test.rb
+++ b/test/unit/organization_test.rb
@@ -253,25 +253,6 @@ class OrganizationTest < ActiveSupport::TestCase
     assert organization.closed
   end
 
-  should 'escape malformed html tags' do
-    organization = Organization.new
-    organization.acronym = "<h1 Malformed >> html >< tag"
-    organization.contact_person = "<h1 Malformed >,<<<asfdf> html >< tag"
-    organization.contact_email = "<h1<malformed@html.com>>"
-    organization.description = "<h1 Malformed /h1>>><<> html ><>h1< tag"
-    organization.legal_form = "<h1 Malformed /h1>>><<> html ><>h1< tag"
-    organization.economic_activity = "<h1 Malformed /h1>>><<> html ><>h1< tag"
-    organization.management_information = "<h1 Malformed /h1>>><<> html ><>h1< tag"
-    organization.valid?
-
-    assert_no_match /[<>]/, organization.acronym
-    assert_no_match /[<>]/, organization.contact_person
-    assert_no_match /[<>]/, organization.contact_email
-    assert_no_match /[<>]/, organization.legal_form
-    assert_no_match /[<>]/, organization.economic_activity
-    assert_no_match /[<>]/, organization.management_information
-  end
-
   should "the followed_by? be true only to members" do
     o = fast_create(Organization)
     p1 = fast_create(Person)
diff --git a/test/unit/product_test.rb b/test/unit/product_test.rb
index 174903f..b156c05 100644
--- a/test/unit/product_test.rb
+++ b/test/unit/product_test.rb
@@ -171,16 +171,6 @@ class ProductTest < ActiveSupport::TestCase
     assert_equal @product_category.name, product.name
   end
 
-  should 'escape malformed html tags' do
-    product = build(Product, :product_category => @product_category)
-    product.name = "<h1 Malformed >> html >< tag"
-    product.description = "<h1 Malformed</h1>><<<a>> >> html >< tag"
-    product.valid?
-
-    assert_no_match /[<>]/, product.name
-    assert_no_match /[<>]/, product.description
-  end
-
   should 'use name of category when has no name yet' do
     product = Product.new
     product.product_category = @product_category
diff --git a/test/unit/profile_test.rb b/test/unit/profile_test.rb
index 8ad26d8..fdc25b1 100644
--- a/test/unit/profile_test.rb
+++ b/test/unit/profile_test.rb
@@ -1696,34 +1696,6 @@ class ProfileTest < ActiveSupport::TestCase
     assert_equal "<strong> Custom Footer <strong>", profile.custom_footer
   end
 
-  should 'escape malformed html tags' do
-    profile = Profile.new
-    profile.name = "<h1 Malformed >> html >>></a>< tag"
-    profile.nickname = "<h1 Malformed <<h1>>< html >< tag"
-    profile.address = "<h1><</h2< Malformed >> html >< tag"
-    profile.contact_phone = "<h1<< Malformed ><>>> html >< tag"
-    profile.description = "<h1<a> Malformed >> html ></a>< tag"
-    profile.valid?
-
-    assert_no_match /[<>]/, profile.name
-    assert_no_match /[<>]/, profile.nickname
-    assert_no_match /[<>]/, profile.address
-    assert_no_match /[<>]/, profile.contact_phone
-    assert_no_match /[<>]/, profile.description
-    assert_no_match /[<>]/, profile.custom_header
-    assert_no_match /[<>]/, profile.custom_footer
-  end
-
-  should 'escape malformed html tags in header and footer' do
-    profile = fast_create(Profile)
-    profile.custom_header = "<h1<a>><<> Malformed >> html ></a>< tag"
-    profile.custom_footer = "<h1> Malformed <><< html ></a>< tag"
-    profile.save
-
-    assert_no_match /[<>]/, profile.custom_header
-    assert_no_match /[<>]/, profile.custom_footer
-  end
-
   should 'not sanitize html comments' do
     profile = Profile.new
     profile.custom_header = '<p><!-- <asdf> << aasdfa >>> --> <h1> Wellformed html code </h1>'
diff --git a/test/unit/text_article_test.rb b/test/unit/text_article_test.rb
index 6ee1ef5..b4e88bd 100644
--- a/test/unit/text_article_test.rb
+++ b/test/unit/text_article_test.rb
@@ -14,15 +14,6 @@ class TextArticleTest < ActiveSupport::TestCase
     assert_includes TextArticle.find(:all), article
   end
 
-  should 'remove HTML from name' do
-    person = create_user('testuser').person
-    article = TextArticle.new(:profile => person)
-    article.name = "<h1 Malformed >> html >>></a>< tag"
-    article.valid?
-
-    assert_no_match /[<>]/, article.name
-  end
-
   should 'be translatable' do
     assert_kind_of Noosfero::TranslatableContent, TextArticle.new
   end
diff --git a/test/unit/validation_info_test.rb b/test/unit/validation_info_test.rb
index b43a1ba..28771c4 100644
--- a/test/unit/validation_info_test.rb
+++ b/test/unit/validation_info_test.rb
@@ -21,14 +21,4 @@ class ValidationInfoTest < ActiveSupport::TestCase
     end
   end
 
-  should 'escape malformed html tags' do
-    info = ValidationInfo.new
-    info.validation_methodology = "<h1 Malformed >> html >< tag"
-    info.restrictions = "<h1 Malformed >> html >< tag"
-    info.valid?
-
-    assert_no_match /[<>]/, info.validation_methodology
-    assert_no_match /[<>]/, info.restrictions
-  end
-
 end
diff --git a/vendor/plugins/xss_terminate/lib/xss_terminate.rb b/vendor/plugins/xss_terminate/lib/xss_terminate.rb
index 3566d85..341f1a8 100644
--- a/vendor/plugins/xss_terminate/lib/xss_terminate.rb
+++ b/vendor/plugins/xss_terminate/lib/xss_terminate.rb
@@ -38,7 +38,7 @@ module XssTerminate
 
   module InstanceMethods
 
-    def sanitize_field(sanitizer, field, serialized = false, with= :full)
+    def sanitize_field(sanitizer, field, serialized = false)
       field = field.to_sym
       if serialized
         puts field
@@ -49,25 +49,11 @@ module XssTerminate
       else
         if self[field]
           self[field] = sanitizer.sanitize(self[field])
-
-          if with == :full
-            self[field] = CGI.escapeHTML(self[field])
-          elsif with == :white_list
-            self[field] = CGI.escapeHTML(self[field]) if !wellformed_html_code?(self[field])
-          end
-
         else
           value = self.send("#{field}")
           return unless value
           value = sanitizer.sanitize(value)
           self.send("#{field}=", value)
-
-          if with == :full
-            self.send("#{field}=", CGI.escapeHTML(value))
-          elsif with == :white_list
-            self.send("#{field}=", CGI.escapeHTML(value)) if !wellformed_html_code?(value)
-          end
-
         end
       end
     end
@@ -86,7 +72,7 @@ module XssTerminate
       sanitizer = ActionView::Base.full_sanitizer
       columns, columns_serialized = sanitize_columns(:full)
       columns.each do |column|
-        sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column), :full)
+        sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column))
       end
     end
 
@@ -94,7 +80,7 @@ module XssTerminate
       sanitizer = ActionView::Base.white_list_sanitizer
       columns, columns_serialized = sanitize_columns(:white_list)
       columns.each do |column|
-        sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column), :white_list)
+        sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column))
       end
    end
 
@@ -102,38 +88,8 @@ module XssTerminate
       sanitizer = HTML5libSanitize.new
       columns = sanitize_columns(:html5lib)
       columns.each do |column|
-        sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column), :html5lib)
-      end
-    end
-
-    def wellformed_html_code?(field)
-      return true if !field
-      counter = 0
-      in_comment = false
-      field=field.split(//)
-      for i in 0..field.length-1
-        if !in_comment
-          if field[i] == '<'
-            if field[i+1..i+3] == ["!","-","-"]
-              in_comment = true
-            else
-              counter += 1
-            end
-          elsif field[i] == '>'
-            counter -= 1
-          end
-        else
-          if field[i-2..i] == ["-","-",">"]
-            in_comment = false
-          end
-        end
-
-        if counter < 0 || 1 < counter
-          return false
-        end
+        sanitize_field(sanitizer, column.to_sym, columns_serialized.include?(column))
       end
-
-      return counter == 0
     end
 
   end
--
libgit2 0.21.2