Make whitespace filters Unicode-aware

Previously we were only leveraging Ruby's `String#strip` to handle the
logic in these filters but that only covers ASCII whitespace. When
rendering Liquid templates into HTML it would be confusing for these
filters to not strip *all* whitespace.

Additionally, it's helpful when trying to compare two values in, say, a
Liquid conditional.

Gemfile

@@ -32,7 +32,6 @@ group :test do
 end
 
 group :spec do
-  # Using feature branch until https://github.com/Shopify/liquid-spec/pull/144 is merged
-  gem 'liquid-spec', github: 'Shopify/liquid-spec', branch: 'self-drop-env-lookup-specs'
+  gem 'liquid-spec', github: 'Shopify/liquid-spec', branch: 'main'
   gem 'activesupport', require: false
 end

History.md

@@ -1,17 +1,5 @@
 # Liquid Change Log
 
-## 5.13.0
-
-* Add TruffleRuby in CI [Benoit Daloze]
-* Skip slow test raising many exceptions on non-CRuby [Benoit Daloze]
-* Reject bare-bracket syntax in strict2 and introduce `self` keyword by [Alok Swamy]
-* Add strict2_parse to assign and capture tags by [Alok Swamy]
-* Add strict2_parse to increment and decrement tags by [Alok Swamy]
-* Update liquid-spec adapters for `missing_features` [Ian Ker-Seymer]
-* Prevent `SelfDrop` context mutation across render boundaries [Guilherme Carreiro]
-* Fix `SelfDrop` equality [Guilherme Carreiro]
-* Let environment `self` shadow `SelfDrop` [Ian Ker-Seymer]
-
 ## 5.11.0
 * Revert the Inline Snippets tag (#2001), treat its inclusion in the latest Liquid release as a bug, and allow for feedback on RFC#1916 to better support Liquid developers [Guilherme Carreiro]
 * Rename the `:rigid` error mode to `:strict2` and display a warning when users attempt to use the `:rigid` mode [Guilherme Carreiro]

lib/liquid/context.rb

@@ -206,21 +206,16 @@ module Liquid
       # path and find_index() is optimized in MRI to reduce object allocation
       index = @scopes.find_index { |s| s.key?(key) }
 
-      fallback_to_self_drop = key == Expression::SELF && index.nil?
+      # `self` resolves to a SelfDrop (enabling `self['var']` lookups),
+      # but only when it hasn't been explicitly assigned as a local variable.
+      return SelfDrop.new(self) if key == Expression::SELF && !index
 
       variable = if index
         lookup_and_evaluate(@scopes[index], key, raise_on_not_found: raise_on_not_found)
       else
-        try_variable_find_in_environments(
-          key,
-          raise_on_not_found: raise_on_not_found && !fallback_to_self_drop,
-        )
+        try_variable_find_in_environments(key, raise_on_not_found: raise_on_not_found)
       end
 
-      # `self` resolves to a SelfDrop (enabling `self['var']` lookups),
-      # but only after the normal environment lookup doesn't find a value.
-      return @self_drop ||= SelfDrop.new(self) if fallback_to_self_drop && variable.nil?
-
       # update variable's context before invoking #to_liquid
       variable.context = self if variable.respond_to?(:context=)
 

lib/liquid/self_drop.rb

@@ -16,39 +16,23 @@ module Liquid
   #   then the local value takes precedence over the `self` object.
   # @liquid_access global
   class SelfDrop < Drop
-    def initialize(self_context)
+    def initialize(context)
       super()
-      @self_context = self_context
+      @context = context
     end
 
     def [](key)
-      @self_context.find_variable(key)
+      @context.find_variable(key)
     rescue UndefinedVariable
       nil
     end
 
     def key?(key)
-      @self_context.variable_defined?(key)
+      @context.variable_defined?(key)
     end
 
     def to_liquid
       self
     end
-
-    def ==(other)
-      other.is_a?(SelfDrop) && other.self_context.equal?(@self_context)
-    end
-
-    alias_method :eql?, :==
-
-    def hash
-      @self_context.object_id.hash
-    end
-
-    protected
-
-    attr_reader :self_context
-
-    undef context=
   end
 end

lib/liquid/standardfilters.rb

@@ -36,6 +36,19 @@ module Liquid
       %r{<style.*?</style>}m,
     )
     STRIP_HTML_TAGS = /<.*?>/m
+    # Use POSIX whitespace matching so filters handle whitespace beyond Ruby String#strip's ASCII set.
+    WHITESPACE_LEFT = /\A[[:space:]]+/
+    WHITESPACE_RIGHT = /[[:space:]]+\z/
+    WHITESPACE_EDGES = Regexp.union(WHITESPACE_LEFT, WHITESPACE_RIGHT)
+    # Optimized runs regex to find 2 or more [[:space:]] OR a single [[:space:]]
+    # that isn't already `" " `.
+    WHITESPACE_RUNS = /([[:space:]]{2,}|[[[:space:]]&&[^ ]])/
+    private_constant(
+      :WHITESPACE_EDGES,
+      :WHITESPACE_LEFT,
+      :WHITESPACE_RIGHT,
+      :WHITESPACE_RUNS,
+    )
 
     class << self
       def try_coerce_encoding(input, encoding:)
@@ -312,7 +325,7 @@ module Liquid
     def squish(input)
       return if input.nil?
 
-      Utils.to_s(input).strip.gsub(/\s+/, ' ')
+      Utils.to_s(input).gsub(WHITESPACE_RUNS, ' ').strip
     end
 
     # @liquid_public_docs
@@ -324,7 +337,7 @@ module Liquid
     # @liquid_return [string]
     def strip(input)
       input = Utils.to_s(input)
-      input.strip
+      input.gsub(WHITESPACE_EDGES, ' ').strip
     end
 
     # @liquid_public_docs
@@ -336,7 +349,7 @@ module Liquid
     # @liquid_return [string]
     def lstrip(input)
       input = Utils.to_s(input)
-      input.lstrip
+      input.gsub(WHITESPACE_LEFT, ' ').lstrip
     end
 
     # @liquid_public_docs
@@ -348,7 +361,7 @@ module Liquid
     # @liquid_return [string]
     def rstrip(input)
       input = Utils.to_s(input)
-      input.rstrip
+      input.gsub(WHITESPACE_RIGHT, ' ').rstrip
     end
 
     # @liquid_public_docs

lib/liquid/template.rb

@@ -151,10 +151,8 @@ module Liquid
 
         c
       when Liquid::Drop
-        drop = args.shift
-        c    = Context.new([drop, assigns], instance_assigns, registers, @rethrow_errors, @resource_limits, {}, @environment)
-        drop.context = c if drop.respond_to?(:context=)
-        c
+        drop         = args.shift
+        drop.context = Context.new([drop, assigns], instance_assigns, registers, @rethrow_errors, @resource_limits, {}, @environment)
       when Hash
         Context.new([args.shift, assigns], instance_assigns, registers, @rethrow_errors, @resource_limits, {}, @environment)
       when nil

lib/liquid/version.rb

@@ -2,5 +2,5 @@
 # frozen_string_literal: true
 
 module Liquid
-  VERSION = "5.13.0"
+  VERSION = "5.12.0"
 end

test/integration/self_drop_context_test.rb

@@ -1,120 +0,0 @@
-# frozen_string_literal: true
-
-require 'test_helper'
-
-class SelfDropContextTest < Minitest::Test
-  include Liquid
-
-  def test_self_drop_passed_as_render_param_preserves_original_scope
-    source = <<~LIQUID
-      {%- assign var = 42 -%}
-      {%- assign s = self -%}
-      {%- render "snippet1", other_self: s -%}
-    LIQUID
-
-    partials = {
-      'snippet1' => <<~LIQUID,
-        {%- assign var = 43 -%}
-        {{- other_self.var }}|{{ self.var -}}
-      LIQUID
-    }
-
-    assert_template_result('42|43', source, partials: partials)
-  end
-
-  def test_self_drop_in_render_without_passing_resolves_inner_scope
-    source = <<~LIQUID
-      {%- assign var = 42 -%}
-      {%- render "snippet1" -%}
-    LIQUID
-
-    partials = {
-      'snippet1' => <<~LIQUID,
-        {%- assign var = 99 -%}
-        {{- self.var -}}
-      LIQUID
-    }
-
-    assert_template_result('99', source, partials: partials)
-  end
-
-  def test_self_drop_passed_to_nested_renders_preserves_each_level
-    source = <<~LIQUID
-      {%- assign a = 1 -%}
-      {%- assign s1 = self -%}
-      {%- render "snippet1", outer: s1 -%}
-    LIQUID
-
-    partials = {
-      'snippet1' => <<~LIQUID,
-        {%- assign a = 2 -%}
-        {%- assign s2 = self -%}
-        {%- render "snippet2", outer: outer, middle: s2 -%}
-      LIQUID
-      'snippet2' => <<~LIQUID,
-        {%- assign a = 3 -%}
-        {{- outer.a }}|{{ middle.a }}|{{ self.a -}}
-      LIQUID
-    }
-
-    assert_template_result('1|2|3', source, partials: partials)
-  end
-
-  def test_self_drop_reflects_variables_assigned_after_creation
-    source = <<~LIQUID
-      {%- assign s = self -%}
-      {%- assign x = 42 %}{{ s.x -}}
-    LIQUID
-
-    assert_template_result('42', source)
-  end
-
-  def test_self_drop_context_setter_is_undefined
-    context = Context.new
-    drop = SelfDrop.new(context)
-    refute(drop.respond_to?(:context=))
-
-    assert_template_result('42', '{{ self.x }}', { 'x' => 42 })
-  end
-
-  def test_self_drop_repeated_lookups_compare_equal_for_same_context
-    context = Context.new
-    drop = context.find_variable("self")
-    cached_drop = context.find_variable("self")
-
-    assert_same(drop, cached_drop)
-    assert_equal(drop.object_id, cached_drop.object_id)
-    assert_equal(drop, cached_drop)
-  end
-
-  def test_assigned_self_drop_compares_equal_to_itself
-    assert_template_result('T', '{% assign s = self %}{% if s == s %}T{% else %}F{% endif %}')
-  end
-
-  def test_distinct_self_assignments_compare_equal_for_same_context
-    assert_template_result('T', '{% assign a = self %}{% assign b = self %}{% if a == b %}T{% else %}F{% endif %}')
-  end
-
-  def test_bare_self_compares_equal_to_bare_self
-    assert_template_result('T', '{% if self == self %}T{% else %}F{% endif %}')
-  end
-
-  def test_self_drop_with_strict_variables_does_not_raise_for_defined_var
-    t = Template.parse('{{ self.x }}')
-    result = t.render({ 'x' => 42 }, strict_variables: true)
-    assert_equal('42', result)
-  end
-
-  def test_self_drop_with_strict_variables_returns_nil_for_undefined_var
-    t = Template.parse('{{ self.x }}')
-    result = t.render({}, strict_variables: true)
-    assert_equal('', result)
-  end
-
-  def test_self_drop_can_be_passed_as_bare_drop_to_render
-    t = Template.parse('{{ self.x }}')
-    drop = SelfDrop.new(Context.new({ 'x' => 42 }))
-    result = t.render(drop)
-    assert_equal('42', result)
-  end
-end

test/integration/standard_filter_test.rb

@@ -169,6 +169,15 @@ class StandardFiltersTest < Minitest::Test
 \t   boo   " | squish }})).render)
     assert_equal("", Liquid::Template.parse('{{ nil | squish }}').render)
     assert_equal("", Liquid::Template.parse('{{ " " | squish }}').render)
+
+    unicode_spaces = "\u00A0\u202F\u2009\u2007"
+
+    assert_template_result(
+      "foo bar boo",
+      "{{ source | squish }}",
+      { 'source' => "#{unicode_spaces}foo\u202F\u2009bar\t\n\u2007boo#{unicode_spaces}" },
+    )
+    assert_template_result("\u200Bfoo\u200B", "{{ source | squish }}", { 'source' => "\u200Bfoo\u200B" })
   end
 
   def test_escape
@@ -703,16 +712,42 @@ class StandardFiltersTest < Minitest::Test
   def test_strip
     assert_template_result('ab c', "{{ source | strip }}", { 'source' => " ab c  " })
     assert_template_result('ab c', "{{ source | strip }}", { 'source' => " \tab c  \n \t" })
+
+    unicode_spaces = "\u00A0\u202F\u2009\u2007"
+
+    assert_template_result(
+      'ab c',
+      "{{ source | strip }}",
+      { 'source' => "#{unicode_spaces}ab c#{unicode_spaces}" },
+    )
+    assert_template_result("a\u00A0b\u202Fc", "{{ source | strip }}", { 'source' => "a\u00A0b\u202Fc" })
+    assert_template_result("\u200Bfoo\u200B", "{{ source | strip }}", { 'source' => "\u200Bfoo\u200B" })
   end
 
   def test_lstrip
     assert_template_result('ab c  ', "{{ source | lstrip }}", { 'source' => " ab c  " })
     assert_template_result("ab c  \n \t", "{{ source | lstrip }}", { 'source' => " \tab c  \n \t" })
+
+    unicode_spaces = "\u00A0\u202F\u2009\u2007"
+
+    assert_template_result(
+      "ab c#{unicode_spaces}",
+      "{{ source | lstrip }}",
+      { 'source' => "#{unicode_spaces}ab c#{unicode_spaces}" },
+    )
   end
 
   def test_rstrip
     assert_template_result(" ab c", "{{ source | rstrip }}", { 'source' => " ab c  " })
     assert_template_result(" \tab c", "{{ source | rstrip }}", { 'source' => " \tab c  \n \t" })
+
+    unicode_spaces = "\u00A0\u202F\u2009\u2007"
+
+    assert_template_result(
+      "#{unicode_spaces}ab c",
+      "{{ source | rstrip }}",
+      { 'source' => "#{unicode_spaces}ab c#{unicode_spaces}" },
+    )
   end
 
   def test_strip_newlines