Merge pull request #6384 from andrewpbrett/fix-product-descriptions

correctly display '&' and ' ' in product descriptions

app/serializers/api/product_serializer.rb

@@ -1,8 +1,6 @@
 require "open_food_network/scope_variant_to_hub"
 
 class Api::ProductSerializer < ActiveModel::Serializer
-  include ActionView::Helpers::SanitizeHelper
-
   attributes :id, :name, :permalink, :meta_keywords
   attributes :group_buy, :notes, :description, :description_html
   attributes :properties_with_values, :price
@@ -18,14 +16,12 @@ class Api::ProductSerializer < ActiveModel::Serializer
 
   # return an unformatted descripton
   def description
-    strip_tags object.description&.strip
+    sanitizer.strip_content(object.description)
   end
 
   # return a sanitized html description
   def description_html
-    d = sanitize(object.description, tags: ["p", "b", "strong", "em", "i", "a", "u"],
-                                     attributes: ["href", "target"])
-    d.to_s.html_safe
+    sanitizer.sanitize_content(object.description)&.html_safe
   end
 
   def properties_with_values
@@ -47,4 +43,10 @@ class Api::ProductSerializer < ActiveModel::Serializer
       object.master.price_with_fees(options[:current_distributor], options[:current_order_cycle])
     end
   end
+
+  private
+
+  def sanitizer
+    @sanitizer ||= ContentSanitizer.new
+  end
 end

app/services/content_sanitizer.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+# Sanitizes and cleans up user-provided content that may contain tags, special characters, etc.
+
+class ContentSanitizer
+  include ActionView::Helpers::SanitizeHelper
+
+  ALLOWED_TAGS = ["p", "b", "strong", "em", "i", "a", "u"].freeze
+  ALLOWED_ATTRIBUTES = ["href", "target"].freeze
+  FILTERED_CHARACTERS = {
+    "&" => "&",
+    "&" => "&",
+    " " => " "
+  }.freeze
+
+  def strip_content(content)
+    return unless content.present?
+
+    content = strip_tags(content.to_s.strip)
+
+    filter_characters(content)
+  end
+
+  def sanitize_content(content)
+    return unless content.present?
+
+    content = sanitize(content.to_s, tags: ALLOWED_TAGS, attributes: ALLOWED_ATTRIBUTES)
+
+    filter_characters(content)
+  end
+
+  private
+
+  def filter_characters(content)
+    FILTERED_CHARACTERS.each do |character, sub|
+      content = content.gsub(character, sub)
+    end
+    content
+  end
+end

spec/services/content_sanitizer_spec.rb

@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ContentSanitizer do
+  let(:service) { described_class.new }
+
+  context "#strip_content" do
+    it "strips disallowed tags" do
+      expect(service.strip_content("I'm friendly!<script>alert('hello! I'm malicious');</script>")).to eq("I'm friendly!")
+    end
+
+    it "replaces spaces" do
+      expect(service.strip_content("swiss&nbsp;chard")).to eq("swiss chard")
+    end
+
+    it "replaces ampersands" do
+      expect(service.strip_content("pb &amp; j")).to eq("pb & j")
+    end
+
+    it "replaces double escaped ampersands" do
+      expect(service.strip_content("pb &amp;amp; j")).to eq("pb & j")
+    end
+
+    it "echos nil if given nil" do
+      expect(service.strip_content(nil)).to be(nil)
+    end
+  end
+
+  context "#sanitize_content" do
+    it "leaves bold tags" do
+      bold = "<b>I'm bold</b>"
+      expect(service.sanitize_content(bold)).to eq(bold)
+    end
+
+    it "leaves links intact" do
+      link = "<a href=\"https://foo.com\">Bar</a>"
+      expect(service.sanitize_content(link)).to eq(link)
+    end
+
+    it "replaces spaces" do
+      expect(service.sanitize_content("swiss&nbsp;chard")).to eq("swiss chard")
+    end
+
+    it "replaces ampersands" do
+      expect(service.sanitize_content("pb &amp; j")).to eq("pb & j")
+    end
+
+    it "replaces double escaped ampersands" do
+      expect(service.sanitize_content("pb &amp;amp; j")).to eq("pb & j")
+    end
+
+    it "echos nil if given nil" do
+      expect(service.sanitize_content(nil)).to be(nil)
+    end
+  end
+end