Repository: marshmallow-code/apispec
Pull request #704 (2021-08-10 19:23:38)

Ensure plugin helpers get component copies
This PR passes the copied structure rather than the original to the helpers. This avoids issues if the plugin helper mutates the component.

It also uses deepcopies, to be safer. It might be a bit overkill but the cost is low anyway. This could bring issues if someone passed non-deepcopiable stuff to a custom plugin, but this issue is not really new as we already deepcopy in ``APISpec.path``.


Patch:
diff --git a/src/apispec/core.py b/src/apispec/core.py
index 21e9bb0..ee14aee 100644
--- a/src/apispec/core.py
+++ b/src/apispec/core.py
@@ -125,14 +125,11 @@ class Components:
             raise DuplicateComponentNameError(
                 f'Another schema with name "{component_id}" is already registered.'
             )
-        component = component or {}
-        ret = component.copy()
+        ret = deepcopy(component) or {}
         # Execute all helpers from plugins
         for plugin in self._plugins:
             try:
-                ret.update(
-                    plugin.schema_helper(component_id, component, **kwargs) or {}
-                )
+                ret.update(plugin.schema_helper(component_id, ret, **kwargs) or {})
             except PluginMethodNotImplementedError:
                 continue
         self._resolve_refs_in_schema(ret)
@@ -151,12 +148,11 @@ class Components:
             raise DuplicateComponentNameError(
                 f'Another response with name "{component_id}" is already registered.'
             )
-        component = component or {}
-        ret = component.copy()
+        ret = deepcopy(component) or {}
         # Execute all helpers from plugins
         for plugin in self._plugins:
             try:
-                ret.update(plugin.response_helper(component, **kwargs) or {})
+                ret.update(plugin.response_helper(ret, **kwargs) or {})
             except PluginMethodNotImplementedError:
                 continue
         self._resolve_refs_in_response(ret)
@@ -178,8 +174,7 @@ class Components:
             raise DuplicateComponentNameError(
                 f'Another parameter with name "{component_id}" is already registered.'
             )
-        component = component or {}
-        ret = component.copy()
+        ret = deepcopy(component) or {}
         ret.setdefault("name", component_id)
         ret["in"] = location
 
@@ -190,7 +185,7 @@ class Components:
         # Execute all helpers from plugins
         for plugin in self._plugins:
             try:
-                ret.update(plugin.parameter_helper(component, **kwargs) or {})
+                ret.update(plugin.parameter_helper(ret, **kwargs) or {})
             except PluginMethodNotImplementedError:
                 continue
         self._resolve_refs_in_parameter_or_header(ret)
@@ -207,8 +202,7 @@ class Components:
 
         https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.1.md#headerObject
         """
-        component = component or {}
-        ret = component.copy()
+        ret = deepcopy(component) or {}
         if component_id in self.headers:
             raise DuplicateComponentNameError(
                 f'Another header with name "{component_id}" is already registered.'
@@ -216,7 +210,7 @@ class Components:
         # Execute all helpers from plugins
         for plugin in self._plugins:
             try:
-                ret.update(plugin.header_helper(component, **kwargs) or {})
+                ret.update(plugin.header_helper(ret, **kwargs) or {})
             except PluginMethodNotImplementedError:
                 continue
         self._resolve_refs_in_parameter_or_header(ret)


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index 763029f..e1c335a 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -1028,19 +1028,29 @@ class TestPlugins:
     @staticmethod
     def test_plugin_factory(return_none=False):
         class TestPlugin(BasePlugin):
+            """Test Plugin
+
+            return_none allows to check plugin helpers returning ``None``
+            Inputs are mutated to allow testing only a copy is passed.
+            """
+
             def schema_helper(self, name, definition, **kwargs):
+                definition.pop("dummy", None)
                 if not return_none:
                     return {"properties": {"name": {"type": "string"}}}
 
             def parameter_helper(self, parameter, **kwargs):
+                parameter.pop("dummy", None)
                 if not return_none:
                     return {"description": "some parameter"}
 
             def response_helper(self, response, **kwargs):
+                response.pop("dummy", None)
                 if not return_none:
                     return {"description": "42"}
 
             def header_helper(self, header, **kwargs):
+                header.pop("dummy", None)
                 if not return_none:
                     return {"description": "some header"}
 
@@ -1066,12 +1076,15 @@ class TestPlugins:
             openapi_version=openapi_version,
             plugins=(self.test_plugin_factory(return_none),),
         )
-        spec.components.schema("Pet")
+        schema = {"dummy": "dummy"}
+        spec.components.schema("Pet", schema)
         definitions = get_schemas(spec)
         if return_none:
             assert definitions["Pet"] == {}
         else:
             assert definitions["Pet"] == {"properties": {"name": {"type": "string"}}}
+        # Check original schema is not modified
+        assert schema == {"dummy": "dummy"}
 
     @pytest.mark.parametrize("openapi_version", ("2.0", "3.0.0"))
     @pytest.mark.parametrize("return_none", (True, False))
@@ -1082,7 +1095,8 @@ class TestPlugins:
             openapi_version=openapi_version,
             plugins=(self.test_plugin_factory(return_none),),
         )
-        spec.components.parameter("Pet", "body", {})
+        parameter = {"dummy": "dummy"}
+        spec.components.parameter("Pet", "body", parameter)
         parameters = get_parameters(spec)
         if return_none:
             assert parameters["Pet"] == {"in": "body", "name": "Pet"}
@@ -1092,6 +1106,8 @@ class TestPlugins:
                 "name": "Pet",
                 "description": "some parameter",
             }
+        # Check original parameter is not modified
+        assert parameter == {"dummy": "dummy"}
 
     @pytest.mark.parametrize("openapi_version", ("2.0", "3.0.0"))
     @pytest.mark.parametrize("return_none", (True, False))
@@ -1102,12 +1118,15 @@ class TestPlugins:
             openapi_version=openapi_version,
             plugins=(self.test_plugin_factory(return_none),),
         )
-        spec.components.response("Pet", {})
+        response = {"dummy": "dummy"}
+        spec.components.response("Pet", response)
         responses = get_responses(spec)
         if return_none:
             assert responses["Pet"] == {}
         else:
             assert responses["Pet"] == {"description": "42"}
+        # Check original response is not modified
+        assert response == {"dummy": "dummy"}
 
     @pytest.mark.parametrize("openapi_version", ("3.0.0",))
     @pytest.mark.parametrize("return_none", (True, False))
@@ -1118,7 +1137,8 @@ class TestPlugins:
             openapi_version=openapi_version,
             plugins=(self.test_plugin_factory(return_none),),
         )
-        spec.components.header("Pet", {})
+        header = {"dummy": "dummy"}
+        spec.components.header("Pet", header)
         headers = get_headers(spec)
         if return_none:
             assert headers["Pet"] == {}
@@ -1126,6 +1146,8 @@ class TestPlugins:
             assert headers["Pet"] == {
                 "description": "some header",
             }
+        # Check original header is not modified
+        assert header == {"dummy": "dummy"}
 
     @pytest.mark.parametrize("openapi_version", ("2.0", "3.0.0"))
     @pytest.mark.parametrize("return_none", (True, False))