From 20a53c7b0037f0024169e55d0fac9c2a6fb52c98 Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Fri, 20 Apr 2018 15:16:51 -0500 Subject: [PATCH 1/5] Support XML-RPC multicall --- tests/unit/legacy/api/xmlrpc/test_xmlrpc.py | 66 +++++++++++++++++++++ warehouse/legacy/api/xmlrpc/views.py | 28 +++++++++ 2 files changed, 94 insertions(+) diff --git a/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py b/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py index 5e94a3782202..c89f82fc7dfa 100644 --- a/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py +++ b/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py @@ -792,3 +792,69 @@ def test_browse(db_request): "Programming Language :: Python", ], )) == {(expected_release.name, expected_release.version)} + + +class TestMulticall: + + def test_multicall(self, monkeypatch): + dumped = pretend.stub(encode=lambda: None) + dumps = pretend.call_recorder(lambda *a, **kw: dumped) + monkeypatch.setattr(xmlrpc.xmlrpc.client, 'dumps', dumps) + + loaded = pretend.stub() + loads = pretend.call_recorder(lambda *a, **kw: loaded) + monkeypatch.setattr(xmlrpc.xmlrpc.client, 'loads', loads) + + subreq = pretend.stub() + blank = pretend.call_recorder(lambda *a, **kw: subreq) + monkeypatch.setattr(xmlrpc.Request, 'blank', blank) + + body = pretend.stub() + response = pretend.stub(body=body) + + invoke_subrequest = pretend.call_recorder(lambda *a, **kw: response) + request = pretend.stub(invoke_subrequest=invoke_subrequest) + + args = [ + {'methodName': 'search', 'params': [{'name': 'foo'}]}, + {'methodName': 'browse', 'params': [{'classifiers': ['bar']}]}, + ] + + responses = xmlrpc.multicall(request, args) + + assert responses == [loaded, loaded] + assert blank.calls == [ + pretend.call('/RPC2', headers={'Content-Type': 'text/xml'}), + pretend.call('/RPC2', headers={'Content-Type': 'text/xml'}), + ] + assert invoke_subrequest.calls == [ + pretend.call(subreq, use_tweens=True), + pretend.call(subreq, use_tweens=True), + ] + assert dumps.calls == [ + pretend.call(({'name': 'foo'},), methodname='search'), + pretend.call(({'classifiers': ['bar']},), methodname='browse'), + ] + assert loads.calls == [pretend.call(body), pretend.call(body)] + + def test_recursive_multicall(self): + request = pretend.stub() + args = [ + {'methodName': 'system.multicall', 'params': []}, + ] + with pytest.raises(xmlrpc.XMLRPCWrappedError) as exc: + xmlrpc.multicall(request, args) + + assert exc.value.faultString == ( + 'ValueError: Cannot use system.multicall inside a multicall' + ) + + def test_missing_multicall_method(self): + request = pretend.stub() + args = [{}] + with pytest.raises(xmlrpc.XMLRPCWrappedError) as exc: + xmlrpc.multicall(request, args) + + assert exc.value.faultString == ( + 'ValueError: Method name not provided' + ) diff --git a/warehouse/legacy/api/xmlrpc/views.py b/warehouse/legacy/api/xmlrpc/views.py index 11151ff16bb5..7c904fbdbc54 100644 --- a/warehouse/legacy/api/xmlrpc/views.py +++ b/warehouse/legacy/api/xmlrpc/views.py @@ -13,10 +13,12 @@ import collections.abc import datetime import functools +import xmlrpc.client import xmlrpc.server from elasticsearch_dsl import Q from packaging.utils import canonicalize_name +from pyramid.request import Request from pyramid.view import view_config from pyramid_rpc.xmlrpc import ( exception_view as _exception_view, xmlrpc_method as _xmlrpc_method @@ -441,3 +443,29 @@ def browse(request, classifiers): ) return [(r.name, r.version) for r in releases] + + +@xmlrpc_method(method='system.multicall') +def multicall(request, args): + if any(arg.get('methodName') == 'system.multicall' for arg in args): + raise XMLRPCWrappedError( + ValueError('Cannot use system.multicall inside a multicall') + ) + + if not all(arg.get('methodName') for arg in args): + raise XMLRPCWrappedError( + ValueError('Method name not provided') + ) + + responses = [] + for arg in args: + name = arg.get('methodName') + subreq = Request.blank('/RPC2', headers={'Content-Type': 'text/xml'}) + subreq.method = 'POST' + subreq.body = xmlrpc.client.dumps( + tuple(arg.get('params')), + methodname=name, + ).encode() + response = request.invoke_subrequest(subreq, use_tweens=True) + responses.append(xmlrpc.client.loads(response.body)) + return responses From f38ae14b86bf9f08b6db67a904f2fa2fe41d5229 Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Fri, 20 Apr 2018 15:38:53 -0500 Subject: [PATCH 2/5] Limit number of multicalls to 20 --- tests/unit/legacy/api/xmlrpc/test_xmlrpc.py | 11 +++++++++++ warehouse/legacy/api/xmlrpc/views.py | 8 +++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py b/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py index c89f82fc7dfa..5b5cfb13ccb2 100644 --- a/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py +++ b/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py @@ -858,3 +858,14 @@ def test_missing_multicall_method(self): assert exc.value.faultString == ( 'ValueError: Method name not provided' ) + + def test_too_many_multicalls_method(self): + request = pretend.stub() + args = [{'methodName': 'nah'}] * 21 + + with pytest.raises(xmlrpc.XMLRPCWrappedError) as exc: + xmlrpc.multicall(request, args) + + assert exc.value.faultString == ( + 'ValueError: Multicall limit is 20 calls' + ) diff --git a/warehouse/legacy/api/xmlrpc/views.py b/warehouse/legacy/api/xmlrpc/views.py index 7c904fbdbc54..0650b8a663cf 100644 --- a/warehouse/legacy/api/xmlrpc/views.py +++ b/warehouse/legacy/api/xmlrpc/views.py @@ -33,6 +33,9 @@ ) +_MAX_MULTICALLS = 20 + + def xmlrpc_method(**kwargs): """ Support multiple endpoints serving the same views by chaining calls to @@ -453,8 +456,11 @@ def multicall(request, args): ) if not all(arg.get('methodName') for arg in args): + raise XMLRPCWrappedError(ValueError('Method name not provided')) + + if len(args) > _MAX_MULTICALLS: raise XMLRPCWrappedError( - ValueError('Method name not provided') + ValueError(f'Multicall limit is {_MAX_MULTICALLS} calls') ) responses = [] From bc95bc774606d9b8ba98b413f88ac0bbdfe6752a Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Mon, 23 Apr 2018 13:54:40 -0500 Subject: [PATCH 3/5] Add metrics for multicall response size --- tests/unit/legacy/api/xmlrpc/test_init.py | 48 +++++++++++++++++++++++ tests/unit/test_config.py | 1 + warehouse/config.py | 3 ++ warehouse/legacy/api/xmlrpc/__init__.py | 17 ++++++++ warehouse/legacy/api/xmlrpc/views.py | 4 ++ 5 files changed, 73 insertions(+) create mode 100644 tests/unit/legacy/api/xmlrpc/test_init.py diff --git a/tests/unit/legacy/api/xmlrpc/test_init.py b/tests/unit/legacy/api/xmlrpc/test_init.py new file mode 100644 index 000000000000..7335ab65417b --- /dev/null +++ b/tests/unit/legacy/api/xmlrpc/test_init.py @@ -0,0 +1,48 @@ +import pretend +from pyramid.events import NewResponse + +from warehouse.legacy.api.xmlrpc import on_new_response, includeme + + +def test_on_new_response_enabled(): + metric_name = pretend.stub() + histogram = pretend.call_recorder(lambda metric_name, value: None) + content_length = pretend.stub() + new_response_event = pretend.stub( + request=pretend.stub( + content_length_metric_name=metric_name, + registry=pretend.stub(datadog=pretend.stub(histogram=histogram)), + ), + response=pretend.stub(content_length=content_length) + ) + + on_new_response(new_response_event) + + assert histogram.calls == [ + pretend.call(metric_name, content_length), + ] + + +def test_on_new_response_disabled(): + histogram = pretend.call_recorder(lambda metric_name, value: None) + new_response_event = pretend.stub( + request=pretend.stub( + registry=pretend.stub(datadog=pretend.stub(histogram=histogram)), + ), + ) + + on_new_response(new_response_event) + + assert histogram.calls == [] + + +def test_includeme(): + config = pretend.stub( + add_subscriber=pretend.call_recorder(lambda subscriber, iface: None), + ) + + includeme(config) + + assert config.add_subscriber.calls == [ + pretend.call(on_new_response, NewResponse), + ] diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index a46fc2283e96..89b6e70cd441 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -402,6 +402,7 @@ def __init__(self): pretend.call("pyramid_retry"), pretend.call("pyramid_tm"), pretend.call("pyramid_services"), + pretend.call(".legacy.api.xmlrpc"), pretend.call(".legacy.api.xmlrpc.cache"), pretend.call("pyramid_rpc.xmlrpc"), pretend.call(".legacy.action_routing"), diff --git a/warehouse/config.py b/warehouse/config.py index 91a56e772cdb..77c908b3e21a 100644 --- a/warehouse/config.py +++ b/warehouse/config.py @@ -377,6 +377,9 @@ def configure(settings=None): # Register support for services config.include("pyramid_services") + # Register our XMLRPC API + config.include(".legacy.api.xmlrpc") + # Register our XMLRPC cache config.include(".legacy.api.xmlrpc.cache") diff --git a/warehouse/legacy/api/xmlrpc/__init__.py b/warehouse/legacy/api/xmlrpc/__init__.py index 36ab810bdee3..c14f7835049d 100644 --- a/warehouse/legacy/api/xmlrpc/__init__.py +++ b/warehouse/legacy/api/xmlrpc/__init__.py @@ -10,4 +10,21 @@ # See the License for the specific language governing permissions and # limitations under the License. +from pyramid.events import NewResponse + import warehouse.legacy.api.xmlrpc.views # noqa + + +def on_new_response(new_response_event): + request = new_response_event.request + if not hasattr(request, 'content_length_metric_name'): + return + + request.registry.datadog.histogram( + request.content_length_metric_name, + new_response_event.response.content_length + ) + + +def includeme(config): + config.add_subscriber(on_new_response, NewResponse) diff --git a/warehouse/legacy/api/xmlrpc/views.py b/warehouse/legacy/api/xmlrpc/views.py index 0650b8a663cf..1528d6ca7ac1 100644 --- a/warehouse/legacy/api/xmlrpc/views.py +++ b/warehouse/legacy/api/xmlrpc/views.py @@ -450,6 +450,10 @@ def browse(request, classifiers): @xmlrpc_method(method='system.multicall') def multicall(request, args): + request.content_length_metric_name = ( + 'warehouse.xmlrpc.system.multicall.content_length' + ) + if any(arg.get('methodName') == 'system.multicall' for arg in args): raise XMLRPCWrappedError( ValueError('Cannot use system.multicall inside a multicall') From d59763fbbba587e14c7d8448e41165a4d049bab1 Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Tue, 24 Apr 2018 09:24:55 -0500 Subject: [PATCH 4/5] Add license header --- tests/unit/legacy/api/xmlrpc/test_init.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/unit/legacy/api/xmlrpc/test_init.py b/tests/unit/legacy/api/xmlrpc/test_init.py index 7335ab65417b..3598598680c9 100644 --- a/tests/unit/legacy/api/xmlrpc/test_init.py +++ b/tests/unit/legacy/api/xmlrpc/test_init.py @@ -1,3 +1,15 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + import pretend from pyramid.events import NewResponse From e4588946c105747003d708ae8b5110476e8f220b Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Tue, 24 Apr 2018 10:42:47 -0500 Subject: [PATCH 5/5] Use a response callback instead --- tests/unit/legacy/api/xmlrpc/test_init.py | 60 --------------------- tests/unit/legacy/api/xmlrpc/test_xmlrpc.py | 38 +++++++++++-- tests/unit/test_config.py | 1 - warehouse/config.py | 3 -- warehouse/legacy/api/xmlrpc/__init__.py | 17 ------ warehouse/legacy/api/xmlrpc/views.py | 21 ++++++-- 6 files changed, 52 insertions(+), 88 deletions(-) delete mode 100644 tests/unit/legacy/api/xmlrpc/test_init.py diff --git a/tests/unit/legacy/api/xmlrpc/test_init.py b/tests/unit/legacy/api/xmlrpc/test_init.py deleted file mode 100644 index 3598598680c9..000000000000 --- a/tests/unit/legacy/api/xmlrpc/test_init.py +++ /dev/null @@ -1,60 +0,0 @@ -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import pretend -from pyramid.events import NewResponse - -from warehouse.legacy.api.xmlrpc import on_new_response, includeme - - -def test_on_new_response_enabled(): - metric_name = pretend.stub() - histogram = pretend.call_recorder(lambda metric_name, value: None) - content_length = pretend.stub() - new_response_event = pretend.stub( - request=pretend.stub( - content_length_metric_name=metric_name, - registry=pretend.stub(datadog=pretend.stub(histogram=histogram)), - ), - response=pretend.stub(content_length=content_length) - ) - - on_new_response(new_response_event) - - assert histogram.calls == [ - pretend.call(metric_name, content_length), - ] - - -def test_on_new_response_disabled(): - histogram = pretend.call_recorder(lambda metric_name, value: None) - new_response_event = pretend.stub( - request=pretend.stub( - registry=pretend.stub(datadog=pretend.stub(histogram=histogram)), - ), - ) - - on_new_response(new_response_event) - - assert histogram.calls == [] - - -def test_includeme(): - config = pretend.stub( - add_subscriber=pretend.call_recorder(lambda subscriber, iface: None), - ) - - includeme(config) - - assert config.add_subscriber.calls == [ - pretend.call(on_new_response, NewResponse), - ] diff --git a/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py b/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py index 5b5cfb13ccb2..6593a833dfae 100644 --- a/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py +++ b/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py @@ -812,8 +812,18 @@ def test_multicall(self, monkeypatch): body = pretend.stub() response = pretend.stub(body=body) - invoke_subrequest = pretend.call_recorder(lambda *a, **kw: response) - request = pretend.stub(invoke_subrequest=invoke_subrequest) + request = pretend.stub( + invoke_subrequest=pretend.call_recorder(lambda *a, **kw: response), + add_response_callback=pretend.call_recorder( + lambda *a, **kw: response), + ) + + callback = pretend.stub() + monkeypatch.setattr( + xmlrpc, + 'measure_response_content_length', + pretend.call_recorder(lambda metric_name: callback) + ) args = [ {'methodName': 'search', 'params': [{'name': 'foo'}]}, @@ -827,10 +837,13 @@ def test_multicall(self, monkeypatch): pretend.call('/RPC2', headers={'Content-Type': 'text/xml'}), pretend.call('/RPC2', headers={'Content-Type': 'text/xml'}), ] - assert invoke_subrequest.calls == [ + assert request.invoke_subrequest.calls == [ pretend.call(subreq, use_tweens=True), pretend.call(subreq, use_tweens=True), ] + assert request.add_response_callback.calls == [ + pretend.call(callback), + ] assert dumps.calls == [ pretend.call(({'name': 'foo'},), methodname='search'), pretend.call(({'classifiers': ['bar']},), methodname='browse'), @@ -869,3 +882,22 @@ def test_too_many_multicalls_method(self): assert exc.value.faultString == ( 'ValueError: Multicall limit is 20 calls' ) + + def test_measure_response_content_length(self): + metric_name = 'some_metric_name' + callback = xmlrpc.measure_response_content_length(metric_name) + + request = pretend.stub( + registry=pretend.stub( + datadog=pretend.stub( + histogram=pretend.call_recorder(lambda *a: None) + ) + ) + ) + response = pretend.stub(content_length=pretend.stub()) + + callback(request, response) + + assert request.registry.datadog.histogram.calls == [ + pretend.call(metric_name, response.content_length), + ] diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 89b6e70cd441..a46fc2283e96 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -402,7 +402,6 @@ def __init__(self): pretend.call("pyramid_retry"), pretend.call("pyramid_tm"), pretend.call("pyramid_services"), - pretend.call(".legacy.api.xmlrpc"), pretend.call(".legacy.api.xmlrpc.cache"), pretend.call("pyramid_rpc.xmlrpc"), pretend.call(".legacy.action_routing"), diff --git a/warehouse/config.py b/warehouse/config.py index 77c908b3e21a..91a56e772cdb 100644 --- a/warehouse/config.py +++ b/warehouse/config.py @@ -377,9 +377,6 @@ def configure(settings=None): # Register support for services config.include("pyramid_services") - # Register our XMLRPC API - config.include(".legacy.api.xmlrpc") - # Register our XMLRPC cache config.include(".legacy.api.xmlrpc.cache") diff --git a/warehouse/legacy/api/xmlrpc/__init__.py b/warehouse/legacy/api/xmlrpc/__init__.py index c14f7835049d..36ab810bdee3 100644 --- a/warehouse/legacy/api/xmlrpc/__init__.py +++ b/warehouse/legacy/api/xmlrpc/__init__.py @@ -10,21 +10,4 @@ # See the License for the specific language governing permissions and # limitations under the License. -from pyramid.events import NewResponse - import warehouse.legacy.api.xmlrpc.views # noqa - - -def on_new_response(new_response_event): - request = new_response_event.request - if not hasattr(request, 'content_length_metric_name'): - return - - request.registry.datadog.histogram( - request.content_length_metric_name, - new_response_event.response.content_length - ) - - -def includeme(config): - config.add_subscriber(on_new_response, NewResponse) diff --git a/warehouse/legacy/api/xmlrpc/views.py b/warehouse/legacy/api/xmlrpc/views.py index 1528d6ca7ac1..99ac8e5c8a61 100644 --- a/warehouse/legacy/api/xmlrpc/views.py +++ b/warehouse/legacy/api/xmlrpc/views.py @@ -448,12 +448,18 @@ def browse(request, classifiers): return [(r.name, r.version) for r in releases] +def measure_response_content_length(metric_name): + + def _callback(request, response): + request.registry.datadog.histogram( + metric_name, response.content_length + ) + + return _callback + + @xmlrpc_method(method='system.multicall') def multicall(request, args): - request.content_length_metric_name = ( - 'warehouse.xmlrpc.system.multicall.content_length' - ) - if any(arg.get('methodName') == 'system.multicall' for arg in args): raise XMLRPCWrappedError( ValueError('Cannot use system.multicall inside a multicall') @@ -478,4 +484,11 @@ def multicall(request, args): ).encode() response = request.invoke_subrequest(subreq, use_tweens=True) responses.append(xmlrpc.client.loads(response.body)) + + request.add_response_callback( + measure_response_content_length( + 'warehouse.xmlrpc.system.multicall.content_length' + ) + ) + return responses