From e0621fa8f7301af5ccd056a5d8a38e7d3da27cf4 Mon Sep 17 00:00:00 2001
From: Christian Decker <decker.christian@gmail.com>
Date: Sat, 5 Jan 2019 16:55:03 +0100
Subject: [PATCH] pylightning: Better inject arguments in plugin method calls

If the `request` or `plugin` parameter that are injected by the
framework where before or inbetween positional arguments we'd be
injecting them incorrectly, i.e., we'd be providing them both as
`args` (and mismapping another argument) as well as `kwargs`.

This is a better way to map arguments, which takes advantage of the
fact that JSON-RPC calls are either all positional or named arguments.

I also included a test for various scenarios, that hopefull cover the
most common cases.

Reported-by: Rene Pickhardt <@renepickhardt>
Signed-off-by: Christian Decker <decker.christian@gmail.com>
---
 contrib/pylightning/lightning/plugin.py      | 36 +++++++++++---
 contrib/pylightning/lightning/test_plugin.py | 51 ++++++++++++++++++++
 2 files changed, 79 insertions(+), 8 deletions(-)
 create mode 100644 contrib/pylightning/lightning/test_plugin.py

diff --git a/contrib/pylightning/lightning/plugin.py b/contrib/pylightning/lightning/plugin.py
index 134f9ee1a..447bce508 100644
--- a/contrib/pylightning/lightning/plugin.py
+++ b/contrib/pylightning/lightning/plugin.py
@@ -1,3 +1,5 @@
+from collections import OrderedDict
+
 import sys
 import os
 import json
@@ -131,18 +133,36 @@ class Plugin(object):
 
     def _exec_func(self, func, request):
         params = request['params']
-        args = params.copy() if isinstance(params, list) else []
-        kwargs = params.copy() if isinstance(params, dict) else {}
-
         sig = inspect.signature(func)
 
-        if 'plugin' in sig.parameters:
-            kwargs['plugin'] = self
+        arguments = OrderedDict()
+        for name, value in sig.parameters.items():
+            arguments[name] = inspect.Signature.empty
 
-        if 'request' in sig.parameters:
-            kwargs['request'] = request
+        # Fill in any injected parameters
+        if 'plugin' in arguments:
+            arguments['plugin'] = self
 
-        ba = sig.bind(*args, **kwargs)
+        if 'request' in arguments:
+            arguments['request'] = request
+
+        # Now zip the provided arguments and the prefilled a together
+        if isinstance(params, dict):
+            arguments.update(params)
+        else:
+            pos = 0
+            for k, v in arguments.items():
+                if v is not inspect.Signature.empty:
+                    continue
+                if pos < len(params):
+                    # Apply positional args if we have them
+                    arguments[k] = params[pos]
+                else:
+                    # For the remainder apply default args
+                    arguments[k] = sig.parameters[k].default
+                pos += 1
+
+        ba = sig.bind(**arguments)
         ba.apply_defaults()
         return func(*ba.args, **ba.kwargs)
 
diff --git a/contrib/pylightning/lightning/test_plugin.py b/contrib/pylightning/lightning/test_plugin.py
new file mode 100644
index 000000000..07d2c8ca2
--- /dev/null
+++ b/contrib/pylightning/lightning/test_plugin.py
@@ -0,0 +1,51 @@
+from .plugin import Plugin
+import itertools
+
+
+def test_positional_inject():
+    p = Plugin()
+    rdict = {
+        'id': 1,
+        'jsonrpc':
+        '2.0',
+        'method': 'func',
+        'params': {'a': 1, 'b': 2, 'kwa': 3, 'kwb': 4}
+    }
+    rarr = {
+        'id': 1,
+        'jsonrpc': '2.0',
+        'method': 'func',
+        'params': [1, 2, 3, 4]
+    }
+
+    def pre_args(plugin, a, b, kwa=3, kwb=4):
+        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
+
+    def in_args(a, plugin, b, kwa=3, kwb=4):
+        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
+
+    def post_args(a, b, plugin, kwa=3, kwb=4):
+        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
+
+    def post_kwargs(a, b, kwa=3, kwb=4, plugin=None):
+        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
+
+    def in_multi_args(a, request, plugin, b, kwa=3, kwb=4):
+        assert request in [rarr, rdict]
+        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
+
+    def in_multi_mix_args(a, plugin, b, request=None, kwa=3, kwb=4):
+        assert request in [rarr, rdict]
+        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
+
+    def extra_def_arg(a, b, c, d, e=42):
+        """ Also uses a different name for kwa and kwb
+        """
+        assert (a, b, c, d, e) == (1, 2, 3, 4, 42)
+
+    funcs = [pre_args, in_args, post_args, post_kwargs, in_multi_args]
+
+    for func, request in itertools.product(funcs, [rdict, rarr]):
+        p._exec_func(func, request)
+
+    p._exec_func(extra_def_arg, rarr)