From 8489dbfe1806b85e6de17c9750a14f851ea01391 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sun, 2 Apr 2023 14:55:25 -0400 Subject: [PATCH 1/5] dev: additional rubocop coverage and packages --- .rubocop.yml | 11 +++++++++++ .rubocop_todo.yml | 0 Rakefile | 32 +++++++++++++++++++++++++++----- loofah.gemspec | 4 ++++ 4 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 .rubocop.yml create mode 100644 .rubocop_todo.yml diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 00000000..7e82ad99 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,11 @@ +require: + - rubocop-minitest + - rubocop-performance + - rubocop-rake +inherit_gem: + rubocop-shopify: rubocop.yml +inherit_from: .rubocop_todo.yml + +AllCops: + NewCops: enable + TargetRubyVersion: "2.5" diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml new file mode 100644 index 00000000..e69de29b diff --git a/Rakefile b/Rakefile index 27303d3a..29656f5d 100644 --- a/Rakefile +++ b/Rakefile @@ -12,13 +12,35 @@ task :generate_safelists do load "tasks/generate-safelists" end -task :rubocop => [:rubocop_security, :rubocop_frozen_string_literals] -task :rubocop_security do - sh "rubocop lib --only Security" + +require "rubocop/rake_task" + +module RubocopHelper + class << self + def common_options(task) + task.patterns += [ + "Gemfile", + "Rakefile", + "lib", + "loofah.gemspec", + "test", + ] + end + end end -task :rubocop_frozen_string_literals do - sh "rubocop lib --auto-correct --only Style/FrozenStringLiteralComment" + +RuboCop::RakeTask.new do |task| + RubocopHelper.common_options(task) +end + +desc("Generate the rubocop todo list") +RuboCop::RakeTask.new("rubocop:todo") do |task| + RubocopHelper.common_options(task) + task.options << "--auto-gen-config" + task.options << "--exclude-limit=50" end +Rake::Task["rubocop:todo:autocorrect"].clear +Rake::Task["rubocop:todo:autocorrect_all"].clear task :default => [:rubocop, :test] diff --git a/loofah.gemspec b/loofah.gemspec index 30bf6190..fbcba6d5 100644 --- a/loofah.gemspec +++ b/loofah.gemspec @@ -40,4 +40,8 @@ Gem::Specification.new do |spec| spec.add_development_dependency("rake", ["~> 13.0"]) spec.add_development_dependency("rdoc", [">= 4.0", "< 7"]) spec.add_development_dependency("rubocop", "~> 1.1") + spec.add_development_dependency("rubocop-minitest", "0.29.0") + spec.add_development_dependency("rubocop-performance", "1.16.0") + spec.add_development_dependency("rubocop-rake", "0.6.0") + spec.add_development_dependency("rubocop-shopify", "2.12.0") end From a8da9bf18efb4b9f094221ab3360c04010676a70 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sun, 2 Apr 2023 15:04:06 -0400 Subject: [PATCH 2/5] style(rubocop): safe autocorrections --- Gemfile | 2 + Rakefile | 8 +- lib/loofah.rb | 1 + lib/loofah/concerns.rb | 25 +- lib/loofah/elements.rb | 154 +- lib/loofah/helpers.rb | 13 +- lib/loofah/html4/document.rb | 1 + lib/loofah/html4/document_fragment.rb | 1 + lib/loofah/html5/document.rb | 1 + lib/loofah/html5/document_fragment.rb | 1 + lib/loofah/html5/libxml2_workarounds.rb | 13 +- lib/loofah/html5/safelist.rb | 1873 ++++++++++++----------- lib/loofah/html5/scrub.rb | 40 +- lib/loofah/metahelpers.rb | 6 +- lib/loofah/scrubber.rb | 16 +- lib/loofah/scrubbers.rb | 39 +- lib/loofah/version.rb | 1 + lib/loofah/xml/document.rb | 1 + lib/loofah/xml/document_fragment.rb | 3 +- loofah.gemspec | 14 +- test/helper.rb | 2 + test/html5/test_safelist_properties.rb | 2 + test/html5/test_sanitizer.rb | 146 +- test/html5/test_scrub_css.rb | 46 +- test/integration/test_ad_hoc.rb | 89 +- test/integration/test_helpers.rb | 25 +- test/integration/test_html.rb | 3 + test/integration/test_scrubbers.rb | 155 +- test/integration/test_xml.rb | 18 +- test/unit/test_api.rb | 36 + test/unit/test_encoding.rb | 5 + test/unit/test_helpers.rb | 4 +- test/unit/test_scrubber.rb | 79 +- test/unit/test_scrubbers.rb | 6 +- 34 files changed, 1538 insertions(+), 1291 deletions(-) diff --git a/Gemfile b/Gemfile index 01102ad5..dee842d5 100644 --- a/Gemfile +++ b/Gemfile @@ -1,2 +1,4 @@ +# frozen_string_literal: true + source "https://rubygems.org/" gemspec diff --git a/Rakefile b/Rakefile index 29656f5d..6bad5aa6 100644 --- a/Rakefile +++ b/Rakefile @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "hoe/markdown" Hoe::Markdown::Standalone.new("loofah").define_markdown_tasks @@ -12,7 +14,6 @@ task :generate_safelists do load "tasks/generate-safelists" end - require "rubocop/rake_task" module RubocopHelper @@ -42,9 +43,8 @@ end Rake::Task["rubocop:todo:autocorrect"].clear Rake::Task["rubocop:todo:autocorrect_all"].clear -task :default => [:rubocop, :test] +task default: [:rubocop, :test] task :debug_manifest do - spec = eval(File.read("loofah.gemspec")) - puts spec.files + puts Bundler.load_gemspec("loofah.gemspec").files end diff --git a/lib/loofah.rb b/lib/loofah.rb index ff4a545a..217dab75 100644 --- a/lib/loofah.rb +++ b/lib/loofah.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + $LOAD_PATH.unshift(File.expand_path(File.dirname(__FILE__))) unless $LOAD_PATH.include?(File.expand_path(File.dirname(__FILE__))) require "nokogiri" diff --git a/lib/loofah/concerns.rb b/lib/loofah/concerns.rb index 4e503a8b..b050309f 100644 --- a/lib/loofah/concerns.rb +++ b/lib/loofah/concerns.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Loofah # # Mixes +scrub!+ into Document, DocumentFragment, Node and NodeSet. @@ -39,7 +40,7 @@ def scrub!(scrubber) when Nokogiri::XML::Document scrubber.traverse(root) if root when Nokogiri::XML::DocumentFragment - children.scrub! scrubber + children.scrub!(scrubber) else scrubber.traverse(self) end @@ -54,11 +55,12 @@ def scrub!(scrubber) end end - def ScrubBehavior.resolve_scrubber(scrubber) # :nodoc: + def self.resolve_scrubber(scrubber) # :nodoc: scrubber = Scrubbers::MAP[scrubber].new if Scrubbers::MAP[scrubber] unless scrubber.is_a?(Loofah::Scrubber) raise Loofah::ScrubberNotFound, "not a Scrubber or a scrubber name: #{scrubber.inspect}" end + scrubber end end @@ -96,12 +98,12 @@ def text(options = {}) if options[:encode_special_chars] == false result # possibly dangerous if rendered in a browser else - encode_special_chars result + encode_special_chars(result) end end - alias :inner_text :text - alias :to_str :text + alias_method :inner_text, :text + alias_method :to_str, :text # # Returns a plain-text version of the markup contained by the fragment, with HTML entities @@ -114,15 +116,15 @@ def text(options = {}) # # => "\nTitle\n\nContent\nNext line\n" # def to_text(options = {}) - Loofah.remove_extraneous_whitespace self.dup.scrub!(:newline_block_elements).text(options) + Loofah.remove_extraneous_whitespace(dup.scrub!(:newline_block_elements).text(options)) end end module DocumentDecorator # :nodoc: def initialize(*args, &block) super - self.decorators(Nokogiri::XML::Node) << ScrubBehavior::Node - self.decorators(Nokogiri::XML::NodeSet) << ScrubBehavior::NodeSet + decorators(Nokogiri::XML::Node) << ScrubBehavior::Node + decorators(Nokogiri::XML::NodeSet) << ScrubBehavior::NodeSet end end @@ -172,9 +174,9 @@ def parse(tags, encoding = nil) end def document_klass - @document_klass ||= if (Loofah.html5_support? && self == Loofah::HTML5::DocumentFragment) + @document_klass ||= if Loofah.html5_support? && self == Loofah::HTML5::DocumentFragment Loofah::HTML5::Document - elsif (self == Loofah::HTML4::DocumentFragment) + elsif self == Loofah::HTML4::DocumentFragment Loofah::HTML4::Document else raise ArgumentError, "unexpected class: #{self}" @@ -190,11 +192,10 @@ def to_s serialize_root.children.to_s end - alias :serialize :to_s + alias_method :serialize, :to_s def serialize_root at_xpath("./body") || self end end end - diff --git a/lib/loofah/elements.rb b/lib/loofah/elements.rb index 5596e6f2..cf3600e7 100644 --- a/lib/loofah/elements.rb +++ b/lib/loofah/elements.rb @@ -1,88 +1,90 @@ # frozen_string_literal: true + require "set" module Loofah module Elements - STRICT_BLOCK_LEVEL_HTML4 = Set.new %w[ - address - blockquote - center - dir - div - dl - fieldset - form - h1 - h2 - h3 - h4 - h5 - h6 - hr - isindex - menu - noframes - noscript - ol - p - pre - table - ul - ] + STRICT_BLOCK_LEVEL_HTML4 = Set.new([ + "address", + "blockquote", + "center", + "dir", + "div", + "dl", + "fieldset", + "form", + "h1", + "h2", + "h3", + "h4", + "h5", + "h6", + "hr", + "isindex", + "menu", + "noframes", + "noscript", + "ol", + "p", + "pre", + "table", + "ul", + ]) # https://developer.mozilla.org/en-US/docs/Web/HTML/Block-level_elements - STRICT_BLOCK_LEVEL_HTML5 = Set.new %w[ - address - article - aside - blockquote - canvas - dd - div - dl - dt - fieldset - figcaption - figure - footer - form - h1 - h2 - h3 - h4 - h5 - h6 - header - hgroup - hr - li - main - nav - noscript - ol - output - p - pre - section - table - tfoot - ul - video - ] + STRICT_BLOCK_LEVEL_HTML5 = Set.new([ + "address", + "article", + "aside", + "blockquote", + "canvas", + "dd", + "div", + "dl", + "dt", + "fieldset", + "figcaption", + "figure", + "footer", + "form", + "h1", + "h2", + "h3", + "h4", + "h5", + "h6", + "header", + "hgroup", + "hr", + "li", + "main", + "nav", + "noscript", + "ol", + "output", + "p", + "pre", + "section", + "table", + "tfoot", + "ul", + "video", + ]) # The following elements may also be considered block-level # elements since they may contain block-level elements - LOOSE_BLOCK_LEVEL = Set.new %w[dd - dt - frameset - li - tbody - td - tfoot - th - thead - tr - ] + LOOSE_BLOCK_LEVEL = Set.new([ + "dd", + "dt", + "frameset", + "li", + "tbody", + "td", + "tfoot", + "th", + "thead", + "tr", + ]) # Elements that aren't block but should generate a newline in #to_text INLINE_LINE_BREAK = Set.new(["br"]) @@ -92,5 +94,5 @@ module Elements LINEBREAKERS = BLOCK_LEVEL + INLINE_LINE_BREAK end - ::Loofah::MetaHelpers.add_downcased_set_members_to_all_set_constants ::Loofah::Elements + ::Loofah::MetaHelpers.add_downcased_set_members_to_all_set_constants(::Loofah::Elements) end diff --git a/lib/loofah/helpers.rb b/lib/loofah/helpers.rb index 07ecadd0..9545d4f5 100644 --- a/lib/loofah/helpers.rb +++ b/lib/loofah/helpers.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Loofah module Helpers class << self @@ -29,7 +30,7 @@ def sanitize(string_or_io) # Loofah::Helpers.sanitize_css("display:block;background-image:url(http://www.ragingplatypus.com/i/cam-full.jpg)") # => "display: block;" # def sanitize_css(style_string) - ::Loofah::HTML5::Scrub.scrub_css style_string + ::Loofah::HTML5::Scrub.scrub_css(style_string) end # @@ -37,7 +38,7 @@ def sanitize_css(style_string) # TODO: remove this in a future major-point-release. # def remove_extraneous_whitespace(string) - Loofah.remove_extraneous_whitespace string + Loofah.remove_extraneous_whitespace(string) end end @@ -52,7 +53,7 @@ def safe_list_sanitizer end def white_list_sanitizer - warn "warning: white_list_sanitizer is deprecated, please use safe_list_sanitizer instead." + warn("warning: white_list_sanitizer is deprecated, please use safe_list_sanitizer instead.") safe_list_sanitizer end end @@ -70,7 +71,7 @@ def white_list_sanitizer # class FullSanitizer def sanitize(html, *args) - Loofah::Helpers.strip_tags html + Loofah::Helpers.strip_tags(html) end end @@ -87,11 +88,11 @@ def sanitize(html, *args) # class SafeListSanitizer def sanitize(html, *args) - Loofah::Helpers.sanitize html + Loofah::Helpers.sanitize(html) end def sanitize_css(style_string, *args) - Loofah::Helpers.sanitize_css style_string + Loofah::Helpers.sanitize_css(style_string) end end diff --git a/lib/loofah/html4/document.rb b/lib/loofah/html4/document.rb index 4c36e144..593380b2 100644 --- a/lib/loofah/html4/document.rb +++ b/lib/loofah/html4/document.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Loofah module HTML4 # :nodoc: # diff --git a/lib/loofah/html4/document_fragment.rb b/lib/loofah/html4/document_fragment.rb index b7b58561..988f4b9b 100644 --- a/lib/loofah/html4/document_fragment.rb +++ b/lib/loofah/html4/document_fragment.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Loofah module HTML4 # :nodoc: # diff --git a/lib/loofah/html5/document.rb b/lib/loofah/html5/document.rb index d055f522..03d9bc44 100644 --- a/lib/loofah/html5/document.rb +++ b/lib/loofah/html5/document.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Loofah module HTML5 # :nodoc: # diff --git a/lib/loofah/html5/document_fragment.rb b/lib/loofah/html5/document_fragment.rb index a6f1c014..79e18575 100644 --- a/lib/loofah/html5/document_fragment.rb +++ b/lib/loofah/html5/document_fragment.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Loofah module HTML5 # :nodoc: # diff --git a/lib/loofah/html5/libxml2_workarounds.rb b/lib/loofah/html5/libxml2_workarounds.rb index 71297605..cd95fd9e 100644 --- a/lib/loofah/html5/libxml2_workarounds.rb +++ b/lib/loofah/html5/libxml2_workarounds.rb @@ -1,5 +1,6 @@ # coding: utf-8 # frozen_string_literal: true + require "set" module Loofah @@ -16,12 +17,12 @@ module LibxmlWorkarounds # # see comments about CVE-2018-8048 within the tests for more information # - BROKEN_ESCAPING_ATTRIBUTES = Set.new %w[ - href - action - src - name - ] + BROKEN_ESCAPING_ATTRIBUTES = Set.new([ + "href", + "action", + "src", + "name", + ]) BROKEN_ESCAPING_ATTRIBUTES_QUALIFYING_TAG = { "name" => "a" } end end diff --git a/lib/loofah/html5/safelist.rb b/lib/loofah/html5/safelist.rb index ae194fe7..18a54501 100644 --- a/lib/loofah/html5/safelist.rb +++ b/lib/loofah/html5/safelist.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + require "set" module Loofah @@ -47,973 +48,973 @@ module HTML5 # :nodoc: # module SafeList ACCEPTABLE_ELEMENTS = Set.new([ - "a", - "abbr", - "acronym", - "address", - "area", - "article", - "aside", - "audio", - "b", - "bdi", - "bdo", - "big", - "blockquote", - "br", - "button", - "canvas", - "caption", - "center", - "cite", - "code", - "col", - "colgroup", - "command", - "datalist", - "dd", - "del", - "details", - "dfn", - "dir", - "div", - "dl", - "dt", - "em", - "fieldset", - "figcaption", - "figure", - "font", - "footer", - "form", - "h1", - "h2", - "h3", - "h4", - "h5", - "h6", - "header", - "hr", - "i", - "img", - "input", - "ins", - "kbd", - "label", - "legend", - "li", - "main", - "map", - "mark", - "menu", - "meter", - "nav", - "ol", - "optgroup", - "option", - "output", - "p", - "pre", - "q", - "s", - "samp", - "section", - "select", - "small", - "span", - "strike", - "strong", - "sub", - "summary", - "sup", - "table", - "tbody", - "td", - "textarea", - "tfoot", - "th", - "thead", - "time", - "tr", - "tt", - "u", - "ul", - "var", - "video", - "wbr", - ]) + "a", + "abbr", + "acronym", + "address", + "area", + "article", + "aside", + "audio", + "b", + "bdi", + "bdo", + "big", + "blockquote", + "br", + "button", + "canvas", + "caption", + "center", + "cite", + "code", + "col", + "colgroup", + "command", + "datalist", + "dd", + "del", + "details", + "dfn", + "dir", + "div", + "dl", + "dt", + "em", + "fieldset", + "figcaption", + "figure", + "font", + "footer", + "form", + "h1", + "h2", + "h3", + "h4", + "h5", + "h6", + "header", + "hr", + "i", + "img", + "input", + "ins", + "kbd", + "label", + "legend", + "li", + "main", + "map", + "mark", + "menu", + "meter", + "nav", + "ol", + "optgroup", + "option", + "output", + "p", + "pre", + "q", + "s", + "samp", + "section", + "select", + "small", + "span", + "strike", + "strong", + "sub", + "summary", + "sup", + "table", + "tbody", + "td", + "textarea", + "tfoot", + "th", + "thead", + "time", + "tr", + "tt", + "u", + "ul", + "var", + "video", + "wbr", + ]) MATHML_ELEMENTS = Set.new([ - "annotation", - "annotation-xml", - "maction", - "math", - "menclose", - "merror", - "mfenced", - "mfrac", - "mi", - "mmultiscripts", - "mn", - "mo", - "mover", - "mpadded", - "mphantom", - "mprescripts", - "mroot", - "mrow", - "ms", - "mspace", - "msqrt", - "mstyle", - "msub", - "msubsup", - "msup", - "mtable", - "mtd", - "mtext", - "mtr", - "munder", - "munderover", - "none", - "semantics", - ]) + "annotation", + "annotation-xml", + "maction", + "math", + "menclose", + "merror", + "mfenced", + "mfrac", + "mi", + "mmultiscripts", + "mn", + "mo", + "mover", + "mpadded", + "mphantom", + "mprescripts", + "mroot", + "mrow", + "ms", + "mspace", + "msqrt", + "mstyle", + "msub", + "msubsup", + "msup", + "mtable", + "mtd", + "mtext", + "mtr", + "munder", + "munderover", + "none", + "semantics", + ]) SVG_ELEMENTS = Set.new([ - "a", - "altGlyph", - "animate", - "animateColor", - "animateMotion", - "animateTransform", - "circle", - "clipPath", - "cursor", - "defs", - "desc", - "ellipse", - "feGaussianBlur", - "feImage", - "filter", - "font-face", - "font-face-name", - "font-face-src", - "foreignObject", - "g", - "glyph", - "hkern", - "line", - "linearGradient", - "marker", - "mask", - "metadata", - "missing-glyph", - "mpath", - "path", - "pattern", - "polygon", - "polyline", - "radialGradient", - "rect", - "set", - "stop", - "svg", - "switch", - "symbol", - "text", - "textPath", - "title", - "tref", - "tspan", - "use", - ]) + "a", + "altGlyph", + "animate", + "animateColor", + "animateMotion", + "animateTransform", + "circle", + "clipPath", + "cursor", + "defs", + "desc", + "ellipse", + "feGaussianBlur", + "feImage", + "filter", + "font-face", + "font-face-name", + "font-face-src", + "foreignObject", + "g", + "glyph", + "hkern", + "line", + "linearGradient", + "marker", + "mask", + "metadata", + "missing-glyph", + "mpath", + "path", + "pattern", + "polygon", + "polyline", + "radialGradient", + "rect", + "set", + "stop", + "svg", + "switch", + "symbol", + "text", + "textPath", + "title", + "tref", + "tspan", + "use", + ]) ACCEPTABLE_ATTRIBUTES = Set.new([ - "abbr", - "accept", - "accept-charset", - "accesskey", - "action", - "align", - "alt", - "axis", - "border", - "cellpadding", - "cellspacing", - "char", - "charoff", - "charset", - "checked", - "cite", - "class", - "clear", - "color", - "cols", - "colspan", - "compact", - "contenteditable", - "coords", - "datetime", - "dir", - "disabled", - "enctype", - "for", - "frame", - "headers", - "height", - "href", - "hreflang", - "hspace", - "id", - "ismap", - "label", - "lang", - "longdesc", - "loop", - "loopcount", - "loopend", - "loopstart", - "maxlength", - "media", - "method", - "multiple", - "name", - "nohref", - "noshade", - "nowrap", - "poster", - "preload", - "prompt", - "readonly", - "rel", - "rev", - "rows", - "rowspan", - "rules", - "scope", - "selected", - "shape", - "size", - "span", - "src", - "start", - "style", - "summary", - "tabindex", - "target", - "title", - "type", - "usemap", - "valign", - "value", - "vspace", - "width", - "xml:lang", - ]) + "abbr", + "accept", + "accept-charset", + "accesskey", + "action", + "align", + "alt", + "axis", + "border", + "cellpadding", + "cellspacing", + "char", + "charoff", + "charset", + "checked", + "cite", + "class", + "clear", + "color", + "cols", + "colspan", + "compact", + "contenteditable", + "coords", + "datetime", + "dir", + "disabled", + "enctype", + "for", + "frame", + "headers", + "height", + "href", + "hreflang", + "hspace", + "id", + "ismap", + "label", + "lang", + "longdesc", + "loop", + "loopcount", + "loopend", + "loopstart", + "maxlength", + "media", + "method", + "multiple", + "name", + "nohref", + "noshade", + "nowrap", + "poster", + "preload", + "prompt", + "readonly", + "rel", + "rev", + "rows", + "rowspan", + "rules", + "scope", + "selected", + "shape", + "size", + "span", + "src", + "start", + "style", + "summary", + "tabindex", + "target", + "title", + "type", + "usemap", + "valign", + "value", + "vspace", + "width", + "xml:lang", + ]) MATHML_ATTRIBUTES = Set.new([ - "actiontype", - "align", - "close", - "columnalign", - "columnlines", - "columnspacing", - "columnspan", - "depth", - "dir", - "display", - "displaystyle", - "encoding", - "equalcolumns", - "equalrows", - "fence", - "fontstyle", - "fontweight", - "frame", - "height", - "href", - "linethickness", - "lquote", - "lspace", - "mathbackground", - "mathcolor", - "mathsize", - "mathvariant", - "maxsize", - "minsize", - "notation", - "open", - "other", - "rowalign", - "rowlines", - "rowspacing", - "rowspan", - "rquote", - "rspace", - "scriptlevel", - "selection", - "separator", - "separators", - "stretchy", - "width", - "xlink:href", - "xlink:show", - "xlink:type", - "xmlns", - "xmlns:xlink", - ]) + "actiontype", + "align", + "close", + "columnalign", + "columnlines", + "columnspacing", + "columnspan", + "depth", + "dir", + "display", + "displaystyle", + "encoding", + "equalcolumns", + "equalrows", + "fence", + "fontstyle", + "fontweight", + "frame", + "height", + "href", + "linethickness", + "lquote", + "lspace", + "mathbackground", + "mathcolor", + "mathsize", + "mathvariant", + "maxsize", + "minsize", + "notation", + "open", + "other", + "rowalign", + "rowlines", + "rowspacing", + "rowspan", + "rquote", + "rspace", + "scriptlevel", + "selection", + "separator", + "separators", + "stretchy", + "width", + "xlink:href", + "xlink:show", + "xlink:type", + "xmlns", + "xmlns:xlink", + ]) SVG_ATTRIBUTES = Set.new([ - "accent-height", - "accumulate", - "additive", - "alphabetic", - "arabic-form", - "ascent", - "attributeName", - "attributeType", - "baseProfile", - "bbox", - "begin", - "calcMode", - "cap-height", - "class", - "clip-path", - "clip-rule", - "color", - "color-interpolation-filters", - "color-profile", - "color-rendering", - "content", - "cursor", - "cx", - "cy", - "d", - "descent", - "display", - "dur", - "dx", - "dy", - "end", - "fill", - "fill-opacity", - "fill-rule", - "filter", - "filterRes", - "filterUnits", - "font-family", - "font-size", - "font-stretch", - "font-style", - "font-variant", - "font-weight", - "fx", - "fy", - "g1", - "g2", - "glyph-name", - "gradientUnits", - "hanging", - "height", - "horiz-adv-x", - "horiz-origin-x", - "id", - "ideographic", - "k", - "keyPoints", - "keySplines", - "keyTimes", - "lang", - "marker", - "marker-end", - "marker-mid", - "marker-start", - "markerHeight", - "markerUnits", - "markerWidth", - "mask", - "maskContentUnits", - "maskUnits", - "mathematical", - "max", - "method", - "min", - "name", - "offset", - "opacity", - "orient", - "origin", - "overline-position", - "overline-thickness", - "panose-1", - "path", - "pathLength", - "patternContentUnits", - "patternTransform", - "patternUnits", - "points", - "preserveAspectRatio", - "primitiveUnits", - "r", - "refX", - "refY", - "repeatCount", - "repeatDur", - "requiredExtensions", - "requiredFeatures", - "restart", - "rotate", - "rx", - "ry", - "slope", - "spacing", - "startOffset", - "stdDeviation", - "stemh", - "stemv", - "stop-color", - "stop-opacity", - "strikethrough-position", - "strikethrough-thickness", - "stroke", - "stroke-dasharray", - "stroke-dashoffset", - "stroke-linecap", - "stroke-linejoin", - "stroke-miterlimit", - "stroke-opacity", - "stroke-width", - "systemLanguage", - "target", - "text-anchor", - "transform", - "type", - "u1", - "u2", - "underline-position", - "underline-thickness", - "unicode", - "unicode-range", - "units-per-em", - "version", - "viewBox", - "visibility", - "width", - "widths", - "x", - "x-height", - "x1", - "x2", - "xlink:actuate", - "xlink:arcrole", - "xlink:href", - "xlink:role", - "xlink:show", - "xlink:title", - "xlink:type", - "xml:base", - "xml:lang", - "xml:space", - "xmlns", - "xmlns:xlink", - "y", - "y1", - "y2", - "zoomAndPan", - ]) + "accent-height", + "accumulate", + "additive", + "alphabetic", + "arabic-form", + "ascent", + "attributeName", + "attributeType", + "baseProfile", + "bbox", + "begin", + "calcMode", + "cap-height", + "class", + "clip-path", + "clip-rule", + "color", + "color-interpolation-filters", + "color-profile", + "color-rendering", + "content", + "cursor", + "cx", + "cy", + "d", + "descent", + "display", + "dur", + "dx", + "dy", + "end", + "fill", + "fill-opacity", + "fill-rule", + "filter", + "filterRes", + "filterUnits", + "font-family", + "font-size", + "font-stretch", + "font-style", + "font-variant", + "font-weight", + "fx", + "fy", + "g1", + "g2", + "glyph-name", + "gradientUnits", + "hanging", + "height", + "horiz-adv-x", + "horiz-origin-x", + "id", + "ideographic", + "k", + "keyPoints", + "keySplines", + "keyTimes", + "lang", + "marker", + "marker-end", + "marker-mid", + "marker-start", + "markerHeight", + "markerUnits", + "markerWidth", + "mask", + "maskContentUnits", + "maskUnits", + "mathematical", + "max", + "method", + "min", + "name", + "offset", + "opacity", + "orient", + "origin", + "overline-position", + "overline-thickness", + "panose-1", + "path", + "pathLength", + "patternContentUnits", + "patternTransform", + "patternUnits", + "points", + "preserveAspectRatio", + "primitiveUnits", + "r", + "refX", + "refY", + "repeatCount", + "repeatDur", + "requiredExtensions", + "requiredFeatures", + "restart", + "rotate", + "rx", + "ry", + "slope", + "spacing", + "startOffset", + "stdDeviation", + "stemh", + "stemv", + "stop-color", + "stop-opacity", + "strikethrough-position", + "strikethrough-thickness", + "stroke", + "stroke-dasharray", + "stroke-dashoffset", + "stroke-linecap", + "stroke-linejoin", + "stroke-miterlimit", + "stroke-opacity", + "stroke-width", + "systemLanguage", + "target", + "text-anchor", + "transform", + "type", + "u1", + "u2", + "underline-position", + "underline-thickness", + "unicode", + "unicode-range", + "units-per-em", + "version", + "viewBox", + "visibility", + "width", + "widths", + "x", + "x-height", + "x1", + "x2", + "xlink:actuate", + "xlink:arcrole", + "xlink:href", + "xlink:role", + "xlink:show", + "xlink:title", + "xlink:type", + "xml:base", + "xml:lang", + "xml:space", + "xmlns", + "xmlns:xlink", + "y", + "y1", + "y2", + "zoomAndPan", + ]) ARIA_ATTRIBUTES = Set.new([ - "aria-activedescendant", - "aria-atomic", - "aria-autocomplete", - "aria-braillelabel", - "aria-brailleroledescription", - "aria-busy", - "aria-checked", - "aria-colcount", - "aria-colindex", - "aria-colindextext", - "aria-colspan", - "aria-controls", - "aria-current", - "aria-describedby", - "aria-description", - "aria-details", - "aria-disabled", - "aria-dropeffect", - "aria-errormessage", - "aria-expanded", - "aria-flowto", - "aria-grabbed", - "aria-haspopup", - "aria-hidden", - "aria-invalid", - "aria-keyshortcuts", - "aria-label", - "aria-labelledby", - "aria-level", - "aria-live", - "aria-multiline", - "aria-multiselectable", - "aria-orientation", - "aria-owns", - "aria-placeholder", - "aria-posinset", - "aria-pressed", - "aria-readonly", - "aria-relevant", - "aria-required", - "aria-roledescription", - "aria-rowcount", - "aria-rowindex", - "aria-rowindextext", - "aria-rowspan", - "aria-selected", - "aria-setsize", - "aria-sort", - "aria-valuemax", - "aria-valuemin", - "aria-valuenow", - "aria-valuetext", - "role", - ]) + "aria-activedescendant", + "aria-atomic", + "aria-autocomplete", + "aria-braillelabel", + "aria-brailleroledescription", + "aria-busy", + "aria-checked", + "aria-colcount", + "aria-colindex", + "aria-colindextext", + "aria-colspan", + "aria-controls", + "aria-current", + "aria-describedby", + "aria-description", + "aria-details", + "aria-disabled", + "aria-dropeffect", + "aria-errormessage", + "aria-expanded", + "aria-flowto", + "aria-grabbed", + "aria-haspopup", + "aria-hidden", + "aria-invalid", + "aria-keyshortcuts", + "aria-label", + "aria-labelledby", + "aria-level", + "aria-live", + "aria-multiline", + "aria-multiselectable", + "aria-orientation", + "aria-owns", + "aria-placeholder", + "aria-posinset", + "aria-pressed", + "aria-readonly", + "aria-relevant", + "aria-required", + "aria-roledescription", + "aria-rowcount", + "aria-rowindex", + "aria-rowindextext", + "aria-rowspan", + "aria-selected", + "aria-setsize", + "aria-sort", + "aria-valuemax", + "aria-valuemin", + "aria-valuenow", + "aria-valuetext", + "role", + ]) ATTR_VAL_IS_URI = Set.new([ - "action", - "cite", - "href", - "longdesc", - "poster", - "preload", - "src", - "xlink:href", - "xml:base", - ]) + "action", + "cite", + "href", + "longdesc", + "poster", + "preload", + "src", + "xlink:href", + "xml:base", + ]) SVG_ATTR_VAL_ALLOWS_REF = Set.new([ - "clip-path", - "color-profile", - "cursor", - "fill", - "filter", - "marker", - "marker-end", - "marker-mid", - "marker-start", - "mask", - "stroke", - ]) + "clip-path", + "color-profile", + "cursor", + "fill", + "filter", + "marker", + "marker-end", + "marker-mid", + "marker-start", + "mask", + "stroke", + ]) SVG_ALLOW_LOCAL_HREF = Set.new([ - "altGlyph", - "animate", - "animateColor", - "animateMotion", - "animateTransform", - "cursor", - "feImage", - "filter", - "linearGradient", - "pattern", - "radialGradient", - "set", - "textpath", - "tref", - "use", - ]) + "altGlyph", + "animate", + "animateColor", + "animateMotion", + "animateTransform", + "cursor", + "feImage", + "filter", + "linearGradient", + "pattern", + "radialGradient", + "set", + "textpath", + "tref", + "use", + ]) ACCEPTABLE_CSS_PROPERTIES = Set.new([ - "azimuth", - "align-content", - "align-items", - "align-self", - "aspect-ratio", - "background-color", - "border-bottom-color", - "border-collapse", - "border-color", - "border-left-color", - "border-right-color", - "border-top-color", - "clear", - "color", - "cursor", - "direction", - "display", - "elevation", - "flex", - "flex-basis", - "flex-direction", - "flex-flow", - "flex-grow", - "flex-shrink", - "flex-wrap", - "float", - "font", - "font-family", - "font-size", - "font-style", - "font-variant", - "font-weight", - "height", - "justify-content", - "letter-spacing", - "line-height", - "list-style", - "list-style-type", - "max-width", - "order", - "overflow", - "overflow-x", - "overflow-y", - "page-break-after", - "page-break-before", - "page-break-inside", - "pause", - "pause-after", - "pause-before", - "pitch", - "pitch-range", - "richness", - "speak", - "speak-header", - "speak-numeral", - "speak-punctuation", - "speech-rate", - "stress", - "text-align", - "text-decoration", - "text-indent", - "unicode-bidi", - "vertical-align", - "voice-family", - "volume", - "white-space", - "width", - ]) + "azimuth", + "align-content", + "align-items", + "align-self", + "aspect-ratio", + "background-color", + "border-bottom-color", + "border-collapse", + "border-color", + "border-left-color", + "border-right-color", + "border-top-color", + "clear", + "color", + "cursor", + "direction", + "display", + "elevation", + "flex", + "flex-basis", + "flex-direction", + "flex-flow", + "flex-grow", + "flex-shrink", + "flex-wrap", + "float", + "font", + "font-family", + "font-size", + "font-style", + "font-variant", + "font-weight", + "height", + "justify-content", + "letter-spacing", + "line-height", + "list-style", + "list-style-type", + "max-width", + "order", + "overflow", + "overflow-x", + "overflow-y", + "page-break-after", + "page-break-before", + "page-break-inside", + "pause", + "pause-after", + "pause-before", + "pitch", + "pitch-range", + "richness", + "speak", + "speak-header", + "speak-numeral", + "speak-punctuation", + "speech-rate", + "stress", + "text-align", + "text-decoration", + "text-indent", + "unicode-bidi", + "vertical-align", + "voice-family", + "volume", + "white-space", + "width", + ]) ACCEPTABLE_CSS_KEYWORDS = Set.new([ - "!important", - "auto", - "block", - "bold", - "both", - "bottom", - "center", - "collapse", - "dashed", - "dotted", - "double", - "groove", - "hidden", - "inherit", - "initial", - "inset", - "italic", - "left", - "medium", - "none", - "normal", - "nowrap", - "outset", - "pointer", - "revert", - "ridge", - "right", - "separate", - "solid", - "thick", - "thin", - "top", - "transparent", - "underline", - "unset", - ]) + "!important", + "auto", + "block", + "bold", + "both", + "bottom", + "center", + "collapse", + "dashed", + "dotted", + "double", + "groove", + "hidden", + "inherit", + "initial", + "inset", + "italic", + "left", + "medium", + "none", + "normal", + "nowrap", + "outset", + "pointer", + "revert", + "ridge", + "right", + "separate", + "solid", + "thick", + "thin", + "top", + "transparent", + "underline", + "unset", + ]) # https://www.w3.org/TR/css-color-3/#html4 ACCEPTABLE_CSS_COLORS = Set.new([ - "aqua", - "black", - "blue", - "fuchsia", - "gray", - "green", - "lime", - "maroon", - "navy", - "olive", - "purple", - "red", - "silver", - "teal", - "white", - "yellow", - ]) + "aqua", + "black", + "blue", + "fuchsia", + "gray", + "green", + "lime", + "maroon", + "navy", + "olive", + "purple", + "red", + "silver", + "teal", + "white", + "yellow", + ]) # https://www.w3.org/TR/css-color-3/#svg-color ACCEPTABLE_CSS_EXTENDED_COLORS = Set.new([ - "aliceblue", - "antiquewhite", - "aqua", - "aquamarine", - "azure", - "beige", - "bisque", - "black", - "blanchedalmond", - "blue", - "blueviolet", - "brown", - "burlywood", - "cadetblue", - "chartreuse", - "chocolate", - "coral", - "cornflowerblue", - "cornsilk", - "crimson", - "cyan", - "darkblue", - "darkcyan", - "darkgoldenrod", - "darkgray", - "darkgreen", - "darkgrey", - "darkkhaki", - "darkmagenta", - "darkolivegreen", - "darkorange", - "darkorchid", - "darkred", - "darksalmon", - "darkseagreen", - "darkslateblue", - "darkslategray", - "darkslategrey", - "darkturquoise", - "darkviolet", - "deeppink", - "deepskyblue", - "dimgray", - "dimgrey", - "dodgerblue", - "firebrick", - "floralwhite", - "forestgreen", - "fuchsia", - "gainsboro", - "ghostwhite", - "gold", - "goldenrod", - "gray", - "green", - "greenyellow", - "grey", - "honeydew", - "hotpink", - "indianred", - "indigo", - "ivory", - "khaki", - "lavender", - "lavenderblush", - "lawngreen", - "lemonchiffon", - "lightblue", - "lightcoral", - "lightcyan", - "lightgoldenrodyellow", - "lightgray", - "lightgreen", - "lightgrey", - "lightpink", - "lightsalmon", - "lightseagreen", - "lightskyblue", - "lightslategray", - "lightslategrey", - "lightsteelblue", - "lightyellow", - "lime", - "limegreen", - "linen", - "magenta", - "maroon", - "mediumaquamarine", - "mediumblue", - "mediumorchid", - "mediumpurple", - "mediumseagreen", - "mediumslateblue", - "mediumspringgreen", - "mediumturquoise", - "mediumvioletred", - "midnightblue", - "mintcream", - "mistyrose", - "moccasin", - "navajowhite", - "navy", - "oldlace", - "olive", - "olivedrab", - "orange", - "orangered", - "orchid", - "palegoldenrod", - "palegreen", - "paleturquoise", - "palevioletred", - "papayawhip", - "peachpuff", - "peru", - "pink", - "plum", - "powderblue", - "purple", - "red", - "rosybrown", - "royalblue", - "saddlebrown", - "salmon", - "sandybrown", - "seagreen", - "seashell", - "sienna", - "silver", - "skyblue", - "slateblue", - "slategray", - "slategrey", - "snow", - "springgreen", - "steelblue", - "tan", - "teal", - "thistle", - "tomato", - "turquoise", - "violet", - "wheat", - "white", - "whitesmoke", - "yellow", - "yellowgreen", - ]) + "aliceblue", + "antiquewhite", + "aqua", + "aquamarine", + "azure", + "beige", + "bisque", + "black", + "blanchedalmond", + "blue", + "blueviolet", + "brown", + "burlywood", + "cadetblue", + "chartreuse", + "chocolate", + "coral", + "cornflowerblue", + "cornsilk", + "crimson", + "cyan", + "darkblue", + "darkcyan", + "darkgoldenrod", + "darkgray", + "darkgreen", + "darkgrey", + "darkkhaki", + "darkmagenta", + "darkolivegreen", + "darkorange", + "darkorchid", + "darkred", + "darksalmon", + "darkseagreen", + "darkslateblue", + "darkslategray", + "darkslategrey", + "darkturquoise", + "darkviolet", + "deeppink", + "deepskyblue", + "dimgray", + "dimgrey", + "dodgerblue", + "firebrick", + "floralwhite", + "forestgreen", + "fuchsia", + "gainsboro", + "ghostwhite", + "gold", + "goldenrod", + "gray", + "green", + "greenyellow", + "grey", + "honeydew", + "hotpink", + "indianred", + "indigo", + "ivory", + "khaki", + "lavender", + "lavenderblush", + "lawngreen", + "lemonchiffon", + "lightblue", + "lightcoral", + "lightcyan", + "lightgoldenrodyellow", + "lightgray", + "lightgreen", + "lightgrey", + "lightpink", + "lightsalmon", + "lightseagreen", + "lightskyblue", + "lightslategray", + "lightslategrey", + "lightsteelblue", + "lightyellow", + "lime", + "limegreen", + "linen", + "magenta", + "maroon", + "mediumaquamarine", + "mediumblue", + "mediumorchid", + "mediumpurple", + "mediumseagreen", + "mediumslateblue", + "mediumspringgreen", + "mediumturquoise", + "mediumvioletred", + "midnightblue", + "mintcream", + "mistyrose", + "moccasin", + "navajowhite", + "navy", + "oldlace", + "olive", + "olivedrab", + "orange", + "orangered", + "orchid", + "palegoldenrod", + "palegreen", + "paleturquoise", + "palevioletred", + "papayawhip", + "peachpuff", + "peru", + "pink", + "plum", + "powderblue", + "purple", + "red", + "rosybrown", + "royalblue", + "saddlebrown", + "salmon", + "sandybrown", + "seagreen", + "seashell", + "sienna", + "silver", + "skyblue", + "slateblue", + "slategray", + "slategrey", + "snow", + "springgreen", + "steelblue", + "tan", + "teal", + "thistle", + "tomato", + "turquoise", + "violet", + "wheat", + "white", + "whitesmoke", + "yellow", + "yellowgreen", + ]) # see https://www.quackit.com/css/functions/ # omit `url` and `image` from that list ACCEPTABLE_CSS_FUNCTIONS = Set.new([ - "attr", - "blur", - "brightness", - "calc", - "circle", - "contrast", - "counter", - "counters", - "cubic-bezier", - "drop-shadow", - "ellipse", - "grayscale", - "hsl", - "hsla", - "hue-rotate", - "hwb", - "inset", - "invert", - "linear-gradient", - "matrix", - "matrix3d", - "opacity", - "perspective", - "polygon", - "radial-gradient", - "repeating-linear-gradient", - "repeating-radial-gradient", - "rgb", - "rgba", - "rotate", - "rotate3d", - "rotateX", - "rotateY", - "rotateZ", - "saturate", - "sepia", - "scale", - "scale3d", - "scaleX", - "scaleY", - "scaleZ", - "skew", - "skewX", - "skewY", - "symbols", - "translate", - "translate3d", - "translateX", - "translateY", - "translateZ", - ]) + "attr", + "blur", + "brightness", + "calc", + "circle", + "contrast", + "counter", + "counters", + "cubic-bezier", + "drop-shadow", + "ellipse", + "grayscale", + "hsl", + "hsla", + "hue-rotate", + "hwb", + "inset", + "invert", + "linear-gradient", + "matrix", + "matrix3d", + "opacity", + "perspective", + "polygon", + "radial-gradient", + "repeating-linear-gradient", + "repeating-radial-gradient", + "rgb", + "rgba", + "rotate", + "rotate3d", + "rotateX", + "rotateY", + "rotateZ", + "saturate", + "sepia", + "scale", + "scale3d", + "scaleX", + "scaleY", + "scaleZ", + "skew", + "skewX", + "skewY", + "symbols", + "translate", + "translate3d", + "translateX", + "translateY", + "translateZ", + ]) SHORTHAND_CSS_PROPERTIES = Set.new([ - "background", - "border", - "margin", - "padding", - ]) + "background", + "border", + "margin", + "padding", + ]) ACCEPTABLE_SVG_PROPERTIES = Set.new([ - "fill", - "fill-opacity", - "fill-rule", - "stroke", - "stroke-width", - "stroke-linecap", - "stroke-linejoin", - "stroke-opacity", - ]) + "fill", + "fill-opacity", + "fill-rule", + "stroke", + "stroke-width", + "stroke-linecap", + "stroke-linejoin", + "stroke-opacity", + ]) PROTOCOL_SEPARATOR = /:|(�*58)|(p)|(�*3a)|(%|%)3A/i ACCEPTABLE_PROTOCOLS = Set.new([ - "afs", - "aim", - "callto", - "data", - "ed2k", - "fax", - "feed", - "ftp", - "gopher", - "http", - "https", - "irc", - "line", - "mailto", - "modem", - "news", - "nntp", - "rsync", - "rtsp", - "sftp", - "sms", - "ssh", - "tag", - "tel", - "telnet", - "urn", - "webcal", - "xmpp", - ]) + "afs", + "aim", + "callto", + "data", + "ed2k", + "fax", + "feed", + "ftp", + "gopher", + "http", + "https", + "irc", + "line", + "mailto", + "modem", + "news", + "nntp", + "rsync", + "rtsp", + "sftp", + "sms", + "ssh", + "tag", + "tel", + "telnet", + "urn", + "webcal", + "xmpp", + ]) ACCEPTABLE_URI_DATA_MEDIATYPES = Set.new([ - "image/gif", - "image/jpeg", - "image/png", - "text/css", - "text/plain", - ]) + "image/gif", + "image/jpeg", + "image/png", + "text/css", + "text/plain", + ]) # subclasses may define their own versions of these constants ALLOWED_ELEMENTS = ACCEPTABLE_ELEMENTS + MATHML_ELEMENTS + SVG_ELEMENTS @@ -1028,19 +1029,19 @@ module SafeList # TODO: remove VOID_ELEMENTS in a future major release # and put it in the tests (it is used only for testing, not for functional behavior) VOID_ELEMENTS = Set.new([ - "area", - "br", - "hr", - "img", - "input", - ]) + "area", + "br", + "hr", + "img", + "input", + ]) # additional tags we should consider safe since we have libxml2 fixing up our documents. TAGS_SAFE_WITH_LIBXML2 = Set.new([ - "body", - "head", - "html", - ]) + "body", + "head", + "html", + ]) ALLOWED_ELEMENTS_WITH_LIBXML2 = ALLOWED_ELEMENTS + TAGS_SAFE_WITH_LIBXML2 end @@ -1049,6 +1050,6 @@ module SafeList deprecate_constant :WhiteList end - ::Loofah::MetaHelpers.add_downcased_set_members_to_all_set_constants ::Loofah::HTML5::SafeList + ::Loofah::MetaHelpers.add_downcased_set_members_to_all_set_constants(::Loofah::HTML5::SafeList) end end diff --git a/lib/loofah/html5/scrub.rb b/lib/loofah/html5/scrub.rb index 48079581..66f887dc 100644 --- a/lib/loofah/html5/scrub.rb +++ b/lib/loofah/html5/scrub.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + require "cgi" require "crass" @@ -8,7 +9,7 @@ module Scrub CONTROL_CHARACTERS = /[`\u0000-\u0020\u007f\u0080-\u0101]/ CSS_KEYWORDISH = /\A(#[0-9a-fA-F]+|rgb\(\d+%?,\d*%?,?\d*%?\)?|-?\d{0,3}\.?\d{0,10}(ch|cm|r?em|ex|in|lh|mm|pc|pt|px|Q|vmax|vmin|vw|vh|%|,|\))?)\z/ CRASS_SEMICOLON = { node: :semicolon, raw: ";" } - CSS_IMPORTANT = '!important' + CSS_IMPORTANT = "!important" CSS_PROPERTY_STRING_WITHOUT_EMBEDDED_QUOTES = /\A(["'])?[^"']+\1\z/ DATA_ATTRIBUTE_NAME = /\Adata-[\w-]+\z/ @@ -26,7 +27,7 @@ def scrub_attributes(node) attr_node.node_name end - if attr_name =~ DATA_ATTRIBUTE_NAME + if DATA_ATTRIBUTE_NAME.match?(attr_name) next end @@ -77,18 +78,16 @@ def scrub_css(style) name = node[:name].downcase next unless SafeList::ALLOWED_CSS_PROPERTIES.include?(name) || - SafeList::ALLOWED_SVG_PROPERTIES.include?(name) || - SafeList::SHORTHAND_CSS_PROPERTIES.include?(name.split("-").first) + SafeList::ALLOWED_SVG_PROPERTIES.include?(name) || + SafeList::SHORTHAND_CSS_PROPERTIES.include?(name.split("-").first) value = node[:children].map do |child| case child[:node] when :whitespace nil when :string - if child[:raw] =~ CSS_PROPERTY_STRING_WITHOUT_EMBEDDED_QUOTES + if CSS_PROPERTY_STRING_WITHOUT_EMBEDDED_QUOTES.match?(child[:raw]) Crass::Parser.stringify(child) - else - nil end when :function if SafeList::ALLOWED_CSS_FUNCTIONS.include?(child[:name].downcase) @@ -97,8 +96,8 @@ def scrub_css(style) when :ident keyword = child[:value] if !SafeList::SHORTHAND_CSS_PROPERTIES.include?(name.split("-").first) || - SafeList::ALLOWED_CSS_KEYWORDS.include?(keyword) || - (keyword =~ CSS_KEYWORDISH) + SafeList::ALLOWED_CSS_KEYWORDS.include?(keyword) || + (keyword =~ CSS_KEYWORDISH) keyword end else @@ -107,6 +106,7 @@ def scrub_css(style) end.compact next if value.empty? + value << CSS_IMPORTANT if node[:important] propstring = format("%s:%s", name, value.join(" ")) sanitized_node = Crass.parse_properties(propstring).first @@ -126,13 +126,9 @@ def scrub_attribute_that_allows_local_ref(attr_node) when :url if node[:value].start_with?("#") node[:raw] - else - nil end when :hash, :ident, :string node[:raw] - else - nil end end.compact @@ -198,28 +194,28 @@ def cdata_escape(node) end TABLE_FOR_ESCAPE_HTML__ = { - '<' => '<', - '>' => '>', - '&' => '&', + "<" => "<", + ">" => ">", + "&" => "&", } def escape_tags(string) # modified version of CGI.escapeHTML from ruby 3.1 enc = string.encoding - unless enc.ascii_compatible? + if enc.ascii_compatible? + string = string.b + string.gsub!(/[<>&]/, TABLE_FOR_ESCAPE_HTML__) + string.force_encoding(enc) + else if enc.dummy? origenc = enc enc = Encoding::Converter.asciicompat_encoding(enc) string = enc ? string.encode(enc) : string.b end - table = Hash[TABLE_FOR_ESCAPE_HTML__.map {|pair|pair.map {|s|s.encode(enc)}}] + table = Hash[TABLE_FOR_ESCAPE_HTML__.map { |pair| pair.map { |s| s.encode(enc) } }] string = string.gsub(/#{"[<>&]".encode(enc)}/, table) string.encode!(origenc) if origenc string - else - string = string.b - string.gsub!(/[<>&]/, TABLE_FOR_ESCAPE_HTML__) - string.force_encoding(enc) end end end diff --git a/lib/loofah/metahelpers.rb b/lib/loofah/metahelpers.rb index c31dced0..aecc21c9 100644 --- a/lib/loofah/metahelpers.rb +++ b/lib/loofah/metahelpers.rb @@ -1,12 +1,14 @@ # frozen_string_literal: true + module Loofah module MetaHelpers # :nodoc: def self.add_downcased_set_members_to_all_set_constants(mojule) mojule.constants.each do |constant_sym| - constant = mojule.const_get constant_sym + constant = mojule.const_get(constant_sym) next unless Set === constant + constant.dup.each do |member| - constant.add member.downcase + constant.add(member.downcase) end end end diff --git a/lib/loofah/scrubber.rb b/lib/loofah/scrubber.rb index 8ebde596..c06595a9 100644 --- a/lib/loofah/scrubber.rb +++ b/lib/loofah/scrubber.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Loofah # # A RuntimeError raised when Loofah could not find an appropriate scrubber. @@ -32,7 +33,6 @@ class ScrubberNotFound < RuntimeError; end # Scrubber::STOP to terminate the traversal of a subtree. # class Scrubber - # Top-down Scrubbers may return CONTINUE to indicate that the subtree should be traversed. CONTINUE = Object.new.freeze @@ -67,7 +67,9 @@ def initialize(options = {}, &block) unless [:top_down, :bottom_up].include?(direction) raise ArgumentError, "direction #{direction} must be one of :top_down or :bottom_up" end - @direction, @block = direction, block + + @direction = direction + @block = block end # @@ -84,7 +86,7 @@ def traverse(node) # +scrub+, which will be called for each document node. # def scrub(node) - raise ScrubberNotFound, "No scrub method has been defined on #{self.class.to_s}" + raise ScrubberNotFound, "No scrub method has been defined on #{self.class}" end # @@ -103,8 +105,8 @@ def append_attribute(node, attribute, value) def html5lib_sanitize(node) case node.type when Nokogiri::XML::Node::ELEMENT_NODE - if HTML5::Scrub.allowed_element? node.name - HTML5::Scrub.scrub_attributes node + if HTML5::Scrub.allowed_element?(node.name) + HTML5::Scrub.scrub_attributes(node) return Scrubber::CONTINUE end when Nokogiri::XML::Node::TEXT_NODE, Nokogiri::XML::Node::CDATA_SECTION_NODE @@ -120,8 +122,8 @@ def html5lib_sanitize(node) def traverse_conditionally_top_down(node) if block return if block.call(node) == STOP - else - return if scrub(node) == STOP + elsif scrub(node) == STOP + return end node.children.each { |j| traverse_conditionally_top_down(j) } end diff --git a/lib/loofah/scrubbers.rb b/lib/loofah/scrubbers.rb index 7e883820..85885612 100644 --- a/lib/loofah/scrubbers.rb +++ b/lib/loofah/scrubbers.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Loofah # # Loofah provides some built-in scrubbers for sanitizing with @@ -100,9 +101,10 @@ def initialize def scrub(node) return CONTINUE if html5lib_sanitize(node) == CONTINUE + node.before(node.children) node.remove - return STOP + STOP end end @@ -122,8 +124,9 @@ def initialize def scrub(node) return CONTINUE if html5lib_sanitize(node) == CONTINUE + node.remove - return STOP + STOP end end @@ -143,9 +146,10 @@ def initialize def scrub(node) return CONTINUE if html5lib_sanitize(node) == CONTINUE - node.add_next_sibling Nokogiri::XML::Text.new(node.to_s, node.document) + + node.add_next_sibling(Nokogiri::XML::Text.new(node.to_s, node.document)) node.remove - return STOP + STOP end end @@ -175,7 +179,7 @@ def initialize def scrub(node) case node.type when Nokogiri::XML::Node::ELEMENT_NODE - if HTML5::Scrub.allowed_element? node.name + if HTML5::Scrub.allowed_element?(node.name) node.attributes.each { |attr| node.remove_attribute(attr.first) } return CONTINUE if node.namespaces.empty? end @@ -203,8 +207,9 @@ def initialize def scrub(node) return CONTINUE unless (node.type == Nokogiri::XML::Node::ELEMENT_NODE) && (node.name == "a") + append_attribute(node, "rel", "nofollow") - return STOP + STOP end end @@ -224,8 +229,9 @@ def initialize def scrub(node) return CONTINUE unless (node.type == Nokogiri::XML::Node::ELEMENT_NODE) && (node.name == "a") + append_attribute(node, "rel", "noopener") - return STOP + STOP end end @@ -237,12 +243,13 @@ def initialize def scrub(node) return CONTINUE unless Loofah::Elements::LINEBREAKERS.include?(node.name) + replacement = if Loofah::Elements::INLINE_LINE_BREAK.include?(node.name) "\n" else "\n#{node.content}\n" end - node.add_next_sibling Nokogiri::XML::Text.new(replacement, node.document) + node.add_next_sibling(Nokogiri::XML::Text.new(replacement, node.document)) node.remove end end @@ -279,14 +286,14 @@ def scrub(node) # A hash that maps a symbol (like +:prune+) to the appropriate Scrubber (Loofah::Scrubbers::Prune). # MAP = { - :escape => Escape, - :prune => Prune, - :whitewash => Whitewash, - :strip => Strip, - :nofollow => NoFollow, - :noopener => NoOpener, - :newline_block_elements => NewlineBlockElements, - :unprintable => Unprintable, + escape: Escape, + prune: Prune, + whitewash: Whitewash, + strip: Strip, + nofollow: NoFollow, + noopener: NoOpener, + newline_block_elements: NewlineBlockElements, + unprintable: Unprintable, } # diff --git a/lib/loofah/version.rb b/lib/loofah/version.rb index 26f134a3..4e25947d 100644 --- a/lib/loofah/version.rb +++ b/lib/loofah/version.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Loofah # The version of Loofah you are using VERSION = "2.21.0.dev" diff --git a/lib/loofah/xml/document.rb b/lib/loofah/xml/document.rb index 9f00851f..cbdc8043 100644 --- a/lib/loofah/xml/document.rb +++ b/lib/loofah/xml/document.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Loofah module XML # :nodoc: # diff --git a/lib/loofah/xml/document_fragment.rb b/lib/loofah/xml/document_fragment.rb index 823f5178..aecf65af 100644 --- a/lib/loofah/xml/document_fragment.rb +++ b/lib/loofah/xml/document_fragment.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + module Loofah module XML # :nodoc: # @@ -11,7 +12,7 @@ class << self def parse(tags) doc = Loofah::XML::Document.new doc.encoding = tags.encoding.name if tags.respond_to?(:encoding) - self.new(doc, tags) + new(doc, tags) end end end diff --git a/loofah.gemspec b/loofah.gemspec index fbcba6d5..84a1a297 100644 --- a/loofah.gemspec +++ b/loofah.gemspec @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "./lib/loofah/version" Gem::Specification.new do |spec| @@ -22,12 +24,12 @@ Gem::Specification.new do |spec| } spec.require_paths = ["lib"] - spec.files = Dir.chdir(File.expand_path("..", __FILE__)) do - spec.files = %w[ - CHANGELOG.md - MIT-LICENSE.txt - README.md - SECURITY.md + Dir.chdir(File.expand_path("..", __FILE__)) do + spec.files = [ + "CHANGELOG.md", + "MIT-LICENSE.txt", + "README.md", + "SECURITY.md", ] + Dir.glob("lib/**/*.*") end diff --git a/test/helper.rb b/test/helper.rb index 00de7be9..4387861a 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "minitest/autorun" require "minitest/unit" require "minitest/spec" diff --git a/test/html5/test_safelist_properties.rb b/test/html5/test_safelist_properties.rb index 7221f462..1792cfc1 100644 --- a/test/html5/test_safelist_properties.rb +++ b/test/html5/test_safelist_properties.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "helper" class Html5TestSafelistProperties < Loofah::TestCase diff --git a/test/html5/test_sanitizer.rb b/test/html5/test_sanitizer.rb index c530fc4b..d3bcb4a8 100755 --- a/test/html5/test_sanitizer.rb +++ b/test/html5/test_sanitizer.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # # these tests taken from the HTML5 sanitization project and modified for use with Loofah # see the original here: http://code.google.com/p/html5lib/source/browse/ruby/test/test_sanitizer.rb @@ -20,18 +22,18 @@ def sanitize_html5(stream) # shotgun approach - if any of the possible answers match, we win def check_sanitization(input, *possible_answers) # libxml uses double-quotes, so let's swappo-boppo our quotes before comparing. - sane = sanitize_html4(input).gsub('"', "'") + sane = sanitize_html4(input).tr('"', "'") possible_output = possible_answers.compact.map do |possible_answer| - possible_answer.gsub('"', "'") + possible_answer.tr('"', "'") end assert_includes(possible_output, sane, caller(1..1).first) if Loofah.html5_support? # now do libgumbo - sane = sanitize_html5(input).gsub('"', "'") + sane = sanitize_html5(input).tr('"', "'") possible_output = possible_answers.compact.map do |possible_answer| - possible_answer.gsub('"', "'") + possible_answer.tr('"', "'") end assert_includes(possible_output, sane, caller(1..1).first) @@ -40,8 +42,9 @@ def check_sanitization(input, *possible_answers) def assert_completes_in_reasonable_time(&block) t0 = Time.now - block.call - assert_in_delta t0, Time.now, 0.1 # arbitrary seconds + yield + + assert_in_delta(t0, Time.now, 0.1) # arbitrary seconds end ALLOWED_ELEMENTS_PARENT = { @@ -56,7 +59,7 @@ def assert_completes_in_reasonable_time(&block) "thead" => "table", "tr" => "table", } - (HTML5::SafeList::ALLOWED_ELEMENTS).each do |tag_name| + HTML5::SafeList::ALLOWED_ELEMENTS.each do |tag_name| define_method "test_should_allow_#{tag_name}_tag" do parent = ALLOWED_ELEMENTS_PARENT[tag_name] if parent @@ -112,7 +115,7 @@ def assert_completes_in_reasonable_time(&block) if HTML5::SafeList::VOID_ELEMENTS.include?(tag_name) || tag_name == "wbr" outputs << "<#{tag_name} title='1'>foo" end - if !HTML5::SafeList::VOID_ELEMENTS.include?(tag_name) + unless HTML5::SafeList::VOID_ELEMENTS.include?(tag_name) outputs << naive_output end end @@ -140,9 +143,21 @@ def assert_completes_in_reasonable_time(&block) HTML5::SafeList::ALLOWED_ATTRIBUTES.each do |attribute_name| next if attribute_name == "style" + define_method "test_should_allow_#{attribute_name}_attribute" do input = "

foo bar baz

" - if %w[checked compact disabled ismap multiple nohref noshade nowrap readonly selected].include?(attribute_name) + if [ + "checked", + "compact", + "disabled", + "ismap", + "multiple", + "nohref", + "noshade", + "nowrap", + "readonly", + "selected", + ].include?(attribute_name) htmloutput = "

foo <bad>bar</bad> baz

" html5output = "

foo <bad>bar</bad> baz

" else @@ -295,7 +310,8 @@ def test_should_disallow_other_uri_mediatypes def test_figure_element_is_valid fragment = Loofah.scrub_html4_fragment("hello
asd
", :prune) - assert fragment.at_css("figure"), "
tag was scrubbed" + + assert(fragment.at_css("figure"), "
tag was scrubbed") end ## @@ -326,7 +342,7 @@ def test_figure_element_is_valid ## require "json" Dir[File.join(File.dirname(__FILE__), "..", "assets", "testdata_sanitizer_tests1.dat")].each do |filename| - JSON::parse(open(filename).read).each do |test| + JSON.parse(open(filename).read).each do |test| it "testdata sanitizer #{test["name"]}" do test.delete("name") input = test.delete("input") @@ -358,206 +374,241 @@ def test_figure_element_is_valid def test_css_list_style html = '
    ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/list-style/, sane.inner_html + + assert_match(/list-style/, sane.inner_html) end def test_css_negative_value_sanitization html = "" sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/-0.03em/, sane.inner_html + + assert_match(/-0.03em/, sane.inner_html) end def test_css_negative_value_sanitization_shorthand_css_properties html = "" sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/-0.05em/, sane.inner_html + + assert_match(/-0.05em/, sane.inner_html) end def test_css_high_precision_value_shorthand_css_properties html = "" sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/0.3333333334em/, sane.inner_html + + assert_match(/0.3333333334em/, sane.inner_html) end def test_css_rem_value html = "" sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/10rem/, sane.inner_html + + assert_match(/10rem/, sane.inner_html) end def test_css_ch_value html = "
    " sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/60ch/, sane.inner_html + + assert_match(/60ch/, sane.inner_html) end def test_css_vw_value html = "
    " sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/1vw/, sane.inner_html + + assert_match(/1vw/, sane.inner_html) end def test_css_vh_value html = "
    " sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/100vh/, sane.inner_html + + assert_match(/100vh/, sane.inner_html) end def test_css_Q_value html = "
    " sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/10Q/, sane.inner_html + + assert_match(/10Q/, sane.inner_html) end def test_css_lh_value html = "

    " sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/2lh/, sane.inner_html + + assert_match(/2lh/, sane.inner_html) end def test_css_vmin_value html = "

    " sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/42vmin/, sane.inner_html + + assert_match(/42vmin/, sane.inner_html) end def test_css_vmax_value html = "
    " sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/42vmax/, sane.inner_html + + assert_match(/42vmax/, sane.inner_html) end def test_css_function_sanitization_leaves_safelisted_functions_calc html = "" sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :strip).to_html) - assert_match %r/calc\(5%\)/, sane.inner_html + + assert_match(/calc\(5%\)/, sane.inner_html) html = "" sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :strip).to_html) - assert_match %r/calc\(5%\)/, sane.inner_html + + assert_match(/calc\(5%\)/, sane.inner_html) end def test_css_function_sanitization_leaves_safelisted_functions_rgb html = '' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :strip).to_html) - assert_match %r/rgb\(255, 0, 0\)/, sane.inner_html + + assert_match(/rgb\(255, 0, 0\)/, sane.inner_html) end def test_css_function_sanitization_leaves_safelisted_list_style_type html = "
      " sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :strip).to_html) - assert_match %r/list-style-type:lower-greek/, sane.inner_html + + assert_match(/list-style-type:lower-greek/, sane.inner_html) end def test_css_function_sanitization_strips_style_attributes_with_unsafe_functions html = "" sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :strip).to_html) - assert_match %r/<\/span>/, sane.inner_html + + assert_match(%r/<\/span>/, sane.inner_html) html = "" sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :strip).to_html) - assert_match %r/<\/span>/, sane.inner_html + + assert_match(%r/<\/span>/, sane.inner_html) end def test_css_max_width html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/max-width/, sane.inner_html + + assert_match(/max-width/, sane.inner_html) end def test_css_page_break_after html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/page-break-after:always/, sane.inner_html + + assert_match(/page-break-after:always/, sane.inner_html) end def test_css_page_break_before html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/page-break-before:always/, sane.inner_html + + assert_match(/page-break-before:always/, sane.inner_html) end def test_css_page_break_inside html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/page-break-inside:auto/, sane.inner_html + + assert_match(/page-break-inside:auto/, sane.inner_html) end def test_css_align_content html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/align-content:flex-start/, sane.inner_html + + assert_match(/align-content:flex-start/, sane.inner_html) end def test_css_align_items html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/align-items:stretch/, sane.inner_html + + assert_match(/align-items:stretch/, sane.inner_html) end def test_css_align_self html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/align-self:auto/, sane.inner_html + + assert_match(/align-self:auto/, sane.inner_html) end def test_css_flex html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/flex:none/, sane.inner_html + + assert_match(/flex:none/, sane.inner_html) end def test_css_flex_basis html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/flex-basis:auto/, sane.inner_html + + assert_match(/flex-basis:auto/, sane.inner_html) end def test_css_flex_direction html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/flex-direction:row/, sane.inner_html + + assert_match(/flex-direction:row/, sane.inner_html) end def test_css_flex_flow html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/flex-flow:column wrap/, sane.inner_html + + assert_match(/flex-flow:column wrap/, sane.inner_html) end def test_css_flex_grow html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/flex-grow:4/, sane.inner_html + + assert_match(/flex-grow:4/, sane.inner_html) end def test_css_flex_shrink html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/flex-shrink:3/, sane.inner_html + + assert_match(/flex-shrink:3/, sane.inner_html) end def test_css_flex_wrap html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/flex-wrap:wrap/, sane.inner_html + + assert_match(/flex-wrap:wrap/, sane.inner_html) end def test_css_justify_content html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/justify-content:flex-start/, sane.inner_html + + assert_match(/justify-content:flex-start/, sane.inner_html) end def test_css_order html = '
      ' sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) - assert_match %r/order:5/, sane.inner_html + + assert_match(/order:5/, sane.inner_html) end def test_upper_case_css_property html = "
      asdf
      " sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :strip).to_xml) + assert_match(/COLOR:\s*BLUE/i, sane.at_css("div")["style"]) refute_match(/NOTAPROPERTY/i, sane.at_css("div")["style"]) end @@ -565,18 +616,21 @@ def test_upper_case_css_property def test_many_properties_some_allowed html = "
      asdf
      " sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :strip).to_xml) + assert_match(/bold\s+center\s+10px/, sane.at_css("div")["style"]) end def test_many_properties_non_allowed html = "
      asdf
      " sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :strip).to_xml) - assert_nil sane.at_css("div")["style"] + + assert_nil(sane.at_css("div")["style"]) end def test_svg_properties html = "" sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :strip).to_xml) + assert_match(/stroke-width:\s*10px/, sane.at_css("line")["style"]) end end diff --git a/test/html5/test_scrub_css.rb b/test/html5/test_scrub_css.rb index 49291432..dea6804d 100644 --- a/test/html5/test_scrub_css.rb +++ b/test/html5/test_scrub_css.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + require "helper" class UnitHTML5Scrub < Loofah::TestCase @@ -15,35 +16,46 @@ class UnitHTML5Scrub < Loofah::TestCase describe "css functions" do it "allows safe functions" do - assert_equal "background-color:linear-gradient(transparent 50%, #ffff66 50%);", - Loofah::HTML5::Scrub.scrub_css("background-color: linear-gradient(transparent 50%, #ffff66 50%);") + assert_equal( + "background-color:linear-gradient(transparent 50%, #ffff66 50%);", + Loofah::HTML5::Scrub.scrub_css("background-color: linear-gradient(transparent 50%, #ffff66 50%);"), + ) end it "disallows unsafe functions" do - assert_equal "", Loofah::HTML5::Scrub.scrub_css("background-color: haxxor-fun(transparent 50%, #ffff66 50%);") + assert_equal( + "", + Loofah::HTML5::Scrub.scrub_css("background-color: haxxor-fun(transparent 50%, #ffff66 50%);"), + ) end # see #199 for the bug we're testing here it "allows safe functions in shorthand css properties" do - assert_equal "background:linear-gradient(transparent 50%, #ffff66 50%);", - Loofah::HTML5::Scrub.scrub_css("background: linear-gradient(transparent 50%, #ffff66 50%);") + assert_equal( + "background:linear-gradient(transparent 50%, #ffff66 50%);", + Loofah::HTML5::Scrub.scrub_css("background: linear-gradient(transparent 50%, #ffff66 50%);"), + ) end end describe "property string values" do it "allows hypenated values" do - text = %q(font-family:'AvenirNext-Regular';) + text = "font-family:'AvenirNext-Regular';" + assert_equal(text, Loofah::HTML5::Scrub.scrub_css(text)) - text = %q(font-family:"AvenirNext-Regular";) + text = 'font-family:"AvenirNext-Regular";' + assert_equal(text, Loofah::HTML5::Scrub.scrub_css(text)) end it "allows embedded spaces in values" do - text = %q(font-family:'Avenir Next';) + text = "font-family:'Avenir Next';" + assert_equal(text, Loofah::HTML5::Scrub.scrub_css(text)) - text = %q(font-family:"Avenir Next";) + text = 'font-family:"Avenir Next";' + assert_equal(text, Loofah::HTML5::Scrub.scrub_css(text)) end @@ -51,28 +63,32 @@ class UnitHTML5Scrub < Loofah::TestCase assert_empty(Loofah::HTML5::Scrub.scrub_css(%q(font-family:'AvenirNext"-Regular';))) assert_empty(Loofah::HTML5::Scrub.scrub_css(%q(font-family:"AvenirNext'-Regular";))) - assert_empty(Loofah::HTML5::Scrub.scrub_css(%q(font-family:'AvenirNext-Regular;))) + assert_empty(Loofah::HTML5::Scrub.scrub_css("font-family:'AvenirNext-Regular;")) assert_empty(Loofah::HTML5::Scrub.scrub_css(%q(font-family:'AvenirNext-Regular";))) - assert_empty(Loofah::HTML5::Scrub.scrub_css(%q(font-family:"AvenirNext-Regular;))) + assert_empty(Loofah::HTML5::Scrub.scrub_css('font-family:"AvenirNext-Regular;')) assert_empty(Loofah::HTML5::Scrub.scrub_css(%q(font-family:"AvenirNext-Regular';))) end end describe "colors" do it "allows basic and extended colors" do - text = %q(background-color:blue;) + text = "background-color:blue;" + assert_equal(text, Loofah::HTML5::Scrub.scrub_css(text)) - text = %q(background-color:brown;) + text = "background-color:brown;" + assert_equal(text, Loofah::HTML5::Scrub.scrub_css(text)) - text = %q(background-color:lightblue;) + text = "background-color:lightblue;" + assert_equal(text, Loofah::HTML5::Scrub.scrub_css(text)) end it "does not allow non-colors" do - text = %q(background-color:blurple;) + text = "background-color:blurple;" + assert_empty(Loofah::HTML5::Scrub.scrub_css(text)) end end diff --git a/test/integration/test_ad_hoc.rb b/test/integration/test_ad_hoc.rb index 14a128d0..811fedd3 100644 --- a/test/integration/test_ad_hoc.rb +++ b/test/integration/test_ad_hoc.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "helper" class IntegrationTestAdHoc < Loofah::TestCase @@ -54,16 +56,18 @@ def test_removal_of_illegal_tag was there? HTML sane = Nokogiri::HTML(scrub_fragment(html, :escape).to_xml) - assert sane.xpath("//jim").empty? + + assert_empty(sane.xpath("//jim")) end def test_removal_of_illegal_attribute html = "

      " sane = Nokogiri::HTML(scrub_fragment(html, :escape).to_xml) node = sane.xpath("//p").first - assert node.attributes["class"] - assert node.attributes["abbr"] - assert_nil node.attributes["foo"] + + assert(node.attributes["class"]) + assert(node.attributes["abbr"]) + assert_nil(node.attributes["foo"]) end def test_removal_of_illegal_url_in_href @@ -73,31 +77,39 @@ def test_removal_of_illegal_url_in_href HTML sane = Nokogiri::HTML(scrub_fragment(html, :escape).to_xml) nodes = sane.xpath("//a") - assert_nil nodes.first.attributes["href"] - assert nodes.last.attributes["href"] + + assert_nil(nodes.first.attributes["href"]) + assert(nodes.last.attributes["href"]) end def test_css_sanitization html = "

      " sane = Nokogiri::HTML(scrub_fragment(html, :escape).to_xml) - assert_match %r/#000/, sane.inner_html - refute_match %r/foo\.com/, sane.inner_html + + assert_match(/#000/, sane.inner_html) + refute_match(/foo\.com/, sane.inner_html) end def test_fragment_with_no_tags - assert_equal "This fragment has no tags.", scrub_fragment("This fragment has no tags.", :escape).to_xml + assert_equal("This fragment has no tags.", scrub_fragment("This fragment has no tags.", :escape).to_xml) end def test_fragment_in_p_tag - assert_equal "

      This fragment is in a p.

      ", scrub_fragment("

      This fragment is in a p.

      ", :escape).to_xml + assert_equal( + "

      This fragment is in a p.

      ", + scrub_fragment("

      This fragment is in a p.

      ", :escape).to_xml, + ) end def test_fragment_in_p_tag_plus_stuff - assert_equal "

      This fragment is in a p.

      foobar", scrub_fragment("

      This fragment is in a p.

      foobar", :escape).to_xml + assert_equal( + "

      This fragment is in a p.

      foobar", + scrub_fragment("

      This fragment is in a p.

      foobar", :escape).to_xml, + ) end def test_fragment_with_text_nodes_leading_and_trailing - assert_equal "text

      fragment

      text", scrub_fragment("text

      fragment

      text", :escape).to_xml + assert_equal("text

      fragment

      text", scrub_fragment("text

      fragment

      text", :escape).to_xml) end def test_whitewash_on_document @@ -105,7 +117,7 @@ def test_whitewash_on_document whitewashed = scrub_document(html, :whitewash).xpath("/html/body") expected = "safe description" - assert_equal(expected, whitewashed.to_s.gsub("\n", "")) + assert_equal(expected, whitewashed.to_s.delete("\n")) end def test_whitewash_on_fragment @@ -113,7 +125,7 @@ def test_whitewash_on_fragment whitewashed = scrub_fragment(html, :whitewash) expected = "safe description" - assert_equal(expected, whitewashed.to_s.gsub("\n", "")) + assert_equal(expected, whitewashed.to_s.delete("\n")) end def test_fragment_whitewash_on_microsofty_markup @@ -125,6 +137,7 @@ def test_fragment_whitewash_on_microsofty_markup else "

      Foo BOLD

      " end + assert_equal(expected, whitewashed.to_s.strip) end @@ -137,18 +150,19 @@ def test_document_whitewash_on_microsofty_markup else "

      Foo BOLD

      " end + assert_equal(expected, whitewashed.xpath("/html/body/*").to_s) end def test_return_empty_string_when_nothing_left - assert_equal "", scrub_document("", :prune).text + assert_equal("", scrub_document("", :prune).text) end def test_nested_script_cdata_tags_should_be_scrubbed html = "" stripped = fragment(html).scrub!(:strip) - assert_empty stripped.xpath("//script") + assert_empty(stripped.xpath("//script")) assert_equal("<script src=\"malicious.js\">this & that", stripped.to_html) end @@ -156,14 +170,14 @@ def test_nested_script_cdata_tags_should_be_scrubbed_2 html = "" stripped = fragment(html).scrub!(:strip) - assert_empty stripped.xpath("//script") + assert_empty(stripped.xpath("//script")) assert_equal("<script>alert('a');", stripped.to_html) end def test_nested_script_cdata_tags_should_be_scrubbed_max_recursion n = 40 html = "
      " + ("" * n) + "
      " - expected = "
      " + ("<script>" * (n-1)) + "alert(1);
      " + expected = "
      " + ("<script>" * (n - 1)) + "alert(1);
      " actual = fragment(html).scrub!(:strip).to_html assert_equal(expected, actual) @@ -174,17 +188,20 @@ def test_removal_of_all_tags What's up doc? HTML stripped = scrub_document(html, :prune).text - assert_equal %Q(What\'s up doc?).strip, stripped.strip + + assert_equal(%(What's up doc?).strip, stripped.strip) end def test_dont_remove_whitespace html = "Foo\nBar" - assert_equal html, scrub_document(html, :prune).text + + assert_equal(html, scrub_document(html, :prune).text) end def test_dont_remove_whitespace_between_tags html = "

      Foo

      \n

      Bar

      " - assert_equal "Foo\nBar", scrub_document(html, :prune).text + + assert_equal("Foo\nBar", scrub_document(html, :prune).text) end # @@ -261,6 +278,7 @@ def test_dont_remove_whitespace_between_tags sanitized = scrub_fragment(html, :escape) attributes = sanitized.at_css("div").attributes + assert_includes(attributes.keys, "role") assert_includes(attributes.keys, "aria-label") assert_includes(attributes.keys, "aria-description") @@ -275,9 +293,10 @@ def test_dont_remove_whitespace_between_tags # see: # - https://github.com/flavorjones/loofah/issues/154 # - https://hackerone.com/reports/429267 - html = %Q{} + html = %{} sanitized = scrub_fragment(html, :escape) + assert_nil sanitized.at_css("animate")["from"] assert_nil sanitized.at_css("animate")["to"] assert_nil sanitized.at_css("animate")["by"] @@ -288,9 +307,10 @@ def test_dont_remove_whitespace_between_tags # see: # - https://github.com/flavorjones/loofah/issues/171 # - https://hackerone.com/reports/709009 - html = %Q{ } + html = %{ } sanitized = scrub_fragment(html, :escape) + assert_nil sanitized.at_css("animate")["values"] end end @@ -304,12 +324,14 @@ def test_dont_remove_whitespace_between_tags it "document removes the comment" do sanitized = document(html) - refute(sanitized.children.any? { |node| node.comment? } ) + + refute(sanitized.children.any? { |node| node.comment? }) end it "scrub_document removes the comment" do sanitized = scrub_document(html, :prune) sanitized_html = sanitized.to_html + refute_match(/--/, sanitized_html) end end @@ -328,12 +350,14 @@ def test_dont_remove_whitespace_between_tags it "document removes the comment" do sanitized = document(html) sanitized_html = sanitized.to_html + refute_match(/--/, sanitized_html) end it "scrub_document removes the comment" do sanitized = scrub_document(html, :prune) sanitized_html = sanitized.to_html + refute_match(/--/, sanitized_html) end end @@ -373,13 +397,13 @@ def test_dont_remove_whitespace_between_tags HTML expected = <<~HTML -
      -
      +
      +
      -
      -
      -
      -
      +
      +
      +
      +
      HTML actual = scrub_fragment(input, :escape) @@ -392,6 +416,7 @@ def test_dont_remove_whitespace_between_tags input = %{
      this & that
      } expected = %{
      this & that
      } actual = scrub_fragment(input, :escape) + assert_equal(expected, actual.to_html) end @@ -399,6 +424,7 @@ def test_dont_remove_whitespace_between_tags input = %{
      this > that
      } expected = %{
      this > that
      } actual = scrub_fragment(input, :escape) + assert_equal(expected, actual.to_html) end @@ -406,6 +432,7 @@ def test_dont_remove_whitespace_between_tags input = %{
      this < that
      } expected = %{
      this < that
      } actual = scrub_fragment(input, :escape) + assert_equal(expected, actual.to_html) end @@ -425,6 +452,7 @@ def test_dont_remove_whitespace_between_tags input = %{
      this <<
      } expected = %{
      this <<
      } actual = scrub_fragment(input, :escape) + assert_equal(expected, actual.to_html) end @@ -432,6 +460,7 @@ def test_dont_remove_whitespace_between_tags input = %{
      this < that
      } expected = input actual = scrub_fragment(input, :escape) + assert_equal(expected, actual.to_html) end end diff --git a/test/integration/test_helpers.rb b/test/integration/test_helpers.rb index ed52c82c..7fd0c9c7 100644 --- a/test/integration/test_helpers.rb +++ b/test/integration/test_helpers.rb @@ -1,17 +1,20 @@ +# frozen_string_literal: true + require "helper" class IntegrationTestHelpers < Loofah::TestCase context ".strip_tags" do context "on safe markup" do it "strip out tags" do - assert_equal "omgwtfbbq!!1!", Loofah::Helpers.strip_tags("
      omgwtfbbq
      !!1!") + assert_equal("omgwtfbbq!!1!", Loofah::Helpers.strip_tags("
      omgwtfbbq
      !!1!")) end end context "on hack attack" do it "strip escape html entities" do bad_shit = "<script>alert('evil')</script>" - assert_equal bad_shit, Loofah::Helpers.strip_tags(bad_shit) + + assert_equal(bad_shit, Loofah::Helpers.strip_tags(bad_shit)) end end end @@ -20,24 +23,34 @@ class IntegrationTestHelpers < Loofah::TestCase context "on safe markup" do it "render the safe html" do html = "
      omgwtfbbq
      !!1!" - assert_equal html, Loofah::Helpers.sanitize(html) + + assert_equal(html, Loofah::Helpers.sanitize(html)) end end context "on hack attack" do it "strip the unsafe tags" do - assert_equal "alert('evil')w00t", Loofah::Helpers.sanitize("w00t") + assert_equal( + "alert('evil')w00t", + Loofah::Helpers.sanitize("w00t"), + ) end it "strips form tags" do - assert_equal "alert('evil')w00t", Loofah::Helpers.sanitize("
      w00t") + assert_equal( + "alert('evil')w00t", + Loofah::Helpers.sanitize("
      w00t"), + ) end end end context ".sanitize_css" do it "removes unsafe css properties" do - assert_match(/display:\s*block;\s*background-color:\s*blue;/, Loofah::Helpers.sanitize_css("display:block;background-image:url(http://www.ragingplatypus.com/i/cam-full.jpg);background-color:blue")) + assert_match( + /display:\s*block;\s*background-color:\s*blue;/, + Loofah::Helpers.sanitize_css("display:block;background-image:url(http://www.ragingplatypus.com/i/cam-full.jpg);background-color:blue"), + ) end end end diff --git a/test/integration/test_html.rb b/test/integration/test_html.rb index 1e52af4c..899d610b 100644 --- a/test/integration/test_html.rb +++ b/test/integration/test_html.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "helper" class IntegrationTestHtml < Loofah::TestCase @@ -133,6 +135,7 @@ class IntegrationTestHtml < Loofah::TestCase "#{html_version}_document", "
      tweedle\n\n\t\n\s\nbeetle
      ", ) + assert_equal "\ntweedle\n\nbeetle\n", html.to_text end diff --git a/test/integration/test_scrubbers.rb b/test/integration/test_scrubbers.rb index a10a5e1d..93d7f063 100644 --- a/test/integration/test_scrubbers.rb +++ b/test/integration/test_scrubbers.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "helper" class IntegrationTestScrubbers < Loofah::TestCase @@ -27,7 +29,7 @@ class IntegrationTestScrubbers < Loofah::TestCase UNPRINTABLE_RESULT = "Loofah rocks!" ENTITY_FRAGMENT = "

      this is < that "&" the other > boo'ya

      w00t
      " - ENTITY_TEXT = %Q(this is < that "&" the other > boo\'yaw00t) + ENTITY_TEXT = %(this is < that "&" the other > boo'yaw00t) ENTITY_HACK_ATTACK = "
      Hack attack!
      <script>alert('evil')</script>
      " ENTITY_HACK_ATTACK_TEXT_SCRUB = "Hack attack!<script>alert('evil')</script>" @@ -124,8 +126,8 @@ def html5? context "#scrub!" do context ":escape" do it "escape bad tags" do - doc = klass.parse "#{INVALID_FRAGMENT}" - result = doc.scrub! :escape + doc = klass.parse("#{INVALID_FRAGMENT}") + result = doc.scrub!(:escape) assert_equal INVALID_ESCAPED, doc.xpath("/html/body").inner_html assert_equal doc, result @@ -134,8 +136,8 @@ def html5? context ":prune" do it "prune bad tags" do - doc = klass.parse "#{INVALID_FRAGMENT}" - result = doc.scrub! :prune + doc = klass.parse("#{INVALID_FRAGMENT}") + result = doc.scrub!(:prune) assert_equal INVALID_PRUNED, doc.xpath("/html/body").inner_html assert_equal doc, result @@ -144,8 +146,8 @@ def html5? context ":strip" do it "strip bad tags" do - doc = klass.parse "#{INVALID_FRAGMENT}" - result = doc.scrub! :strip + doc = klass.parse("#{INVALID_FRAGMENT}") + result = doc.scrub!(:strip) assert_equal INVALID_STRIPPED, doc.xpath("/html/body").inner_html assert_equal doc, result @@ -154,8 +156,8 @@ def html5? context ":whitewash" do it "whitewash the markup" do - doc = klass.parse "#{WHITEWASH_FRAGMENT}" - result = doc.scrub! :whitewash + doc = klass.parse("#{WHITEWASH_FRAGMENT}") + result = doc.scrub!(:whitewash) ww_result = if Nokogiri.uses_libxml?("<2.9.11") || html5? WHITEWASH_RESULT @@ -164,6 +166,7 @@ def html5? else WHITEWASH_RESULT_LIBXML2911 end + assert_equal ww_result, doc.xpath("/html/body").inner_html assert_equal doc, result end @@ -171,8 +174,8 @@ def html5? context ":nofollow" do it "add a 'nofollow' attribute to hyperlinks" do - doc = klass.parse "#{NOFOLLOW_FRAGMENT}" - result = doc.scrub! :nofollow + doc = klass.parse("#{NOFOLLOW_FRAGMENT}") + result = doc.scrub!(:nofollow) assert_equal NOFOLLOW_RESULT, doc.xpath("/html/body").inner_html assert_equal doc, result @@ -181,8 +184,8 @@ def html5? context ":unprintable" do it "removes unprintable unicode characters" do - doc = klass.parse "#{UNPRINTABLE_FRAGMENT}" - result = doc.scrub! :unprintable + doc = klass.parse("#{UNPRINTABLE_FRAGMENT}") + result = doc.scrub!(:unprintable) assert_equal UNPRINTABLE_RESULT, doc.xpath("/html/body").inner_html assert_equal doc, result @@ -192,7 +195,7 @@ def html5? context "#text" do it "leave behind only inner text with html entities still escaped" do - doc = klass.parse "#{ENTITY_HACK_ATTACK}" + doc = klass.parse("#{ENTITY_HACK_ATTACK}") result = doc.text assert_equal ENTITY_HACK_ATTACK_TEXT_SCRUB, result @@ -200,8 +203,8 @@ def html5? context "with encode_special_chars => false" do it "leave behind only inner text with html entities unescaped" do - doc = klass.parse "#{ENTITY_HACK_ATTACK}" - result = doc.text(:encode_special_chars => false) + doc = klass.parse("#{ENTITY_HACK_ATTACK}") + result = doc.text(encode_special_chars: false) assert_equal ENTITY_HACK_ATTACK_TEXT_SCRUB_UNESC, result end @@ -209,8 +212,8 @@ def html5? context "with encode_special_chars => true" do it "leave behind only inner text with html entities still escaped" do - doc = klass.parse "#{ENTITY_HACK_ATTACK}" - result = doc.text(:encode_special_chars => true) + doc = klass.parse("#{ENTITY_HACK_ATTACK}") + result = doc.text(encode_special_chars: true) assert_equal ENTITY_HACK_ATTACK_TEXT_SCRUB, result end @@ -227,13 +230,14 @@ def html5? refute_nil doc.xpath("/html/body").first if html5? || Nokogiri.jruby? - refute_match %r//, string - assert_match %r//, string - assert_match %r//, string + + assert_match(//, string) + assert_match(//, string) + assert_match(//, string) end end @@ -247,13 +251,14 @@ def html5? refute_nil doc.xpath("/html/body").first if html5? || Nokogiri.jruby? - refute_match %r//, string - assert_match %r//, string - assert_match %r//, string + + assert_match(//, string) + assert_match(//, string) + assert_match(//, string) end end @@ -261,20 +266,20 @@ def html5? context "#scrub!" do it "only scrub subtree" do xml = klass.parse(<<~HTML) - -
      - -
      -
      - -
      - + +
      + +
      +
      + +
      + HTML - node = xml.at_css "div.scrub" + node = xml.at_css("div.scrub") node.scrub!(:prune) - assert_match %r/I should remain/, xml.to_s - refute_match %r/I should be removed/, xml.to_s + assert_match(/I should remain/, xml.to_s) + refute_match(/I should be removed/, xml.to_s) end end end @@ -295,13 +300,14 @@ def html5?
      HTML - node_set = xml.css "div.scrub" + node_set = xml.css("div.scrub") + assert_equal 2, node_set.length node_set.scrub!(:prune) - assert_match %r/I should remain/, xml.to_s - refute_match %r/I should be removed/, xml.to_s - refute_match %r/I should also be removed/, xml.to_s + assert_match(/I should remain/, xml.to_s) + refute_match(/I should be removed/, xml.to_s) + refute_match(/I should also be removed/, xml.to_s) end end end @@ -319,8 +325,8 @@ def html5? context "#scrub!" do context ":escape" do it "escape bad tags" do - doc = klass.parse "
      #{INVALID_FRAGMENT}
      " - result = doc.scrub! :escape + doc = klass.parse("
      #{INVALID_FRAGMENT}
      ") + result = doc.scrub!(:escape) assert_equal INVALID_ESCAPED, doc.xpath("./div").inner_html assert_equal doc, result @@ -329,8 +335,8 @@ def html5? context ":prune" do it "prune bad tags" do - doc = klass.parse "
      #{INVALID_FRAGMENT}
      " - result = doc.scrub! :prune + doc = klass.parse("
      #{INVALID_FRAGMENT}
      ") + result = doc.scrub!(:prune) assert_equal INVALID_PRUNED, doc.xpath("./div").inner_html assert_equal doc, result @@ -339,8 +345,8 @@ def html5? context ":strip" do it "strip bad tags" do - doc = klass.parse "
      #{INVALID_FRAGMENT}
      " - result = doc.scrub! :strip + doc = klass.parse("
      #{INVALID_FRAGMENT}
      ") + result = doc.scrub!(:strip) assert_equal INVALID_STRIPPED, doc.xpath("./div").inner_html assert_equal doc, result @@ -349,8 +355,8 @@ def html5? context ":whitewash" do it "whitewash the markup" do - doc = klass.parse "
      #{WHITEWASH_FRAGMENT}
      " - result = doc.scrub! :whitewash + doc = klass.parse("
      #{WHITEWASH_FRAGMENT}
      ") + result = doc.scrub!(:whitewash) ww_result = if Nokogiri.uses_libxml?("<2.9.11") || html5? WHITEWASH_RESULT @@ -359,6 +365,7 @@ def html5? else WHITEWASH_RESULT_LIBXML2911 end + assert_equal ww_result, doc.xpath("./div").inner_html assert_equal doc, result end @@ -367,8 +374,8 @@ def html5? context ":nofollow" do context "for a hyperlink that does not have a rel attribute" do it "add a 'nofollow' attribute to hyperlinks" do - doc = klass.parse "
      #{NOFOLLOW_FRAGMENT}
      " - result = doc.scrub! :nofollow + doc = klass.parse("
      #{NOFOLLOW_FRAGMENT}
      ") + result = doc.scrub!(:nofollow) assert_equal NOFOLLOW_RESULT, doc.xpath("./div").inner_html assert_equal doc, result @@ -377,8 +384,8 @@ def html5? context "for a hyperlink that does have a rel attribute" do it "appends nofollow to rel attribute" do - doc = klass.parse "
      #{NOFOLLOW_WITH_REL_FRAGMENT}
      " - result = doc.scrub! :nofollow + doc = klass.parse("
      #{NOFOLLOW_WITH_REL_FRAGMENT}
      ") + result = doc.scrub!(:nofollow) assert_equal NOFOLLOW_WITH_REL_RESULT, doc.xpath("./div").inner_html assert_equal doc, result @@ -389,8 +396,8 @@ def html5? context ":noopener" do context "for a hyperlink without a 'rel' attribute" do it "add a 'noopener' attribute to hyperlinks" do - doc = klass.parse "
      #{NOOPENER_FRAGMENT}
      " - result = doc.scrub! :noopener + doc = klass.parse("
      #{NOOPENER_FRAGMENT}
      ") + result = doc.scrub!(:noopener) assert_equal NOOPENER_RESULT, doc.xpath("./div").inner_html assert_equal doc, result @@ -399,8 +406,8 @@ def html5? context "for a hyperlink that does have a rel attribute" do it "appends 'noopener' to 'rel' attribute" do - doc = klass.parse "
      #{NOOPENER_WITH_REL_FRAGMENT}
      " - result = doc.scrub! :noopener + doc = klass.parse("
      #{NOOPENER_WITH_REL_FRAGMENT}
      ") + result = doc.scrub!(:noopener) assert_equal NOOPENER_WITH_REL_RESULT, doc.xpath("./div").inner_html assert_equal doc, result @@ -410,8 +417,8 @@ def html5? context ":unprintable" do it "removes unprintable unicode characters" do - doc = klass.parse "
      #{UNPRINTABLE_FRAGMENT}
      " - result = doc.scrub! :unprintable + doc = klass.parse("
      #{UNPRINTABLE_FRAGMENT}
      ") + result = doc.scrub!(:unprintable) assert_equal UNPRINTABLE_RESULT, doc.xpath("./div").inner_html assert_equal doc, result @@ -421,7 +428,7 @@ def html5? context "#text" do it "leave behind only inner text with html entities still escaped" do - doc = klass.parse "
      #{ENTITY_HACK_ATTACK}
      " + doc = klass.parse("
      #{ENTITY_HACK_ATTACK}
      ") result = doc.text assert_equal ENTITY_HACK_ATTACK_TEXT_SCRUB, result @@ -429,8 +436,8 @@ def html5? context "with encode_special_chars => false" do it "leave behind only inner text with html entities unescaped" do - doc = klass.parse "
      #{ENTITY_HACK_ATTACK}
      " - result = doc.text(:encode_special_chars => false) + doc = klass.parse("
      #{ENTITY_HACK_ATTACK}
      ") + result = doc.text(encode_special_chars: false) assert_equal ENTITY_HACK_ATTACK_TEXT_SCRUB_UNESC, result end @@ -438,8 +445,8 @@ def html5? context "with encode_special_chars => true" do it "leave behind only inner text with html entities still escaped" do - doc = klass.parse "
      #{ENTITY_HACK_ATTACK}
      " - result = doc.text(:encode_special_chars => true) + doc = klass.parse("
      #{ENTITY_HACK_ATTACK}
      ") + result = doc.text(encode_special_chars: true) assert_equal ENTITY_HACK_ATTACK_TEXT_SCRUB, result end @@ -450,7 +457,7 @@ def html5? it "not remove entities" do string = klass.parse(ENTITY_FRAGMENT).to_s - assert_match %r/this is </, string + assert_match(/this is </, string) end end @@ -465,11 +472,11 @@ def html5?
      HTML - node = xml.at_css "div.scrub" + node = xml.at_css("div.scrub") node.scrub!(:prune) - assert_match %r(I should remain), xml.to_s - refute_match %r(I should be removed), xml.to_s + assert_match(/I should remain/, xml.to_s) + refute_match(/I should be removed/, xml.to_s) end end end @@ -488,15 +495,15 @@ def html5?
      HTML - node_set = xml.css "div.scrub" + node_set = xml.css("div.scrub") assert_equal 2, node_set.length node_set.scrub!(:prune) - assert_match %r/I should remain/, xml.to_s - refute_match %r/I should be removed/, xml.to_s - refute_match %r/I should also be removed/, xml.to_s + assert_match(/I should remain/, xml.to_s) + refute_match(/I should be removed/, xml.to_s) + refute_match(/I should also be removed/, xml.to_s) end end end diff --git a/test/integration/test_xml.rb b/test/integration/test_xml.rb index 7abc048d..acfcd215 100644 --- a/test/integration/test_xml.rb +++ b/test/integration/test_xml.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "helper" class IntegrationTestXml < Loofah::TestCase @@ -5,23 +7,25 @@ class IntegrationTestXml < Loofah::TestCase context "xml document" do context "custom scrubber" do it "act as expected" do - xml = Loofah.xml_document <<-EOXML + xml = Loofah.xml_document(<<~XML) Abraham Lincoln Abe Vigoda - EOXML + XML bring_out_your_dead = Loofah::Scrubber.new do |node| if node.name == "employee" and node["deceased"] == "true" node.remove Loofah::Scrubber::STOP # don't bother with the rest of the subtree end end + assert_equal 2, xml.css("employee").length xml.scrub!(bring_out_your_dead) - employees = xml.css "employee" + employees = xml.css("employee") + assert_equal 1, employees.length assert_equal "Abe Vigoda", employees.first.inner_text end @@ -31,21 +35,23 @@ class IntegrationTestXml < Loofah::TestCase context "xml fragment" do context "custom scrubber" do it "act as expected" do - xml = Loofah.xml_fragment <<-EOXML + xml = Loofah.xml_fragment(<<~XML) Abraham Lincoln Abe Vigoda - EOXML + XML bring_out_your_dead = Loofah::Scrubber.new do |node| if node.name == "employee" and node["deceased"] == "true" node.remove Loofah::Scrubber::STOP # don't bother with the rest of the subtree end end + assert_equal 2, xml.css("employee").length xml.scrub!(bring_out_your_dead) - employees = xml.css "employee" + employees = xml.css("employee") + assert_equal 1, employees.length assert_equal "Abe Vigoda", employees.first.inner_text end diff --git a/test/unit/test_api.rb b/test/unit/test_api.rb index 6706549c..30cc288a 100644 --- a/test/unit/test_api.rb +++ b/test/unit/test_api.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "helper" class UnitTestApi < Loofah::TestCase @@ -14,24 +16,28 @@ class UnitTestApi < Loofah::TestCase describe "generic class methods" do it "creates html4 documents" do doc = Loofah.document(html) + assert_kind_of(Loofah::HTML4::Document, doc) assert_equal html, doc.xpath("/html/body").inner_html end it "scrubs html4 documents" do doc = Loofah.scrub_document(html, :strip) + assert_kind_of(Loofah::HTML4::Document, doc) assert_equal html, doc.xpath("/html/body").inner_html end it "creates html4 fragments" do doc = Loofah.fragment(html) + assert_kind_of(Loofah::HTML4::DocumentFragment, doc) assert_equal html, doc.inner_html end it "scrubs html4 fragments" do doc = Loofah.scrub_fragment(html, :strip) + assert_kind_of(Loofah::HTML4::DocumentFragment, doc) assert_equal html, doc.inner_html end @@ -40,24 +46,28 @@ class UnitTestApi < Loofah::TestCase describe "html4 methods" do it "creates html4 documents" do doc = Loofah.html4_document(html) + assert_kind_of(Loofah::HTML4::Document, doc) assert_equal html, doc.xpath("/html/body").inner_html end it "scrubs html4 documents" do doc = Loofah.scrub_html4_document(html, :strip) + assert_kind_of(Loofah::HTML4::Document, doc) assert_equal html, doc.xpath("/html/body").inner_html end it "creates html4 fragments" do doc = Loofah.html4_fragment(html) + assert_kind_of(Loofah::HTML4::DocumentFragment, doc) assert_equal html, doc.inner_html end it "scrubs html4 fragments" do doc = Loofah.scrub_html4_fragment(html, :strip) + assert_kind_of(Loofah::HTML4::DocumentFragment, doc) assert_equal html, doc.inner_html end @@ -67,24 +77,28 @@ class UnitTestApi < Loofah::TestCase if Loofah.html5_support? it "creates html5 documents" do doc = Loofah.html5_document(html) + assert_kind_of(Loofah::HTML5::Document, doc) assert_equal html, doc.xpath("/html/body").inner_html end it "scrubs html5 documents" do doc = Loofah.scrub_html5_document(html, :strip) + assert_kind_of(Loofah::HTML5::Document, doc) assert_equal html, doc.xpath("/html/body").inner_html end it "creates html5 fragments" do doc = Loofah.html5_fragment(html) + assert_kind_of(Loofah::HTML5::DocumentFragment, doc) assert_equal html, doc.inner_html end it "scrubs html5 fragments" do doc = Loofah.scrub_html5_fragment(html, :strip) + assert_kind_of(Loofah::HTML5::DocumentFragment, doc) assert_equal html, doc.inner_html end @@ -101,24 +115,28 @@ class UnitTestApi < Loofah::TestCase describe "xml methods" do it "creates xml documents" do doc = Loofah.xml_document(xml) + assert_kind_of(Loofah::XML::Document, doc) assert_equal xml, doc.root.to_xml end it "scrubs xml documents" do doc = Loofah.scrub_xml_document(xml, xml_scrubber) + assert_kind_of(Loofah::XML::Document, doc) assert_equal xml, doc.root.to_xml end it "creates xml fragments" do doc = Loofah.xml_fragment(xml_fragment) + assert_kind_of(Loofah::XML::DocumentFragment, doc) assert_equal xml_fragment, doc.children.to_xml end it "scrubs xml fragments" do doc = Loofah.scrub_xml_fragment(xml_fragment, :strip) + assert_kind_of(Loofah::XML::DocumentFragment, doc) assert_equal xml_fragment, doc.children.to_xml end @@ -133,23 +151,27 @@ class UnitTestApi < Loofah::TestCase it "parses documents" do doc = Loofah::HTML4::Document.parse(html) + assert_kind_of(Loofah::HTML4::Document, doc) assert_equal html, doc.xpath("/html/body").inner_html end it "parses document fragment" do doc = Loofah::HTML4::DocumentFragment.parse(html) + assert_kind_of(Loofah::HTML4::DocumentFragment, doc) assert_equal html, doc.inner_html end it "scrubs documents" do doc = Loofah::HTML4::Document.parse(html).scrub!(:strip) + assert_equal html, doc.xpath("/html/body").inner_html end it "scrubs fragments" do doc = Loofah::HTML4::DocumentFragment.parse(html).scrub!(:strip) + assert_equal html, doc.inner_html end @@ -160,6 +182,7 @@ class UnitTestApi < Loofah::TestCase it "adds instance methods to document nodes" do doc = Loofah::HTML4::Document.parse(html) + assert(node = doc.at_css("div")) node.scrub!(:strip) end @@ -171,12 +194,14 @@ class UnitTestApi < Loofah::TestCase it "adds instance methods to fragment nodes" do doc = Loofah::HTML4::DocumentFragment.parse(html) + assert(node = doc.at_css("div")) node.scrub!(:strip) end it "adds instance methods to document nodesets" do doc = Loofah.html4_document(html) + assert(node_set = doc.css("div")) assert_instance_of Nokogiri::XML::NodeSet, node_set node_set.scrub!(:strip) @@ -184,6 +209,7 @@ class UnitTestApi < Loofah::TestCase it "adds instance methods to fragment nodesets" do doc = Loofah.html4_fragment(html) + assert(node_set = doc.css("div")) assert_instance_of Nokogiri::XML::NodeSet, node_set node_set.scrub!(:strip) @@ -191,11 +217,13 @@ class UnitTestApi < Loofah::TestCase it "exposes serialize_root on Loofah::HTML4::DocumentFragment" do doc = Loofah.html4_fragment(html) + assert_equal html, doc.serialize_root.to_html end it "exposes serialize_root on Loofah::HTML4::Document" do doc = Loofah.html4_document(html) + assert_equal html, doc.serialize_root.children.to_html end end @@ -208,12 +236,14 @@ class UnitTestApi < Loofah::TestCase it "parses documents" do doc = Loofah::XML::Document.parse(xml) + assert_kind_of(Loofah::XML::Document, doc) assert_equal xml, doc.root.to_xml end it "parses document fragments" do doc = Loofah::XML::DocumentFragment.parse(xml_fragment) + assert_kind_of(Loofah::XML::DocumentFragment, doc) assert_equal xml_fragment, doc.children.to_xml end @@ -221,12 +251,14 @@ class UnitTestApi < Loofah::TestCase it "scrubs documents" do scrubber = Loofah::Scrubber.new { |node| } doc = Loofah.xml_document(xml).scrub!(scrubber) + assert_equal xml, doc.root.to_xml end it "scrubs fragments" do scrubber = Loofah::Scrubber.new { |node| } doc = Loofah.xml_fragment(xml_fragment).scrub!(scrubber) + assert_equal xml_fragment, doc.children.to_xml end @@ -238,6 +270,7 @@ class UnitTestApi < Loofah::TestCase it "adds instance methods to document nodes" do doc = Loofah.xml_document(xml) + assert(node = doc.at_css("div")) node.scrub!(:strip) end @@ -249,12 +282,14 @@ class UnitTestApi < Loofah::TestCase it "adds instance methods to fragment nodes" do doc = Loofah.xml_fragment(xml) + assert(node = doc.at_css("div")) node.scrub!(:strip) end it "adds instance methods to document nodesets" do doc = Loofah.xml_document(xml) + assert(node_set = doc.css("div")) assert_instance_of Nokogiri::XML::NodeSet, node_set node_set.scrub!(:strip) @@ -262,6 +297,7 @@ class UnitTestApi < Loofah::TestCase it "adds instance methods to document nodesets" do doc = Loofah.xml_fragment(xml) + assert(node_set = doc.css("div")) assert_instance_of Nokogiri::XML::NodeSet, node_set node_set.scrub!(:strip) diff --git a/test/unit/test_encoding.rb b/test/unit/test_encoding.rb index ade02fe0..1b0a56c9 100644 --- a/test/unit/test_encoding.rb +++ b/test/unit/test_encoding.rb @@ -1,4 +1,6 @@ # :coding: utf-8 +# frozen_string_literal: true + require "helper" class UnitTestEncoding < Loofah::TestCase @@ -8,6 +10,7 @@ class UnitTestEncoding < Loofah::TestCase describe "#scrub_html4_fragment" do it "sets the encoding for html" do escaped = Loofah.scrub_html4_fragment(UTF8_STRING, :escape).to_s + assert_equal UTF8_STRING.encoding, escaped.encoding end end @@ -15,6 +18,7 @@ class UnitTestEncoding < Loofah::TestCase describe "#scrub_html5_fragment" do it "sets the encoding for html" do escaped = Loofah.scrub_html5_fragment(UTF8_STRING, :escape).to_s + assert_equal UTF8_STRING.encoding, escaped.encoding end end if Loofah.html5_support? @@ -22,6 +26,7 @@ class UnitTestEncoding < Loofah::TestCase describe "#scrub_xml_fragment" do it "sets the encoding for xml" do escaped = Loofah.scrub_xml_fragment(UTF8_STRING, :escape).to_s + assert_equal UTF8_STRING.encoding, escaped.encoding end end diff --git a/test/unit/test_helpers.rb b/test/unit/test_helpers.rb index 4f8a2815..bb4067da 100644 --- a/test/unit/test_helpers.rb +++ b/test/unit/test_helpers.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "helper" class UnitTestHelpers < Loofah::TestCase @@ -70,7 +72,7 @@ class UnitTestHelpers < Loofah::TestCase it "calls .sanitize_css" do actual = nil Loofah::Helpers.stub(:sanitize_css, "sanitized", ["foobar"]) do - actual = Loofah::Helpers::ActionView::SafeListSanitizer.new.sanitize_css "foobar" + actual = Loofah::Helpers::ActionView::SafeListSanitizer.new.sanitize_css("foobar") end assert_equal("sanitized", actual) diff --git a/test/unit/test_scrubber.rb b/test/unit/test_scrubber.rb index ab3712d0..5640ba58 100644 --- a/test/unit/test_scrubber.rb +++ b/test/unit/test_scrubber.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "helper" class UnitTestScrubber < Loofah::TestCase @@ -15,7 +17,7 @@ class UnitTestScrubber < Loofah::TestCase context "returning CONTINUE" do before do - @scrubber = Loofah::Scrubber.new do |node| + @scrubber = Loofah::Scrubber.new do |_node| @count += 1 Loofah::Scrubber::CONTINUE end @@ -23,28 +25,32 @@ class UnitTestScrubber < Loofah::TestCase it "operates properly" do Loofah.scrub_html4_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_COUNT, @count end it "operates properly" do Loofah.scrub_html4_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_COUNT, @count end it "operates properly" do Loofah.scrub_html5_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_COUNT, @count end if Loofah.html5_support? it "operates properly" do Loofah.scrub_html5_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_COUNT, @count end if Loofah.html5_support? end context "returning STOP" do before do - @scrubber = Loofah::Scrubber.new do |node| + @scrubber = Loofah::Scrubber.new do |_node| @count += 1 Loofah::Scrubber::STOP end @@ -52,56 +58,64 @@ class UnitTestScrubber < Loofah::TestCase it "operates as top-down" do Loofah.scrub_html4_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_STOP_TOP_DOWN, @count end it "operates as top-down" do Loofah.scrub_html4_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_STOP_TOP_DOWN, @count end it "operates as top-down" do Loofah.scrub_html5_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_STOP_TOP_DOWN, @count end if Loofah.html5_support? it "operates as top-down" do Loofah.scrub_html5_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_STOP_TOP_DOWN, @count end if Loofah.html5_support? end context "returning neither CONTINUE nor STOP" do before do - @scrubber = Loofah::Scrubber.new do |node| + @scrubber = Loofah::Scrubber.new do |_node| @count += 1 end end it "acts as if CONTINUE was returned" do Loofah.scrub_html4_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_COUNT, @count end it "acts as if CONTINUE was returned" do Loofah.scrub_html4_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_COUNT, @count end it "acts as if CONTINUE was returned" do Loofah.scrub_html5_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_COUNT, @count end if Loofah.html5_support? it "acts as if CONTINUE was returned" do Loofah.scrub_html5_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_COUNT, @count end if Loofah.html5_support? end context "not specifying direction" do before do - @scrubber = Loofah::Scrubber.new() do |node| + @scrubber = Loofah::Scrubber.new do |_node| @count += 1 Loofah::Scrubber::STOP end @@ -109,28 +123,32 @@ class UnitTestScrubber < Loofah::TestCase it "operates as top-down" do Loofah.scrub_html4_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_STOP_TOP_DOWN, @count end it "operates as top-down" do Loofah.scrub_html4_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_STOP_TOP_DOWN, @count end it "operates as top-down" do Loofah.scrub_html5_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_STOP_TOP_DOWN, @count end if Loofah.html5_support? it "operates as top-down" do Loofah.scrub_html5_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_STOP_TOP_DOWN, @count end if Loofah.html5_support? end context "specifying top-down direction" do before do - @scrubber = Loofah::Scrubber.new(:direction => :top_down) do |node| + @scrubber = Loofah::Scrubber.new(direction: :top_down) do |_node| @count += 1 Loofah::Scrubber::STOP end @@ -138,58 +156,66 @@ class UnitTestScrubber < Loofah::TestCase it "operates as top-down" do Loofah.scrub_html4_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_STOP_TOP_DOWN, @count end it "operates as top-down" do Loofah.scrub_html4_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_STOP_TOP_DOWN, @count end it "operates as top-down" do Loofah.scrub_html5_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_STOP_TOP_DOWN, @count end if Loofah.html5_support? it "operates as top-down" do Loofah.scrub_html5_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_STOP_TOP_DOWN, @count end if Loofah.html5_support? end context "specifying bottom-up direction" do before do - @scrubber = Loofah::Scrubber.new(:direction => :bottom_up) do |node| + @scrubber = Loofah::Scrubber.new(direction: :bottom_up) do |_node| @count += 1 end end it "operates as bottom-up" do Loofah.scrub_html4_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_COUNT, @count end it "operates as bottom-up" do Loofah.scrub_html4_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_COUNT, @count end it "operates as bottom-up" do Loofah.scrub_html5_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_COUNT, @count end if Loofah.html5_support? it "operates as bottom-up" do Loofah.scrub_html5_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_COUNT, @count end if Loofah.html5_support? end context "invalid direction" do it "raise an exception" do - assert_raises(ArgumentError) { - Loofah::Scrubber.new(:direction => :quux) { } - } + assert_raises(ArgumentError) do + Loofah::Scrubber.new(direction: :quux) {} + end end end @@ -202,21 +228,25 @@ class UnitTestScrubber < Loofah::TestCase it "works anyway, shrug" do Loofah.scrub_html4_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_COUNT, @count end it "works anyway, shrug" do Loofah.scrub_html4_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_COUNT, @count end it "works anyway, shrug" do Loofah.scrub_html5_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_COUNT, @count end if Loofah.html5_support? it "works anyway, shrug" do Loofah.scrub_html5_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_COUNT, @count end if Loofah.html5_support? end @@ -242,26 +272,31 @@ def scrub(node) context "when not specifying direction" do before do @scrubber = @klass.new + assert_nil @scrubber.direction end it "operates as top-down" do Loofah.scrub_html4_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_STOP_TOP_DOWN, @scrubber.count end it "operates as top-down" do Loofah.scrub_html4_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_STOP_TOP_DOWN, @scrubber.count end it "operates as top-down" do Loofah.scrub_html5_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_STOP_TOP_DOWN, @scrubber.count end if Loofah.html5_support? it "operates as top-down" do Loofah.scrub_html5_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_STOP_TOP_DOWN, @scrubber.count end if Loofah.html5_support? end @@ -269,26 +304,31 @@ def scrub(node) context "when direction is specified as top_down" do before do @scrubber = @klass.new(:top_down) + assert_equal :top_down, @scrubber.direction end it "operates as top-down" do Loofah.scrub_html4_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_STOP_TOP_DOWN, @scrubber.count end it "operates as top-down" do Loofah.scrub_html4_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_STOP_TOP_DOWN, @scrubber.count end it "operates as top-down" do Loofah.scrub_html5_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_STOP_TOP_DOWN, @scrubber.count end if Loofah.html5_support? it "operates as top-down" do Loofah.scrub_html5_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_STOP_TOP_DOWN, @scrubber.count end if Loofah.html5_support? end @@ -296,26 +336,31 @@ def scrub(node) context "when direction is specified as bottom_up" do before do @scrubber = @klass.new(:bottom_up) + assert_equal :bottom_up, @scrubber.direction end it "operates as bottom-up" do Loofah.scrub_html4_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_COUNT, @scrubber.count end it "operates as bottom-up" do Loofah.scrub_html4_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_COUNT, @scrubber.count end it "operates as bottom-up" do Loofah.scrub_html5_fragment(FRAGMENT, @scrubber) + assert_equal FRAGMENT_NODE_COUNT, @scrubber.count end if Loofah.html5_support? it "operates as bottom-up" do Loofah.scrub_html5_document(DOCUMENT, @scrubber) + assert_equal DOCUMENT_NODE_COUNT, @scrubber.count end if Loofah.html5_support? end @@ -330,21 +375,21 @@ def initialize; end end it "raises an exception" do - assert_raises(Loofah::ScrubberNotFound) { + assert_raises(Loofah::ScrubberNotFound) do Loofah.scrub_html4_fragment(FRAGMENT, @scrubber) - } + end - assert_raises(Loofah::ScrubberNotFound) { + assert_raises(Loofah::ScrubberNotFound) do Loofah.scrub_html4_document(DOCUMENT, @scrubber) - } + end - assert_raises(Loofah::ScrubberNotFound) { + assert_raises(Loofah::ScrubberNotFound) do Loofah.scrub_html5_fragment(FRAGMENT, @scrubber) - } if Loofah.html5_support? + end if Loofah.html5_support? - assert_raises(Loofah::ScrubberNotFound) { + assert_raises(Loofah::ScrubberNotFound) do Loofah.scrub_html5_document(DOCUMENT, @scrubber) - } if Loofah.html5_support? + end if Loofah.html5_support? end end end diff --git a/test/unit/test_scrubbers.rb b/test/unit/test_scrubbers.rb index 343badb1..43ba31ce 100644 --- a/test/unit/test_scrubbers.rb +++ b/test/unit/test_scrubbers.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "helper" class UnitTestScrubbers < Loofah::TestCase @@ -5,8 +7,8 @@ class UnitTestScrubbers < Loofah::TestCase context klass do context "bad scrub method" do it "raise a ScrubberNotFound exception" do - doc = klass.parse "

      foo

      " - assert_raises(Loofah::ScrubberNotFound) { doc.scrub! :frippery } + doc = klass.parse("

      foo

      ") + assert_raises(Loofah::ScrubberNotFound) { doc.scrub!(:frippery) } end end end From 9926529cf70563922fca96e56bf766378177bf01 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sun, 2 Apr 2023 15:07:48 -0400 Subject: [PATCH 3/5] style(rubocop): unsafe autocorrections --- .rubocop.yml | 4 ++++ lib/loofah/helpers.rb | 2 +- test/integration/test_ad_hoc.rb | 2 +- test/integration/test_xml.rb | 4 ++-- test/unit/test_encoding.rb | 2 +- 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 7e82ad99..8bee9eab 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -9,3 +9,7 @@ inherit_from: .rubocop_todo.yml AllCops: NewCops: enable TargetRubyVersion: "2.5" + +Style/ClassAndModuleChildren: + Exclude: + - test/** diff --git a/lib/loofah/helpers.rb b/lib/loofah/helpers.rb index 9545d4f5..4f59fcde 100644 --- a/lib/loofah/helpers.rb +++ b/lib/loofah/helpers.rb @@ -20,7 +20,7 @@ def strip_tags(string_or_io) def sanitize(string_or_io) loofah_fragment = Loofah.html4_fragment(string_or_io) loofah_fragment.scrub!(:strip) - loofah_fragment.xpath("./form").each { |form| form.remove } + loofah_fragment.xpath("./form").each(&:remove) loofah_fragment.to_s end diff --git a/test/integration/test_ad_hoc.rb b/test/integration/test_ad_hoc.rb index 811fedd3..8d24f961 100644 --- a/test/integration/test_ad_hoc.rb +++ b/test/integration/test_ad_hoc.rb @@ -325,7 +325,7 @@ def test_dont_remove_whitespace_between_tags it "document removes the comment" do sanitized = document(html) - refute(sanitized.children.any? { |node| node.comment? }) + refute(sanitized.children.any?(&:comment?)) end it "scrub_document removes the comment" do diff --git a/test/integration/test_xml.rb b/test/integration/test_xml.rb index acfcd215..0130c710 100644 --- a/test/integration/test_xml.rb +++ b/test/integration/test_xml.rb @@ -14,7 +14,7 @@ class IntegrationTestXml < Loofah::TestCase XML bring_out_your_dead = Loofah::Scrubber.new do |node| - if node.name == "employee" and node["deceased"] == "true" + if (node.name == "employee") && (node["deceased"] == "true") node.remove Loofah::Scrubber::STOP # don't bother with the rest of the subtree end @@ -40,7 +40,7 @@ class IntegrationTestXml < Loofah::TestCase Abe Vigoda XML bring_out_your_dead = Loofah::Scrubber.new do |node| - if node.name == "employee" and node["deceased"] == "true" + if (node.name == "employee") && (node["deceased"] == "true") node.remove Loofah::Scrubber::STOP # don't bother with the rest of the subtree end diff --git a/test/unit/test_encoding.rb b/test/unit/test_encoding.rb index 1b0a56c9..d7ebfab5 100644 --- a/test/unit/test_encoding.rb +++ b/test/unit/test_encoding.rb @@ -6,7 +6,7 @@ class UnitTestEncoding < Loofah::TestCase UTF8_STRING = "日本語" - if String.new.respond_to?(:encoding) + if (+"").respond_to?(:encoding) describe "#scrub_html4_fragment" do it "sets the encoding for html" do escaped = Loofah.scrub_html4_fragment(UTF8_STRING, :escape).to_s From 9ff910f365abef99b36cb40a44e16aad37eb1d49 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sun, 2 Apr 2023 15:27:15 -0400 Subject: [PATCH 4/5] style(rubocop): remaining manual fixes and todos --- .rubocop.yml | 9 +++++++++ .rubocop_todo.yml | 14 ++++++++++++++ Rakefile | 1 + lib/loofah.rb | 2 -- lib/loofah/concerns.rb | 26 ++++++++++++++++---------- lib/loofah/helpers.rb | 17 +++++++++++------ lib/loofah/html5/scrub.rb | 18 +++++++++++------- lib/loofah/metahelpers.rb | 14 ++++++++------ lib/loofah/scrubbers.rb | 28 +++++++++++++++------------- loofah.gemspec | 13 +++++++++++-- test/html5/test_sanitizer.rb | 6 +++--- test/unit/test_scrubber.rb | 4 ++-- 12 files changed, 101 insertions(+), 51 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 8bee9eab..6dae8c95 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -13,3 +13,12 @@ AllCops: Style/ClassAndModuleChildren: Exclude: - test/** + +Naming/InclusiveLanguage: + Exclude: + - lib/loofah/helpers.rb + - lib/loofah/html5/safelist.rb + +Performance/CollectionLiteralInLoop: + Exclude: + - test/**/*.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e69de29b..b17e4a96 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -0,0 +1,14 @@ +# This configuration was generated by +# `rubocop --auto-gen-config --exclude-limit 50` +# on 2023-04-02 19:26:30 UTC using RuboCop version 1.48.1. +# The point is for the user to remove these configuration records +# one by one as the offenses are removed from the code base. +# Note that changes in the inspected code, or installation of new +# versions of RuboCop, may require this file to be generated again. + +# Offense count: 3 +# This cop supports safe autocorrection (--autocorrect). +# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns. +# URISchemes: http, https +Layout/LineLength: + Max: 146 diff --git a/Rakefile b/Rakefile index 6bad5aa6..7795056a 100644 --- a/Rakefile +++ b/Rakefile @@ -45,6 +45,7 @@ Rake::Task["rubocop:todo:autocorrect_all"].clear task default: [:rubocop, :test] +desc "Print out the files packaged in the gem" task :debug_manifest do puts Bundler.load_gemspec("loofah.gemspec").files end diff --git a/lib/loofah.rb b/lib/loofah.rb index 217dab75..d4a1be2e 100644 --- a/lib/loofah.rb +++ b/lib/loofah.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -$LOAD_PATH.unshift(File.expand_path(File.dirname(__FILE__))) unless $LOAD_PATH.include?(File.expand_path(File.dirname(__FILE__))) - require "nokogiri" module Loofah diff --git a/lib/loofah/concerns.rb b/lib/loofah/concerns.rb index b050309f..9e0f5d7d 100644 --- a/lib/loofah/concerns.rb +++ b/lib/loofah/concerns.rb @@ -55,13 +55,15 @@ def scrub!(scrubber) end end - def self.resolve_scrubber(scrubber) # :nodoc: - scrubber = Scrubbers::MAP[scrubber].new if Scrubbers::MAP[scrubber] - unless scrubber.is_a?(Loofah::Scrubber) - raise Loofah::ScrubberNotFound, "not a Scrubber or a scrubber name: #{scrubber.inspect}" - end + class << self + def resolve_scrubber(scrubber) # :nodoc: + scrubber = Scrubbers::MAP[scrubber].new if Scrubbers::MAP[scrubber] + unless scrubber.is_a?(Loofah::Scrubber) + raise Loofah::ScrubberNotFound, "not a Scrubber or a scrubber name: #{scrubber.inspect}" + end - scrubber + scrubber + end end end @@ -153,8 +155,10 @@ def remove_comments_before_html_element(doc) end end - def self.included(base) - base.extend(ClassMethods) + class << self + def included(base) + base.extend(ClassMethods) + end end def serialize_root @@ -184,8 +188,10 @@ def document_klass end end - def self.included(base) - base.extend(ClassMethods) + class << self + def included(base) + base.extend(ClassMethods) + end end def to_s diff --git a/lib/loofah/helpers.rb b/lib/loofah/helpers.rb index 4f59fcde..6aebb4f3 100644 --- a/lib/loofah/helpers.rb +++ b/lib/loofah/helpers.rb @@ -6,7 +6,7 @@ class << self # # A replacement for Rails's built-in +strip_tags+ helper. # - # Loofah::Helpers.strip_tags("
      Hello there
      ") # => "Hello there" + # Loofah::Helpers.strip_tags("
      Hello there
      ") # => "Hello there" # def strip_tags(string_or_io) Loofah.html4_fragment(string_or_io).text @@ -15,7 +15,8 @@ def strip_tags(string_or_io) # # A replacement for Rails's built-in +sanitize+ helper. # - # Loofah::Helpers.sanitize("") # => "<script src=\"http://ha.ckers.org/xss.js\"></script>" + # Loofah::Helpers.sanitize("") + # # => "<script src=\"http://ha.ckers.org/xss.js\"></script>" # def sanitize(string_or_io) loofah_fragment = Loofah.html4_fragment(string_or_io) @@ -27,14 +28,16 @@ def sanitize(string_or_io) # # A replacement for Rails's built-in +sanitize_css+ helper. # - # Loofah::Helpers.sanitize_css("display:block;background-image:url(http://www.ragingplatypus.com/i/cam-full.jpg)") # => "display: block;" + # Loofah::Helpers.sanitize_css("display:block;background-image:url(http://example.com/foo.jpg)") + # # => "display: block;" # def sanitize_css(style_string) ::Loofah::HTML5::Scrub.scrub_css(style_string) end # - # A helper to remove extraneous whitespace from text-ified HTML + # A helper to remove extraneous whitespace from text-ified HTML. + # # TODO: remove this in a future major-point-release. # def remove_extraneous_whitespace(string) @@ -63,7 +66,8 @@ def white_list_sanitizer # # To use by default, call this in an application initializer: # - # ActionView::Helpers::SanitizeHelper.full_sanitizer = ::Loofah::Helpers::ActionView::FullSanitizer.new + # ActionView::Helpers::SanitizeHelper.full_sanitizer = \ + # Loofah::Helpers::ActionView::FullSanitizer.new # # Or, to generally opt-in to Loofah's view sanitizers: # @@ -80,7 +84,8 @@ def sanitize(html, *args) # # To use by default, call this in an application initializer: # - # ActionView::Helpers::SanitizeHelper.safe_list_sanitizer = ::Loofah::Helpers::ActionView::SafeListSanitizer.new + # ActionView::Helpers::SanitizeHelper.safe_list_sanitizer = \ + # Loofah::Helpers::ActionView::SafeListSanitizer.new # # Or, to generally opt-in to Loofah's view sanitizers: # diff --git a/lib/loofah/html5/scrub.rb b/lib/loofah/html5/scrub.rb index 66f887dc..a3972462 100644 --- a/lib/loofah/html5/scrub.rb +++ b/lib/loofah/html5/scrub.rb @@ -7,7 +7,7 @@ module Loofah module HTML5 # :nodoc: module Scrub CONTROL_CHARACTERS = /[`\u0000-\u0020\u007f\u0080-\u0101]/ - CSS_KEYWORDISH = /\A(#[0-9a-fA-F]+|rgb\(\d+%?,\d*%?,?\d*%?\)?|-?\d{0,3}\.?\d{0,10}(ch|cm|r?em|ex|in|lh|mm|pc|pt|px|Q|vmax|vmin|vw|vh|%|,|\))?)\z/ + CSS_KEYWORDISH = /\A(#[0-9a-fA-F]+|rgb\(\d+%?,\d*%?,?\d*%?\)?|-?\d{0,3}\.?\d{0,10}(ch|cm|r?em|ex|in|lh|mm|pc|pt|px|Q|vmax|vmin|vw|vh|%|,|\))?)\z/ # rubocop:disable Layout/LineLength CRASS_SEMICOLON = { node: :semicolon, raw: ";" } CSS_IMPORTANT = "!important" CSS_PROPERTY_STRING_WITHOUT_EMBEDDED_QUOTES = /\A(["'])?[^"']+\1\z/ @@ -44,10 +44,12 @@ def scrub_attributes(node) scrub_attribute_that_allows_local_ref(attr_node) end - if SafeList::SVG_ALLOW_LOCAL_HREF.include?(node.name) && attr_name == "xlink:href" && attr_node.value =~ /^\s*[^#\s].*/m - attr_node.remove - next - end + next unless SafeList::SVG_ALLOW_LOCAL_HREF.include?(node.name) && + attr_name == "xlink:href" && + attr_node.value =~ /^\s*[^#\s].*/m + + attr_node.remove + next end scrub_css_attribute(node) @@ -67,13 +69,14 @@ def scrub_css_attribute(node) end def scrub_css(style) + url_flags = [:url, :bad_url] style_tree = Crass.parse_properties(style) sanitized_tree = [] style_tree.each do |node| next unless node[:node] == :property next if node[:children].any? do |child| - [:url, :bad_url].include?(child[:node]) + url_flags.include?(child[:node]) end name = node[:name].downcase @@ -138,7 +141,8 @@ def scrub_attribute_that_allows_local_ref(attr_node) def scrub_uri_attribute(attr_node) # this block lifted nearly verbatim from HTML5 sanitization val_unescaped = CGI.unescapeHTML(attr_node.value).gsub(CONTROL_CHARACTERS, "").downcase - if val_unescaped =~ /^[a-z0-9][-+.a-z0-9]*:/ && !SafeList::ALLOWED_PROTOCOLS.include?(val_unescaped.split(SafeList::PROTOCOL_SEPARATOR)[0]) + if val_unescaped =~ /^[a-z0-9][-+.a-z0-9]*:/ && + !SafeList::ALLOWED_PROTOCOLS.include?(val_unescaped.split(SafeList::PROTOCOL_SEPARATOR)[0]) attr_node.remove return true elsif val_unescaped.split(SafeList::PROTOCOL_SEPARATOR)[0] == "data" diff --git a/lib/loofah/metahelpers.rb b/lib/loofah/metahelpers.rb index aecc21c9..7507f71d 100644 --- a/lib/loofah/metahelpers.rb +++ b/lib/loofah/metahelpers.rb @@ -2,13 +2,15 @@ module Loofah module MetaHelpers # :nodoc: - def self.add_downcased_set_members_to_all_set_constants(mojule) - mojule.constants.each do |constant_sym| - constant = mojule.const_get(constant_sym) - next unless Set === constant + class << self + def add_downcased_set_members_to_all_set_constants(mojule) + mojule.constants.each do |constant_sym| + constant = mojule.const_get(constant_sym) + next unless Set === constant - constant.dup.each do |member| - constant.add(member.downcase) + constant.dup.each do |member| + constant.add(member.downcase) + end end end end diff --git a/lib/loofah/scrubbers.rb b/lib/loofah/scrubbers.rb index 85885612..6823018b 100644 --- a/lib/loofah/scrubbers.rb +++ b/lib/loofah/scrubbers.rb @@ -95,7 +95,7 @@ module Scrubbers # => "ohai!
      div is safe
      but foo is not" # class Strip < Scrubber - def initialize + def initialize # rubocop:disable Lint/MissingSuper @direction = :bottom_up end @@ -118,7 +118,7 @@ def scrub(node) # => "ohai!
      div is safe
      " # class Prune < Scrubber - def initialize + def initialize # rubocop:disable Lint/MissingSuper @direction = :top_down end @@ -140,7 +140,7 @@ def scrub(node) # => "ohai!
      div is safe
      <foo>but foo is <b>not</b></foo>" # class Escape < Scrubber - def initialize + def initialize # rubocop:disable Lint/MissingSuper @direction = :top_down end @@ -172,7 +172,7 @@ def scrub(node) # Certainly not me. # class Whitewash < Scrubber - def initialize + def initialize # rubocop:disable Lint/MissingSuper @direction = :top_down end @@ -201,7 +201,7 @@ def scrub(node) # => "ohai! I like your blog post" # class NoFollow < Scrubber - def initialize + def initialize # rubocop:disable Lint/MissingSuper @direction = :top_down end @@ -223,7 +223,7 @@ def scrub(node) # => "ohai! I like your blog post" # class NoOpener < Scrubber - def initialize + def initialize # rubocop:disable Lint/MissingSuper @direction = :top_down end @@ -237,7 +237,7 @@ def scrub(node) # This class probably isn't useful publicly, but is used for #to_text's current implemention class NewlineBlockElements < Scrubber # :nodoc: - def initialize + def initialize # rubocop:disable Lint/MissingSuper @direction = :bottom_up end @@ -270,7 +270,7 @@ def scrub(node) # http://timelessrepo.com/json-isnt-a-javascript-subset # class Unprintable < Scrubber - def initialize + def initialize # rubocop:disable Lint/MissingSuper @direction = :top_down end @@ -296,11 +296,13 @@ def scrub(node) unprintable: Unprintable, } - # - # Returns an array of symbols representing the built-in scrubbers - # - def self.scrubber_symbols - MAP.keys + class << self + # + # Returns an array of symbols representing the built-in scrubbers + # + def scrubber_symbols + MAP.keys + end end end end diff --git a/loofah.gemspec b/loofah.gemspec index 84a1a297..916d2f21 100644 --- a/loofah.gemspec +++ b/loofah.gemspec @@ -9,8 +9,17 @@ Gem::Specification.new do |spec| spec.authors = ["Mike Dalessio", "Bryan Helmkamp"] spec.email = ["mike.dalessio@gmail.com", "bryan@brynary.com"] - spec.summary = "Loofah is a general library for manipulating and transforming HTML/XML documents and fragments, built on top of Nokogiri" - spec.description = "Loofah is a general library for manipulating and transforming HTML/XML documents and fragments, built on top of Nokogiri.\n\nLoofah excels at HTML sanitization (XSS prevention). It includes some nice HTML sanitizers, which are based on HTML5lib's safelist, so it most likely won't make your codes less secure. (These statements have not been evaluated by Netexperts.)\n\nActiveRecord extensions for sanitization are available in the [`loofah-activerecord` gem](https://github.com/flavorjones/loofah-activerecord)." + spec.summary = <<~TEXT + Loofah is a general library for manipulating and transforming HTML/XML documents and fragments, + built on top of Nokogiri. + TEXT + spec.description = <<~TEXT + Loofah is a general library for manipulating and transforming HTML/XML documents and fragments, + built on top of Nokogiri. + + Loofah also includes some HTML sanitizers based on `html5lib`'s safelist, which are a specific + application of the general transformation functionality. + TEXT spec.homepage = "https://github.com/flavorjones/loofah" spec.license = "MIT" diff --git a/test/html5/test_sanitizer.rb b/test/html5/test_sanitizer.rb index d3bcb4a8..8f533f6b 100755 --- a/test/html5/test_sanitizer.rb +++ b/test/html5/test_sanitizer.rb @@ -183,7 +183,7 @@ def test_should_allow_multi_word_data_attributes end def test_should_allow_empty_data_attributes - input = "

      foo bar baz

      " + input = '

      foo bar baz

      ' check_sanitization( input, @@ -342,7 +342,7 @@ def test_figure_element_is_valid ## require "json" Dir[File.join(File.dirname(__FILE__), "..", "assets", "testdata_sanitizer_tests1.dat")].each do |filename| - JSON.parse(open(filename).read).each do |test| + JSON.parse(File.read(filename)).each do |test| it "testdata sanitizer #{test["name"]}" do test.delete("name") input = test.delete("input") @@ -427,7 +427,7 @@ def test_css_vh_value assert_match(/100vh/, sane.inner_html) end - def test_css_Q_value + def test_css_q_value html = "
      " sane = Nokogiri::HTML(Loofah.scrub_html4_fragment(html, :escape).to_xml) diff --git a/test/unit/test_scrubber.rb b/test/unit/test_scrubber.rb index 5640ba58..cea02a9c 100644 --- a/test/unit/test_scrubber.rb +++ b/test/unit/test_scrubber.rb @@ -257,7 +257,7 @@ class UnitTestScrubber < Loofah::TestCase @klass = Class.new(Loofah::Scrubber) do attr_accessor :count - def initialize(direction = nil) + def initialize(direction = nil) # rubocop:disable Lint/MissingSuper @direction = direction @count = 0 end @@ -369,7 +369,7 @@ def scrub(node) context "creating a new Scrubber class with no scrub method" do before do @klass = Class.new(Loofah::Scrubber) do - def initialize; end + def initialize; end # rubocop:disable Lint/MissingSuper end @scrubber = @klass.new end From f226118d4d573ba16a59448d23215ca3ee91d9b5 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sun, 2 Apr 2023 21:11:49 -0400 Subject: [PATCH 5/5] dep: move dev dependencies into Gemfile and make Rubocop conditional on Ruby version, because 2.5 and 2.6 are too old for the rubocop version we have, and it's causing CI to fail. --- .rubocop.yml | 3 +++ Gemfile | 17 +++++++++++++++ Rakefile | 56 ++++++++++++++++++++++++++++---------------------- loofah.gemspec | 11 ---------- 4 files changed, 51 insertions(+), 36 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 6dae8c95..cb167424 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -22,3 +22,6 @@ Naming/InclusiveLanguage: Performance/CollectionLiteralInLoop: Exclude: - test/**/*.rb + +Rake/DuplicateTask: + Enabled: false diff --git a/Gemfile b/Gemfile index dee842d5..d39379d1 100644 --- a/Gemfile +++ b/Gemfile @@ -1,4 +1,21 @@ # frozen_string_literal: true source "https://rubygems.org/" + gemspec + +group :development do + gem("hoe-markdown", ["~> 1.3"]) + gem("json", ["~> 2.2"]) + gem("minitest", ["~> 5.14"]) + gem("rake", ["~> 13.0"]) + gem("rdoc", [">= 4.0", "< 7"]) + + if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("3.0.0") + gem("rubocop", "~> 1.1") + gem("rubocop-minitest", "0.29.0") + gem("rubocop-performance", "1.16.0") + gem("rubocop-rake", "0.6.0") + gem("rubocop-shopify", "2.12.0") + end +end diff --git a/Rakefile b/Rakefile index 7795056a..a61efce8 100644 --- a/Rakefile +++ b/Rakefile @@ -14,38 +14,44 @@ task :generate_safelists do load "tasks/generate-safelists" end -require "rubocop/rake_task" - -module RubocopHelper - class << self - def common_options(task) - task.patterns += [ - "Gemfile", - "Rakefile", - "lib", - "loofah.gemspec", - "test", - ] +begin + require "rubocop/rake_task" + + module RubocopHelper + class << self + def common_options(task) + task.patterns += [ + "Gemfile", + "Rakefile", + "lib", + "loofah.gemspec", + "test", + ] + end end end -end -RuboCop::RakeTask.new do |task| - RubocopHelper.common_options(task) -end + RuboCop::RakeTask.new do |task| + RubocopHelper.common_options(task) + end -desc("Generate the rubocop todo list") -RuboCop::RakeTask.new("rubocop:todo") do |task| - RubocopHelper.common_options(task) - task.options << "--auto-gen-config" - task.options << "--exclude-limit=50" -end -Rake::Task["rubocop:todo:autocorrect"].clear -Rake::Task["rubocop:todo:autocorrect_all"].clear + desc("Generate the rubocop todo list") + RuboCop::RakeTask.new("rubocop:todo") do |task| + RubocopHelper.common_options(task) + task.options << "--auto-gen-config" + task.options << "--exclude-limit=50" + end + Rake::Task["rubocop:todo:autocorrect"].clear + Rake::Task["rubocop:todo:autocorrect_all"].clear -task default: [:rubocop, :test] + task(default: :rubocop) +rescue LoadError + warn("NOTE: rubocop not available") +end desc "Print out the files packaged in the gem" task :debug_manifest do puts Bundler.load_gemspec("loofah.gemspec").files end + +task(default: :test) diff --git a/loofah.gemspec b/loofah.gemspec index 916d2f21..ca822fb0 100644 --- a/loofah.gemspec +++ b/loofah.gemspec @@ -44,15 +44,4 @@ Gem::Specification.new do |spec| spec.add_runtime_dependency("crass", ["~> 1.0.2"]) spec.add_runtime_dependency("nokogiri", [">= 1.5.9"]) - - spec.add_development_dependency("hoe-markdown", ["~> 1.3"]) - spec.add_development_dependency("json", ["~> 2.2"]) - spec.add_development_dependency("minitest", ["~> 5.14"]) - spec.add_development_dependency("rake", ["~> 13.0"]) - spec.add_development_dependency("rdoc", [">= 4.0", "< 7"]) - spec.add_development_dependency("rubocop", "~> 1.1") - spec.add_development_dependency("rubocop-minitest", "0.29.0") - spec.add_development_dependency("rubocop-performance", "1.16.0") - spec.add_development_dependency("rubocop-rake", "0.6.0") - spec.add_development_dependency("rubocop-shopify", "2.12.0") end