From ae0f7a1b5a179fa8f8510a7fc10531da6471fcc7 Mon Sep 17 00:00:00 2001
From: tompng <tomoyapenguin@gmail.com>
Date: Sun, 7 Jun 2026 23:20:15 +0900
Subject: [PATCH] Thread safety Functions

REXML::Functions have a state and is not thread-safe.
Change it to a class, always create a new instance in XPath#match.
`REXML::Functions = REXML::FunctionsClass.new` is kept for untested REXML::Quickpath and for test code.
---
 lib/rexml/functions.rb    | 100 ++++++++++++++++++++------------------
 lib/rexml/quickpath.rb    |   1 -
 lib/rexml/xpath.rb        |   1 -
 lib/rexml/xpath_parser.rb |  61 +++++++++++------------
 4 files changed, 85 insertions(+), 78 deletions(-)

diff --git a/lib/rexml/functions.rb b/lib/rexml/functions.rb
index fe1cd8a4..5cdbf52e 100644
--- a/lib/rexml/functions.rb
+++ b/lib/rexml/functions.rb
@@ -7,11 +7,14 @@ module REXML
   # (2) all method calls from XML will have "-" replaced with "_".
   # Therefore, in XML, "local-name()" is identical (and actually becomes)
   # "local_name()"
-  module Functions
+  class FunctionsClass
     @@available_functions = {}
-    @@context = nil
-    @@namespace_context = {}
-    @@variables = {}
+
+    def initialize
+      @context = nil
+      @namespace_context = {}
+      @variables = {}
+    end
 
     INTERNAL_METHODS = [
       :namespace_context,
@@ -23,54 +26,54 @@ module Functions
       :send,
     ]
     class << self
-      def singleton_method_added(name)
+      def method_added(name)
         unless INTERNAL_METHODS.include?(name)
           @@available_functions[name] = true
         end
       end
     end
 
-    def Functions::namespace_context=(x) ; @@namespace_context=x ; end
-    def Functions::variables=(x) ; @@variables=x ; end
-    def Functions::namespace_context ; @@namespace_context ; end
-    def Functions::variables ; @@variables ; end
+    def namespace_context=(x) ; @namespace_context=x ; end
+    def variables=(x) ; @variables=x ; end
+    def namespace_context ; @namespace_context ; end
+    def variables ; @variables ; end
 
-    def Functions::context=(value); @@context = value; end
+    def context=(value); @context = value; end
 
     # Returns the last node of the given list of nodes.
-    def Functions::last( )
-      @@context[:size]
+    def last( )
+      @context[:size]
     end
 
-    def Functions::position( )
-      @@context[:position]
+    def position( )
+      @context[:position]
     end
 
     # Returns the size of the given list of nodes.
-    def Functions::count( node_set )
+    def count( node_set )
       node_set.size
     end
 
     # Since REXML is non-validating, this method is not implemented as it
     # requires a DTD
-    def Functions::id( object )
+    def id( object )
     end
 
-    def Functions::local_name(node_set=nil)
+    def local_name(node_set=nil)
       get_namespace(node_set) do |node|
         return node.local_name
       end
       ""
     end
 
-    def Functions::namespace_uri( node_set=nil )
+    def namespace_uri( node_set=nil )
       get_namespace( node_set ) do |node|
         return node.namespace
       end
       ""
     end
 
-    def Functions::name( node_set=nil )
+    def name( node_set=nil )
       get_namespace( node_set ) do |node|
         return node.expanded_name
       end
@@ -78,9 +81,9 @@ def Functions::name( node_set=nil )
     end
 
     # Helper method.
-    def Functions::get_namespace( node_set = nil )
+    def get_namespace( node_set = nil )
       if node_set == nil
-        yield @@context[:node] if @@context[:node].respond_to?(:namespace)
+        yield @context[:node] if @context[:node].respond_to?(:namespace)
       else
         if node_set.respond_to? :each
           result = []
@@ -129,7 +132,7 @@ def Functions::get_namespace( node_set = nil )
     #
     # An object of a type other than the four basic types is converted to a
     # string in a way that is dependent on that type.
-    def Functions::string( object=@@context[:node] )
+    def string( object=@context[:node] )
       if object.respond_to?(:node_type)
         case object.node_type
         when :attribute
@@ -169,7 +172,7 @@ def Functions::string( object=@@context[:node] )
     # of each of the children of the node in the
     # node-set that is first in document order.
     # If the node-set is empty, an empty string is returned.
-    def Functions::string_value( o )
+    def string_value( o )
       rv = ""
       o.children.each { |e|
         if e.node_type == :text
@@ -181,7 +184,7 @@ def Functions::string_value( o )
       rv
     end
 
-    def Functions::concat( *objects )
+    def concat( *objects )
       concatenated = ""
       objects.each do |object|
         concatenated << string(object)
@@ -190,17 +193,17 @@ def Functions::concat( *objects )
     end
 
     # Fixed by Mike Stok
-    def Functions::starts_with( string, test )
+    def starts_with( string, test )
       string(string).index(string(test)) == 0
     end
 
     # Fixed by Mike Stok
-    def Functions::contains( string, test )
+    def contains( string, test )
       string(string).include?(string(test))
     end
 
     # Kouhei fixed this
-    def Functions::substring_before( string, test )
+    def substring_before( string, test )
       ruby_string = string(string)
       ruby_index = ruby_string.index(string(test))
       if ruby_index.nil?
@@ -211,7 +214,7 @@ def Functions::substring_before( string, test )
     end
 
     # Kouhei fixed this too
-    def Functions::substring_after( string, test )
+    def substring_after( string, test )
       ruby_string = string(string)
       return $1 if ruby_string =~ /#{test}(.*)/
       ""
@@ -219,7 +222,7 @@ def Functions::substring_after( string, test )
 
     # Take equal portions of Mike Stok and Sean Russell; mix
     # vigorously, and pour into a tall, chilled glass.  Serves 10,000.
-    def Functions::substring( string, start, length=nil )
+    def substring( string, start, length=nil )
       ruby_string = string(string)
       ruby_length = if length.nil?
                       ruby_string.length.to_f
@@ -252,16 +255,16 @@ def Functions::substring( string, start, length=nil )
     end
 
     # UNTESTED
-    def Functions::string_length( string )
+    def string_length( string )
       string(string).length
     end
 
-    def Functions::normalize_space( object=@@context[:node] )
+    def normalize_space( object=@context[:node] )
       string(object).strip.gsub(/\s+/um, ' ')
     end
 
     # This is entirely Mike Stok's beast
-    def Functions::translate( string, tr1, tr2 )
+    def translate( string, tr1, tr2 )
       from = string(tr1)
       to = string(tr2)
 
@@ -303,7 +306,7 @@ def Functions::translate( string, tr1, tr2 )
       end
     end
 
-    def Functions::boolean(object=@@context[:node])
+    def boolean(object=@context[:node])
       case object
       when true, false
         object
@@ -323,24 +326,24 @@ def Functions::boolean(object=@@context[:node])
     end
 
     # UNTESTED
-    def Functions::not( object )
+    def not( object )
       not boolean( object )
     end
 
     # UNTESTED
-    def Functions::true( )
+    def true( )
       true
     end
 
     # UNTESTED
-    def Functions::false(  )
+    def false(  )
       false
     end
 
     # UNTESTED
-    def Functions::lang( language )
+    def lang( language )
       lang = false
-      node = @@context[:node]
+      node = @context[:node]
       attr = nil
       until node.nil?
         if node.node_type == :element
@@ -356,7 +359,7 @@ def Functions::lang( language )
       lang
     end
 
-    def Functions::compare_language lang1, lang2
+    def compare_language lang1, lang2
       lang2.downcase.index(lang1.downcase) == 0
     end
 
@@ -373,7 +376,7 @@ def Functions::compare_language lang1, lang2
     #
     # an object of a type other than the four basic types is converted to a
     # number in a way that is dependent on that type
-    def Functions::number(object=@@context[:node])
+    def number(object=@context[:node])
       case object
       when true
         Float(1)
@@ -394,20 +397,20 @@ def Functions::number(object=@@context[:node])
       end
     end
 
-    def Functions::sum( nodes )
+    def sum( nodes )
       nodes = [nodes] unless nodes.kind_of? Array
       nodes.inject(0) { |r,n| r + number(string(n)) }
     end
 
-    def Functions::floor( number )
+    def floor( number )
       number(number).floor
     end
 
-    def Functions::ceiling( number )
+    def ceiling( number )
       number(number).ceil
     end
 
-    def Functions::round( number )
+    def round( number )
       number = number(number)
       begin
         neg = number.negative?
@@ -418,14 +421,19 @@ def Functions::round( number )
       end
     end
 
-    def Functions::send(name, *args)
+    def send(name, *args)
       if @@available_functions[name.to_sym]
         super
       else
         # TODO: Maybe, this is not XPath spec behavior.
         # This behavior must be reconsidered.
-        XPath.match(@@context[:node], name.to_s)
+        XPath.match(@context[:node], name.to_s)
       end
     end
   end
+
+  # Using this singleton instance may cause thread-safety issues
+  # especially when accessing variables, context and namespace_context.
+  # Consider instantiating your own FunctionsClass object.
+  Functions = FunctionsClass.new
 end
diff --git a/lib/rexml/quickpath.rb b/lib/rexml/quickpath.rb
index cded06f5..41ee7f8f 100644
--- a/lib/rexml/quickpath.rb
+++ b/lib/rexml/quickpath.rb
@@ -4,7 +4,6 @@
 
 module REXML
   class QuickPath
-    include Functions
     include XMLTokens
 
     # A base Hash object to be used when initializing a
diff --git a/lib/rexml/xpath.rb b/lib/rexml/xpath.rb
index eed0300c..82e9df33 100644
--- a/lib/rexml/xpath.rb
+++ b/lib/rexml/xpath.rb
@@ -5,7 +5,6 @@
 module REXML
   # Wrapper class.  Use this class to access the XPath functions.
   class XPath
-    include Functions
     # A base Hash object, supposing to be used when initializing a
     # default empty namespaces set, but is currently unused.
     # TODO: either set the namespaces=EMPTY_HASH, or deprecate this.
diff --git a/lib/rexml/xpath_parser.rb b/lib/rexml/xpath_parser.rb
index b97c672e..170ff7d4 100644
--- a/lib/rexml/xpath_parser.rb
+++ b/lib/rexml/xpath_parser.rb
@@ -62,17 +62,18 @@ def initialize(strict: false)
       @parser = REXML::Parsers::XPathParser.new
       @namespaces = nil
       @variables = {}
+      @functions = FunctionsClass.new
       @nest = 0
       @strict = strict
     end
 
     def namespaces=( namespaces={} )
-      Functions::namespace_context = namespaces
+      @functions.namespace_context = namespaces
       @namespaces = namespaces
     end
 
     def variables=( vars={} )
-      Functions::variables = vars
+      @functions.variables = vars
       @variables = vars
     end
 
@@ -324,21 +325,21 @@ def expr( path_stack, nodeset, context=nil )
 
         when :or
           left = expr(path_stack.shift, nodeset.dup, context)
-          return true if Functions.boolean(left)
+          return true if @functions.boolean(left)
           right = expr(path_stack.shift, nodeset.dup, context)
-          return Functions.boolean(right)
+          return @functions.boolean(right)
 
         when :and
           left = expr(path_stack.shift, nodeset.dup, context)
-          return false unless Functions.boolean(left)
+          return false unless @functions.boolean(left)
           right = expr(path_stack.shift, nodeset.dup, context)
-          return Functions.boolean(right)
+          return @functions.boolean(right)
 
         when :div, :mod, :mult, :plus, :minus
           left = expr(path_stack.shift, nodeset, context)
           right = expr(path_stack.shift, nodeset, context)
-          left = Functions::number(left)
-          right = Functions::number(right)
+          left = @functions.number(left)
+          right = @functions.number(right)
           case op
           when :div
             return left / right
@@ -359,7 +360,7 @@ def expr( path_stack, nodeset, context=nil )
           return (left | right)
         when :neg
           res = expr( path_stack, nodeset, context )
-          return -Functions.number(res)
+          return -@functions.number(res)
         when :not
         when :function
           func_name = path_stack.shift.tr('-','_')
@@ -384,8 +385,8 @@ def expr( path_stack, nodeset, context=nil )
             result = expr(arg, nodeset, target_context)
             result
           end
-          Functions.context = target_context
-          return Functions.send(func_name, *args)
+          @functions.context = target_context
+          return @functions.send(func_name, *args)
         when :group
           sub_expression = path_stack.shift
           result = expr(sub_expression, nodeset, context)
@@ -716,11 +717,11 @@ def norm b
       when true, false
         b
       when 'true', 'false'
-        Functions::boolean( b )
+        @functions.boolean( b )
       when /^\d+(\.\d+)?$/, Numeric
-        Functions::number( b )
+        @functions.number( b )
       else
-        Functions::string( b )
+        @functions.string( b )
       end
     end
 
@@ -732,8 +733,8 @@ def equality_relational_compare(set1, op, set2)
         # result of performing the comparison on the string-values of
         # the two nodes is true.
         set1.product(set2).any? do |node1, node2|
-          node_string1 = Functions.string(node1)
-          node_string2 = Functions.string(node2)
+          node_string1 = @functions.string(node1)
+          node_string2 = @functions.string(node2)
           compare(node_string1, op, node_string2)
         end
       elsif set1.kind_of? Array or set2.kind_of? Array
@@ -754,21 +755,21 @@ def equality_relational_compare(set1, op, set2)
         case b
         when true, false
           a.any? do |node|
-            compare(Functions.boolean(node), op, b)
+            compare(@functions.boolean(node), op, b)
           end
         when Numeric
           a.any? do |node|
-            compare(Functions.number(node), op, b)
+            compare(@functions.number(node), op, b)
           end
         when /\A\d+(\.\d+)?\z/
-          b = Functions.number(b)
+          b = @functions.number(b)
           a.any? do |node|
-            compare(Functions.number(node), op, b)
+            compare(@functions.number(node), op, b)
           end
         else
-          b = Functions::string(b)
+          b = @functions.string(b)
           a.any? do |node|
-            compare(Functions::string(node), op, b)
+            compare(@functions.string(node), op, b)
           end
         end
       else
@@ -802,18 +803,18 @@ def normalize_compare_values(a, operator, b)
       case operator
       when :eq, :neq
         if a_type == :boolean or b_type == :boolean
-          a = Functions.boolean(a) unless a_type == :boolean
-          b = Functions.boolean(b) unless b_type == :boolean
+          a = @functions.boolean(a) unless a_type == :boolean
+          b = @functions.boolean(b) unless b_type == :boolean
         elsif a_type == :number or b_type == :number
-          a = Functions.number(a) unless a_type == :number
-          b = Functions.number(b) unless b_type == :number
+          a = @functions.number(a) unless a_type == :number
+          b = @functions.number(b) unless b_type == :number
         else
-          a = Functions.string(a) unless a_type == :string
-          b = Functions.string(b) unless b_type == :string
+          a = @functions.string(a) unless a_type == :string
+          b = @functions.string(b) unless b_type == :string
         end
       when :lt, :lteq, :gt, :gteq
-        a = Functions.number(a) unless a_type == :number
-        b = Functions.number(b) unless b_type == :number
+        a = @functions.number(a) unless a_type == :number
+        b = @functions.number(b) unless b_type == :number
       else
         message = "[BUG] Unexpected compare operator: " +
           "<#{operator.inspect}>: <#{a.inspect}>: <#{b.inspect}>"