diff --git a/CHANGELOG.md b/CHANGELOG.md index 52e1cbe..57632be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ### Unreleased * Speed up `RuleSet::Declarations#to_s` ~20x: its clean-up regexp matched the empty string at every character (`[\n\r\f\t]*`), making `gsub!` replace at every position; the unreachable `\s+$` branch is dropped, output unchanged * Allocate less in `RuleSet`: skip the downcased copy of already normalized property names, and build dimension shorthands without enumerators +* Allocate less when expanding and creating dimension shorthands, and skip the colour regexp on values without a comma +* Return already sanitized media types as-is from `sanitize_media_query`, and allocate less when splitting selectors and counting ids ### Version 3.3.0 * Fix Arbitrary local-file read via attacker-controlled `@import` diff --git a/lib/css_parser.rb b/lib/css_parser.rb index 62e87da..97f4f47 100644 --- a/lib/css_parser.rb +++ b/lib/css_parser.rb @@ -107,7 +107,7 @@ def self.merge(*rule_sets) #++ def self.calculate_specificity(selector) a = 0 - b = selector.scan('#').length + b = selector.count('#') c = selector.scan(NON_ID_ATTRIBUTES_AND_PSEUDO_CLASSES_RX_NC).length d = selector.scan(ELEMENTS_AND_PSEUDO_ELEMENTS_RX_NC).length @@ -147,6 +147,8 @@ def self.convert_uris(css, base_uri) end def self.sanitize_media_query(raw) + return raw if raw.is_a?(Symbol) && !raw.match?(UNSANITIZED_MEDIA_QUERY_RX) + mq = raw.to_s.gsub(/\s+/, ' ') mq.strip! mq = 'all' if mq.empty? diff --git a/lib/css_parser/regexps.rb b/lib/css_parser/regexps.rb index c150785..b8e823e 100644 --- a/lib/css_parser/regexps.rb +++ b/lib/css_parser/regexps.rb @@ -35,6 +35,9 @@ def self.regex_possible_values(*values) #++ IMPORTANT_IN_PROPERTY_RX = /\s*!important\b\s*/i.freeze + # What sanitize_media_query would change: blank, padded, or not single-space separated + UNSANITIZED_MEDIA_QUERY_RX = /\A\z|\A[\s\0]|[\s\0]\z|\s\s|[^\S ]/.freeze + RE_INSIDE_OUTSIDE = regex_possible_values 'inside', 'outside' RE_SCROLL_FIXED = regex_possible_values 'scroll', 'fixed' RE_REPEAT = regex_possible_values 'repeat(\-x|\-y)*|no\-repeat' diff --git a/lib/css_parser/rule_set.rb b/lib/css_parser/rule_set.rb index cff4bd7..375f871 100644 --- a/lib/css_parser/rule_set.rb +++ b/lib/css_parser/rule_set.rb @@ -403,14 +403,14 @@ def expand_dimensions_shorthand! # :nodoc: DIMENSIONS.each do |property, (top, right, bottom, left)| next unless (declaration = declarations[property]) - value = declaration.value.dup + value = declaration.value # RGB and HSL values in borders are the only units that can have spaces (within params). # We cheat a bit here by stripping spaces after commas in RGB and HSL values so that we # can split easily on spaces. # # TODO: rgba, hsl, hsla - value.gsub!(RE_COLOUR) { |c| c.gsub(/(\s*,\s*)/, ',') } + value = value.gsub(RE_COLOUR) { |c| c.gsub(/(\s*,\s*)/, ',') } if value.include?(',') matches = split_value_preserving_function_whitespace(value) @@ -428,7 +428,7 @@ def expand_dimensions_shorthand! # :nodoc: raise ArgumentError, "Cannot parse #{value}" end - replacement = [top, right, bottom, left].zip(values).to_h + replacement = {top => values[0], right => values[1], bottom => values[2], left => values[3]} declarations.replace_declaration!(property, replacement, preserve_importance: true) end @@ -522,17 +522,13 @@ def create_shorthand! # Combine several properties into a shorthand one def create_shorthand_properties!(properties, shorthand_property) # :nodoc: - values = [] - properties_to_delete = [] - properties.each do |property| - next unless (declaration = declarations[property]) - next if declaration.important - - values << declaration.value - properties_to_delete << property + properties_to_delete = properties.select do |property| + (declaration = declarations[property]) && !declaration.important end - return if values.length <= 1 + return if properties_to_delete.length <= 1 + + values = properties_to_delete.map { |property| declarations[property].value } properties_to_delete.each do |property| declarations.delete(property) @@ -588,15 +584,11 @@ def create_dimensions_shorthand! # :nodoc: return if declarations.size < NUMBER_OF_DIMENSIONS DIMENSIONS.each do |property, dimensions| - values = {} - DIMENSION_DIRECTIONS.each_with_index do |side, index| - next unless (declaration = declarations[dimensions[index]]) - - values[side] = declaration.value - end - # All four dimensions must be present - next if values.size != dimensions.size + next unless dimensions.all? { |dimension| declarations.key?(dimension) } + + top, right, bottom, left = dimensions.map { |dimension| declarations[dimension].value } + values = {top: top, right: right, bottom: bottom, left: left} new_value = values.values_at(*compute_dimensions_shorthand(values)).join(' ').strip declarations[property] = new_value unless new_value.empty? @@ -646,7 +638,7 @@ def create_list_style_shorthand! # :nodoc: def compute_dimensions_shorthand(values) # All four sides are equal, returning single value - return [:top] if values.values.uniq.count == 1 + return [:top] if values[:top] == values[:right] && values[:right] == values[:bottom] && values[:bottom] == values[:left] # `/* top | right | bottom | left */` return DIMENSION_DIRECTIONS if values[:left] != values[:right] @@ -693,7 +685,7 @@ def unmatched_open_parenthesis?(declarations) # TODO: way too simplistic #++ def parse_selectors!(selectors) # :nodoc: - @selectors = selectors.split(',').map do |s| + @selectors = selectors.split(',').map! do |s| s.gsub!(/\s+/, ' ') s.strip! s diff --git a/test/test_css_parser_media_types.rb b/test/test_css_parser_media_types.rb index 87e00e9..c5e2616 100644 --- a/test/test_css_parser_media_types.rb +++ b/test/test_css_parser_media_types.rb @@ -5,6 +5,7 @@ # Test cases for the handling of media types class CssParserMediaTypesTests < Minitest::Test include CssParser + include AllocationCounting def setup @cp = Parser.new @@ -155,4 +156,8 @@ def test_to_s_includes_media_queries @cp.add_rule!(selectors: 'body', block: 'color: black;', media_types: 'aural and (device-aspect-ratio: 16/9)') assert_equal "@media aural and (device-aspect-ratio: 16/9) {\n body {\n color: black;\n }\n}\n", @cp.to_s end + + def test_sanitizing_an_already_sanitized_media_type_does_not_allocate + assert_equal(0, allocations { CssParser.sanitize_media_query(:all) }) + end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 6411176..c39b380 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -7,3 +7,19 @@ require 'net/http' require 'webrick' require 'css_parser' + +module AllocationCounting + def allocations + skip 'GC.stat(:total_allocated_objects) is MRI only' unless RUBY_ENGINE == 'ruby' + + yield + GC.disable + Array.new(3) do + before = GC.stat(:total_allocated_objects) + yield + GC.stat(:total_allocated_objects) - before + end.min + ensure + GC.enable + end +end diff --git a/test/test_rule_set_creating_shorthand.rb b/test/test_rule_set_creating_shorthand.rb index 63262d0..8af4531 100644 --- a/test/test_rule_set_creating_shorthand.rb +++ b/test/test_rule_set_creating_shorthand.rb @@ -5,6 +5,7 @@ # Test cases for reading and generating CSS shorthand properties class RuleSetCreatingShorthandTests < Minitest::Test include CssParser + include AllocationCounting def setup @cp = CssParser::Parser.new @@ -220,6 +221,13 @@ def test_a_single_property_is_not_shorted assert_equal('', combined['background']) end + def test_creating_dimensions_shorthand_allocates_less_than_thirty_objects + block = 'margin-top: 1px; margin-right: 2px; margin-bottom: 1px; margin-left: 2px; padding-top: 0; padding-right: 0; padding-bottom: 0; padding-left: 0' + parsing = allocations { RuleSet.new(block: block) } + + assert_operator allocations { RuleSet.new(block: block).create_shorthand! } - parsing, :<, 30 + end + protected def assert_properties_are_deleted(ruleset, properties)