From acac5c67c35ea1d532edb77f6ba0d6fd22aa2fef Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Thu, 13 Oct 2022 15:32:26 +0200
Subject: [PATCH] Copy process_call implementation from ruby2ruby

This brings it closer to the original, making it easier to see what's
going on. Looking at the git log of ruby2ruby there were no
modifications in this area. The only difference in process_call is that
process_call_receiver is jailed. This implementation actually supports
kwargs and kwsplat.

It also adds tests for kwargs and kwsplat.
---
 lib/safemode/parser.rb       | 51 +++++++++++++++++++++---------------
 test/test_safemode_eval.rb   | 10 +++++++
 test/test_safemode_parser.rb | 22 ++++++++++++++--
 3 files changed, 60 insertions(+), 23 deletions(-)

diff --git a/lib/safemode/parser.rb b/lib/safemode/parser.rb
index 0d298d6..c731bda 100644
--- a/lib/safemode/parser.rb
+++ b/lib/safemode/parser.rb
@@ -36,12 +36,12 @@ def jail(str, parentheses = false, safe_call: false)
 
     # split up #process_call. see below ...
     def process_call(exp, safe_call = false)
-      exp.shift # remove ":call" symbol
-      receiver = jail(process_call_receiver(exp), safe_call: safe_call)
-      name = exp.shift
-      args = process_call_args(exp)
+      _, recv, name, *args = exp
 
-      process_call_code(receiver, name, args, safe_call)
+      receiver = jail(process_call_receiver(recv), safe_call: safe_call)
+      arguments = process_call_args(name, args)
+
+      process_call_code(receiver, name, arguments, safe_call)
     end
 
     def process_fcall(exp)
@@ -143,26 +143,35 @@ def raise_security_error(type, info)
     # split up Ruby2Ruby#process_call monster method so we can hook into it
     # in a more readable manner
 
-    def process_call_receiver(exp)
-      receiver_node_type = exp.first.nil? ? nil : exp.first.first
-      receiver = process exp.shift
-      receiver = "(#{receiver})" if
-        Ruby2Ruby::ASSIGN_NODES.include? receiver_node_type
+    def process_call_receiver(recv)
+      receiver_node_type = recv && recv.sexp_type
+      receiver = process recv
+      receiver = "(#{receiver})" if ASSIGN_NODES.include? receiver_node_type
       receiver
     end
 
-    def process_call_args(exp)
-      args = []
-      while not exp.empty? do
-        args_exp = exp.shift
-        if args_exp && args_exp.first == :array # FIX
-          processed = "#{process(args_exp)[1..-2]}"
-        else
-          processed = process args_exp
-        end
-        args << processed unless (processed.nil? or processed.empty?)
+    def process_call_args(name, args)
+      in_context :arglist do
+        max = args.size - 1
+        args = args.map.with_index { |arg, i|
+          arg_type = arg.sexp_type
+          is_empty_hash = arg == s(:hash)
+          arg = process arg
+
+          next if arg.empty?
+
+          strip_hash = (arg_type == :hash and
+                        not BINARY.include? name and
+                        not is_empty_hash and
+                        (i == max or args[i + 1].sexp_type == :splat))
+          wrap_arg = Ruby2Ruby::ASSIGN_NODES.include? arg_type
+
+          arg = arg[2..-3] if strip_hash
+          arg = "(#{arg})" if wrap_arg
+
+          arg
+        }.compact
       end
-      args
     end
 
     def process_call_code(receiver, name, args, safe_call)
diff --git a/test/test_safemode_eval.rb b/test/test_safemode_eval.rb
index c9a23d9..832fdb3 100644
--- a/test/test_safemode_eval.rb
+++ b/test/test_safemode_eval.rb
@@ -88,6 +88,16 @@ def test_should_not_allow_access_to_bind
     assert_raise_security "self.bind('an arg')"
   end
 
+  def test_sending_of_kwargs_works
+    assert @box.eval("@article.method_with_kwargs(a_keyword: true)", @assigns)
+  end
+
+  def test_sending_to_method_missing
+    assert_raise_with_message(Safemode::NoMethodError, /#no_such_method/) do
+      @box.eval("@article.no_such_method('arg', key: 'value')", @assigns)
+    end
+  end
+
   TestHelper.no_method_error_raising_calls.each do |call|
     call.gsub!('"', '\\\\"')
     class_eval %Q(
diff --git a/test/test_safemode_parser.rb b/test/test_safemode_parser.rb
index 03697d9..f049230 100644
--- a/test/test_safemode_parser.rb
+++ b/test/test_safemode_parser.rb
@@ -25,13 +25,31 @@ def test_ternary_should_be_usable_for_erb
     assert_jailed "if true then\n 1\n else\n2\nend", "true ? 1 : 2"
   end
 
+  def test_call_with_shorthand
+    unsafe = <<~UNSAFE
+      a_keyword = true
+      @article.method_with_kwargs(a_keyword:)
+    UNSAFE
+    jailed = <<~JAILED
+      a_keyword = true
+      @article.to_jail.method_with_kwargs(a_keyword:)
+    JAILED
+    assert_jailed jailed, unsafe
+  end
+
+  def test_call_with_complex_args
+    unsafe = "@article.method_with_kwargs('positional', a_keyword: true, **kwargs)"
+    jailed = "@article.to_jail.method_with_kwargs(\"positional\", :a_keyword => true, **to_jail.kwargs)"
+    assert_jailed jailed, unsafe
+  end
+
   def test_safe_call_simple
     assert_jailed '@article&.to_jail&.method', '@article&.method'
   end
 
   def test_safe_call_with_complex_args
-    unsafe = "@article&.method_with_kwargs('positional')"
-    jailed = "@article&.to_jail&.method_with_kwargs(\"positional\")"
+    unsafe = "@article&.method_with_kwargs('positional', a_keyword: true, **kwargs)"
+    jailed = "@article&.to_jail&.method_with_kwargs(\"positional\", :a_keyword => true, **to_jail.kwargs)"
     assert_jailed jailed, unsafe
   end