From 448924e2c65313aa9899ce1db473c893c1e1e2f0 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 1/19] Patch xmlrpclib for 64-bit integers Related: https://pagure.io/koji/issue/464 --- diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py index a799ad5..2a77980 100644 --- a/hub/kojixmlrpc.py +++ b/hub/kojixmlrpc.py @@ -60,6 +60,24 @@ class Marshaller(xmlrpclib.Marshaller): self.dump_string(value, write) dispatch[datetime.datetime] = dump_datetime + MAXI8 = 2 ** 64 - 1 + MINI8 = -2 ** 64 + def dump_i8(self, value, write): + # python2's xmlrpclib doesn't support i8 extension for marshalling, + # but can unmarshall it correctly. + if value > Marshaller.MAXI8 or value < Marshaller.MINI8: + raise OverflowError, "long int exceeds XML-RPC limits" + elif value > xmlrpclib.MAXINT or value < xmlrpclib.MININT: + write("") + write(str(int(value))) + write("\n") + else: + write("") + write(str(int(value))) + write("\n") + dispatch[types.LongType] = dump_i8 + dispatch[types.IntType] = dump_i8 + xmlrpclib.Marshaller = Marshaller From ab48fa80b255c1389251a61ab1a8e15e6d21169c Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 2/19] move xmlrpc marshaller into lib and use i8 for large ints --- diff --git a/devtools/fakehub b/devtools/fakehub index fed749d..4e4ba07 100755 --- a/devtools/fakehub +++ b/devtools/fakehub @@ -7,7 +7,6 @@ import os import os.path import pprint import sys -import six.moves.xmlrpc_client from six.moves import cStringIO from six.moves.urllib.parse import quote @@ -58,7 +57,7 @@ def get_request(): else: args.append(nice_literal(s)) args = koji.encode_args(*args, **kwargs) - request = six.moves.xmlrpc_client.dumps(args, method, allow_none=1) + request = koji.dumps(args, method, allow_none=1) return request @@ -68,7 +67,7 @@ def start_response(status, headers): def parse_response(data): - p, u = six.moves.xmlrpc_client.getparser() + p, u = koji.getparser() for chunk in data: p.feed(chunk) p.close() diff --git a/hub/kojihub.py b/hub/kojihub.py index 7e57594..8c355b4 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -559,8 +559,7 @@ def make_task(method, arglist, **opts): raise koji.GenericError("invalid channel policy") # encode xmlrpc request - opts['request'] = xmlrpclib.dumps(tuple(arglist), methodname=method, - allow_none=1) + opts['request'] = koji.dumps(tuple(arglist), methodname=method, allow_none=1) opts['state'] = koji.TASK_STATES['FREE'] opts['method'] = method koji.plugin.run_callbacks('preTaskStateChange', attribute='state', old=None, new='FREE', info=opts) diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py index 2a77980..29c2463 100644 --- a/hub/kojixmlrpc.py +++ b/hub/kojixmlrpc.py @@ -19,18 +19,14 @@ # Mike McLean from ConfigParser import RawConfigParser -import datetime import inspect import logging import os import sys import time import traceback -import types import pprint import resource -import xmlrpclib -from xmlrpclib import getparser, dumps, Fault import koji import koji.auth @@ -38,49 +34,11 @@ import koji.db import koji.plugin import koji.policy import koji.util +# import xmlrpclib functions from koji to use tweaked Marshaller +from koji import getparser, dumps, Fault from koji.context import context -# Workaround to allow xmlrpclib deal with iterators -class Marshaller(xmlrpclib.Marshaller): - - dispatch = xmlrpclib.Marshaller.dispatch.copy() - - def dump_generator(self, value, write): - dump = self.__dump - write("\n") - for v in value: - dump(v, write) - write("\n") - dispatch[types.GeneratorType] = dump_generator - - def dump_datetime(self, value, write): - # For backwards compatibility, we return datetime objects as strings - value = value.isoformat(' ') - self.dump_string(value, write) - dispatch[datetime.datetime] = dump_datetime - - MAXI8 = 2 ** 64 - 1 - MINI8 = -2 ** 64 - def dump_i8(self, value, write): - # python2's xmlrpclib doesn't support i8 extension for marshalling, - # but can unmarshall it correctly. - if value > Marshaller.MAXI8 or value < Marshaller.MINI8: - raise OverflowError, "long int exceeds XML-RPC limits" - elif value > xmlrpclib.MAXINT or value < xmlrpclib.MININT: - write("") - write(str(int(value))) - write("\n") - else: - write("") - write(str(int(value))) - write("\n") - dispatch[types.LongType] = dump_i8 - dispatch[types.IntType] = dump_i8 - -xmlrpclib.Marshaller = Marshaller - - class HandlerRegistry(object): """Track handlers for RPC calls""" diff --git a/koji/__init__.py b/koji/__init__.py index dffe9ed..6e3f1c2 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -73,13 +73,55 @@ import struct import tempfile import time import traceback +import types from . import util import warnings -import six.moves.xmlrpc_client import xml.sax import xml.sax.handler -from six.moves.xmlrpc_client import loads, dumps, Fault import six.moves.urllib +from six.moves.xmlrpc_client import getparser, loads, dumps, Fault + +# Workaround to allow xmlrpclib deal with iterators and 64-bit ints +class Marshaller(six.moves.xmlrpc_client.Marshaller): + + dispatch = six.moves.xmlrpc_client.Marshaller.dispatch.copy() + + def dump_generator(self, value, write): + dump = self.__dump + write("\n") + for v in value: + dump(v, write) + write("\n") + dispatch[types.GeneratorType] = dump_generator + + def dump_datetime(self, value, write): + # For backwards compatibility, we return datetime objects as strings + value = value.isoformat(' ') + self.dump_string(value, write) + dispatch[datetime.datetime] = dump_datetime + + MAXI8 = 2 ** 64 - 1 + MINI8 = -2 ** 64 + def dump_i8(self, value, write): + # python2's xmlrpclib doesn't support i8 extension for marshalling, + # but can unmarshall it correctly. + if value > Marshaller.MAXI8 or value < Marshaller.MINI8: + raise OverflowError, "long int exceeds XML-RPC limits" + elif value > six.moves.xmlrpc_client.MAXINT or \ + value < six.moves.xmlrpc_client.MININT: + write("") + write(str(int(value))) + write("\n") + else: + write("") + write(str(int(value))) + write("\n") + dispatch[types.LongType] = dump_i8 + dispatch[types.IntType] = dump_i8 + +six.moves.xmlrpc_client.Marshaller = Marshaller +six.moves.xmlrpc_client.FastMarshaller = None + PROFILE_MODULES = {} # {module_name: module_instance} @@ -2390,7 +2432,7 @@ class ClientSession(object): return ret def _read_xmlrpc_response(self, response): - p, u = six.moves.xmlrpc_client.getparser() + p, u = getparser() for chunk in response.iter_content(8192): if self.opts.get('debug_xmlrpc', False): print("body: %r" % chunk) diff --git a/koji/daemon.py b/koji/daemon.py index 9706437..ba210a2 100644 --- a/koji/daemon.py +++ b/koji/daemon.py @@ -34,7 +34,6 @@ import time import sys import traceback import errno -import xmlrpclib def incremental_upload(session, fname, fd, path, retries=5, logger=None): @@ -1209,12 +1208,12 @@ class TaskManager(object): try: response = (handler.run(),) # note that we wrap response in a singleton tuple - response = xmlrpclib.dumps(response, methodresponse=1, allow_none=1) + response = koji.dumps(response, methodresponse=1, allow_none=1) self.logger.info("RESPONSE: %r" % response) self.session.host.closeTask(handler.id, response) return - except xmlrpclib.Fault, fault: - response = xmlrpclib.dumps(fault) + except koji.Fault, fault: + response = koji.dumps(fault) tb = ''.join(traceback.format_exception(*sys.exc_info())).replace(r"\n", "\n") self.logger.warn("FAULT:\n%s" % tb) except (SystemExit, koji.tasks.ServerExit, KeyboardInterrupt): @@ -1233,7 +1232,7 @@ class TaskManager(object): if issubclass(e_class, koji.GenericError): #just pass it through tb = str(e) - response = xmlrpclib.dumps(xmlrpclib.Fault(faultCode, tb)) + response = koji.dumps(koji.Fault(faultCode, tb)) # if we get here, then we're handling an exception, so fail the task self.session.host.failTask(handler.id, response) From 2865f2421b357503f4f9bc310f3a98e0a9618e2a Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 3/19] avoid using encode_int in some of the client code --- diff --git a/koji/__init__.py b/koji/__init__.py index 6e3f1c2..9ecdfdf 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2693,7 +2693,7 @@ class ClientSession(object): while True: if debug: self.logger.debug("uploadFile(%r,%r,%r,%r,%r,...)" %(path, name, sz, digest, offset)) - if self.callMethod('uploadFile', path, name, encode_int(sz), digest, encode_int(offset), data, **volopts): + if self.callMethod('uploadFile', path, name, sz, digest, offset, data, **volopts): break if tries <= retries: tries += 1 diff --git a/koji/daemon.py b/koji/daemon.py index ba210a2..71f8bf0 100644 --- a/koji/daemon.py +++ b/koji/daemon.py @@ -60,7 +60,7 @@ def incremental_upload(session, fname, fd, path, retries=5, logger=None): tries = 0 while True: - if session.uploadFile(path, fname, koji.encode_int(size), digest, koji.encode_int(offset), data): + if session.uploadFile(path, fname, size, digest, offset, data): break if tries <= retries: diff --git a/vm/kojivmd b/vm/kojivmd index 1976f7b..5491ee9 100755 --- a/vm/kojivmd +++ b/vm/kojivmd @@ -731,8 +731,8 @@ class VMExecTask(BaseTaskHandler): """ remotepath = os.path.dirname(os.path.join(self.getUploadDir(), filepath)) filename = os.path.basename(filepath) - self.session.uploadFile(remotepath, filename, koji.encode_int(size), - md5sum, koji.encode_int(offset), data) + self.session.uploadFile(remotepath, filename, size, + md5sum, offset, data) def verifyChecksum(self, path, checksum, algo='sha1'): local_path = os.path.abspath(os.path.join(self.output_dir, path)) diff --git a/www/kojiweb/index.py b/www/kojiweb/index.py index caa65a7..237e7d0 100644 --- a/www/kojiweb/index.py +++ b/www/kojiweb/index.py @@ -774,14 +774,13 @@ def getfile(environ, taskID, name, volume='DEFAULT', offset=None, size=None): def _chunk_file(server, environ, taskID, name, offset, size, volume): remaining = size - encode_int = koji.encode_int while True: if remaining <= 0: break chunk_size = 1048576 if remaining < chunk_size: chunk_size = remaining - content = server.downloadTaskOutput(taskID, name, offset=encode_int(offset), size=chunk_size, volume=volume) + content = server.downloadTaskOutput(taskID, name, offset=offset, size=chunk_size, volume=volume) if not content: break yield content From 32e6eedca13b59be3c1814bb926ad8b0e8daa029 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 4/19] docs update for xmlrpc i8 extension --- diff --git a/docs/source/using_the_koji_build_system.rst b/docs/source/using_the_koji_build_system.rst index 9a6ba21..d408f2a 100644 --- a/docs/source/using_the_koji_build_system.rst +++ b/docs/source/using_the_koji_build_system.rst @@ -588,3 +588,6 @@ All features supported by command-line client are also accessible by XMLRPC API. You can get listing of all available calls, arguments and basic help via calling `koji list-api` command. This call will also provide you API extensions provided by plugins in that particular koji instance. + +In addition to xmlrpc standard we are using `i8` extension for large +integers. Datetimes are exchanged as text strings in iso format. From be2fae90ae8c7e697f604d0cae47ab6aa9768558 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 5/19] move custom xmlrpc marshaller to its own file --- diff --git a/devtools/fakehub b/devtools/fakehub index 4e4ba07..ccb5f73 100755 --- a/devtools/fakehub +++ b/devtools/fakehub @@ -14,6 +14,7 @@ sys.path.insert(0, os.getcwd()) sys.path.insert(1, os.path.join(os.getcwd(), 'hub')) import koji import kojixmlrpc +import koji.xmlrpcplus def get_url(environ): @@ -57,7 +58,7 @@ def get_request(): else: args.append(nice_literal(s)) args = koji.encode_args(*args, **kwargs) - request = koji.dumps(args, method, allow_none=1) + request = koji.xmlrpcplus.dumps(args, method, allow_none=1) return request @@ -67,7 +68,7 @@ def start_response(status, headers): def parse_response(data): - p, u = koji.getparser() + p, u = koji.xmlrpcplus.getparser() for chunk in data: p.feed(chunk) p.close() diff --git a/hub/kojihub.py b/hub/kojihub.py index 8c355b4..276f30f 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -55,6 +55,8 @@ import time import types import xmlrpclib import zipfile + +import koji.xmlrpcplus from koji.context import context try: @@ -559,7 +561,7 @@ def make_task(method, arglist, **opts): raise koji.GenericError("invalid channel policy") # encode xmlrpc request - opts['request'] = koji.dumps(tuple(arglist), methodname=method, allow_none=1) + opts['request'] = koji.xmlrpcplus.dumps(tuple(arglist), methodname=method, allow_none=1) opts['state'] = koji.TASK_STATES['FREE'] opts['method'] = method koji.plugin.run_callbacks('preTaskStateChange', attribute='state', old=None, new='FREE', info=opts) diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py index 29c2463..4692a76 100644 --- a/hub/kojixmlrpc.py +++ b/hub/kojixmlrpc.py @@ -35,7 +35,7 @@ import koji.plugin import koji.policy import koji.util # import xmlrpclib functions from koji to use tweaked Marshaller -from koji import getparser, dumps, Fault +from koji.xmlrpcplus import getparser, dumps, Fault from koji.context import context diff --git a/koji/__init__.py b/koji/__init__.py index 9ecdfdf..4d5ea76 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -73,54 +73,12 @@ import struct import tempfile import time import traceback -import types from . import util import warnings import xml.sax import xml.sax.handler import six.moves.urllib -from six.moves.xmlrpc_client import getparser, loads, dumps, Fault - -# Workaround to allow xmlrpclib deal with iterators and 64-bit ints -class Marshaller(six.moves.xmlrpc_client.Marshaller): - - dispatch = six.moves.xmlrpc_client.Marshaller.dispatch.copy() - - def dump_generator(self, value, write): - dump = self.__dump - write("\n") - for v in value: - dump(v, write) - write("\n") - dispatch[types.GeneratorType] = dump_generator - - def dump_datetime(self, value, write): - # For backwards compatibility, we return datetime objects as strings - value = value.isoformat(' ') - self.dump_string(value, write) - dispatch[datetime.datetime] = dump_datetime - - MAXI8 = 2 ** 64 - 1 - MINI8 = -2 ** 64 - def dump_i8(self, value, write): - # python2's xmlrpclib doesn't support i8 extension for marshalling, - # but can unmarshall it correctly. - if value > Marshaller.MAXI8 or value < Marshaller.MINI8: - raise OverflowError, "long int exceeds XML-RPC limits" - elif value > six.moves.xmlrpc_client.MAXINT or \ - value < six.moves.xmlrpc_client.MININT: - write("") - write(str(int(value))) - write("\n") - else: - write("") - write(str(int(value))) - write("\n") - dispatch[types.LongType] = dump_i8 - dispatch[types.IntType] = dump_i8 - -six.moves.xmlrpc_client.Marshaller = Marshaller -six.moves.xmlrpc_client.FastMarshaller = None +from koji.xmlrpcplus import getparser, loads, dumps, Fault PROFILE_MODULES = {} # {module_name: module_instance} diff --git a/koji/daemon.py b/koji/daemon.py index 71f8bf0..1947dd3 100644 --- a/koji/daemon.py +++ b/koji/daemon.py @@ -22,6 +22,7 @@ import koji import koji.tasks +import koji.xmlrpcplus from koji.tasks import safe_rmtree from koji.util import md5_constructor, adler32_constructor, parseStatus, dslice import os @@ -1208,12 +1209,12 @@ class TaskManager(object): try: response = (handler.run(),) # note that we wrap response in a singleton tuple - response = koji.dumps(response, methodresponse=1, allow_none=1) + response = koji.xmlrpcplus.dumps(response, methodresponse=1, allow_none=1) self.logger.info("RESPONSE: %r" % response) self.session.host.closeTask(handler.id, response) return - except koji.Fault, fault: - response = koji.dumps(fault) + except koji.xmlrpcplus.Fault, fault: + response = koji.xmlrpcplus.dumps(fault) tb = ''.join(traceback.format_exception(*sys.exc_info())).replace(r"\n", "\n") self.logger.warn("FAULT:\n%s" % tb) except (SystemExit, koji.tasks.ServerExit, KeyboardInterrupt): @@ -1232,7 +1233,7 @@ class TaskManager(object): if issubclass(e_class, koji.GenericError): #just pass it through tb = str(e) - response = koji.dumps(koji.Fault(faultCode, tb)) + response = koji.xmlrpcplus.dumps(koji.xmlrpcplus.Fault(faultCode, tb)) # if we get here, then we're handling an exception, so fail the task self.session.host.failTask(handler.id, response) diff --git a/koji/xmlrpcplus.py b/koji/xmlrpcplus.py new file mode 100644 index 0000000..9505200 --- /dev/null +++ b/koji/xmlrpcplus.py @@ -0,0 +1,109 @@ +""" +Custom xmlrpc handling for Koji +""" + +import six +import six.moves.xmlrpc_client as xmlrpc_client +import types + + +# duplicate a few values that we need +getparser = xmlrpc_client.getparser +loads = xmlrpc_client.loads +Fault = xmlrpc_client.Fault + + +class ExtendedMarshaller(xmlrpc_client.Marshaller): + + dispatch = xmlrpc_client.Marshaller.dispatch.copy() + + def dump_generator(self, value, write): + dump = self.__dump + write("\n") + for v in value: + dump(v, write) + write("\n") + dispatch[types.GeneratorType] = dump_generator + + MAXI8 = 2 ** 64 - 1 + MINI8 = -2 ** 64 + + def dump_i8(self, value, write): + # python2's xmlrpclib doesn't support i8 extension for marshalling, + # but can unmarshall it correctly. + if (value > self.MAXI8 or value < self.MINI8): + raise OverflowError("long int exceeds XML-RPC limits") + elif (value > xmlrpc_client.MAXINT or + value < xmlrpc_client.MININT): + write("") + write(str(int(value))) + write("\n") + else: + write("") + write(str(int(value))) + write("\n") + dispatch[types.LongType] = dump_i8 + dispatch[types.IntType] = dump_i8 + + # we always want to allow None + def dump_nil(self, value, write): + write("") + dispatch[type(None)] = dump_nil + + +def dumps(params, methodname=None, methodresponse=None, encoding=None, + allow_none=1, marshaller=None): + """encode an xmlrpc request or response + + Differences from the xmlrpclib version: + - allow_none is on by default + - uses our ExtendedMarshaller by default + - option to specify marshaller + """ + + if isinstance(params, Fault): + methodresponse = 1 + elif not isinstance(params, tuple): + raise TypeError('params must be a tuple of Fault instance') + elif methodresponse and len(params) != 1: + raise ValueError('response tuple must be a singleton') + + if not encoding: + encoding = "utf-8" + + if marshaller is not None: + m = marshaller(encoding) + else: + m = ExtendedMarshaller(encoding, allow_none) + + data = m.dumps(params) + + if encoding != "utf-8": + xmlheader = "\n" % str(encoding) + else: + xmlheader = "\n" # utf-8 is default + + # standard XML-RPC wrappings + if methodname: + # a method call + if six.PY2 and isinstance(methodname, six.text_type): + # Do we need this? + methodname = methodname.encode(encoding, 'xmlcharrefreplace') + parts = ( + xmlheader, + "\n" + "", methodname, "\n", + data, + "\n" + ) + elif methodresponse: + # a method response, or a fault structure + parts = ( + xmlheader, + "\n", + data, + "\n" + ) + else: + return data # return as is + return ''.join(parts) From 3d44e974d3c79ed2ad9def9f5218e15aaafaf96f Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 6/19] expand a bit on the xmlrpc extensions in the docs --- diff --git a/docs/source/using_the_koji_build_system.rst b/docs/source/using_the_koji_build_system.rst index d408f2a..a6c2b0a 100644 --- a/docs/source/using_the_koji_build_system.rst +++ b/docs/source/using_the_koji_build_system.rst @@ -589,5 +589,14 @@ API. You can get listing of all available calls, arguments and basic help via calling `koji list-api` command. This call will also provide you API extensions provided by plugins in that particular koji instance. -In addition to xmlrpc standard we are using `i8` extension for large -integers. Datetimes are exchanged as text strings in iso format. +Because of the data Koji routinely deals with, we use the following extensions +to the xmlrpc standard: + + * We use the ``nil`` extension to represent null values (e.g. None in + Python). Koji's library handles this automatically. If you are using a + different library, you may need to explicitly enable this (e.g. enabling + allow_none in Python's own xmlrpc library). + * We represent large integers with the ``i8`` tag. This standard is borrowed + from Apache's `ws-xmlrpc ` + implementation. Python's own xmlrpc library understands this tag, even + thought it will not emit it. From a59dd67765d59aaf68c108136ab3f065c44f7746 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 7/19] hub returns datetime objects as strings --- diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py index 4692a76..7f17e4d 100644 --- a/hub/kojixmlrpc.py +++ b/hub/kojixmlrpc.py @@ -35,10 +35,21 @@ import koji.plugin import koji.policy import koji.util # import xmlrpclib functions from koji to use tweaked Marshaller -from koji.xmlrpcplus import getparser, dumps, Fault +from koji.xmlrpcplus import getparser, dumps, Fault, ExtendedMarshaller from koji.context import context +class Marshaller(ExtendedMarshaller): + + dispatch = xmlrpclib.Marshaller.dispatch.copy() + + def dump_datetime(self, value, write): + # For backwards compatibility, we return datetime objects as strings + value = value.isoformat(' ') + self.dump_string(value, write) + dispatch[datetime.datetime] = dump_datetime + + class HandlerRegistry(object): """Track handlers for RPC calls""" @@ -216,10 +227,10 @@ class ModXMLRPCRequestHandler(object): response = handler(environ) # wrap response in a singleton tuple response = (response,) - response = dumps(response, methodresponse=1, allow_none=1) + response = dumps(response, methodresponse=1, marshaller=Marshaller) except Fault, fault: self.traceback = True - response = dumps(fault) + response = dumps(fault, marshaller=Marshaller) except: self.traceback = True # report exception back to server @@ -243,7 +254,7 @@ class ModXMLRPCRequestHandler(object): else: faultString = "%s: %s" % (e_class, e) self.logger.warning(tb_str) - response = dumps(Fault(faultCode, faultString)) + response = dumps(Fault(faultCode, faultString), marshaller=Marshaller) return response From 6835409a2b4d97cf8a2cdc4f9591174b18e6cf7c Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 8/19] allow_none is the default for our dumps --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 276f30f..439bdda 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -561,7 +561,7 @@ def make_task(method, arglist, **opts): raise koji.GenericError("invalid channel policy") # encode xmlrpc request - opts['request'] = koji.xmlrpcplus.dumps(tuple(arglist), methodname=method, allow_none=1) + opts['request'] = koji.xmlrpcplus.dumps(tuple(arglist), methodname=method) opts['state'] = koji.TASK_STATES['FREE'] opts['method'] = method koji.plugin.run_callbacks('preTaskStateChange', attribute='state', old=None, new='FREE', info=opts) diff --git a/hub/kojixmlrpc.py b/hub/kojixmlrpc.py index 7f17e4d..056680a 100644 --- a/hub/kojixmlrpc.py +++ b/hub/kojixmlrpc.py @@ -19,6 +19,7 @@ # Mike McLean from ConfigParser import RawConfigParser +import datetime import inspect import logging import os @@ -41,7 +42,7 @@ from koji.context import context class Marshaller(ExtendedMarshaller): - dispatch = xmlrpclib.Marshaller.dispatch.copy() + dispatch = ExtendedMarshaller.dispatch.copy() def dump_datetime(self, value, write): # For backwards compatibility, we return datetime objects as strings From 98f8b150aa8f7b97adee6f18ec5a1ada5cea22e8 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 9/19] can't use __dump from parent class --- diff --git a/koji/xmlrpcplus.py b/koji/xmlrpcplus.py index 9505200..31ead4d 100644 --- a/koji/xmlrpcplus.py +++ b/koji/xmlrpcplus.py @@ -17,8 +17,13 @@ class ExtendedMarshaller(xmlrpc_client.Marshaller): dispatch = xmlrpc_client.Marshaller.dispatch.copy() + def _dump(self, value, write): + # Parent class is unfriendly to subclasses :-/ + f = self.dispatch[type(value)] + f(self, value, write) + def dump_generator(self, value, write): - dump = self.__dump + dump = self._dump write("\n") for v in value: dump(v, write) From 2925e1cf934cd2fef4022f46eff64681394815b8 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 10/19] some unit tests for xmlrpcplus --- diff --git a/tests/test_lib/test_xmlrpcplus.py b/tests/test_lib/test_xmlrpcplus.py new file mode 100644 index 0000000..e4d9a64 --- /dev/null +++ b/tests/test_lib/test_xmlrpcplus.py @@ -0,0 +1,62 @@ +import unittest + +import xmlrpclib +from koji import xmlrpcplus + + +class TestDump(unittest.TestCase): + + standard_data = [ + "Hello World", + 5, + 5.5, + None, + True, + False, + [1], + {"a": 1}, + ["fnord"], + {"a": ["b", 1, 2, None], "b": {"c": 1}}, + ] + + def test_standard_data(self): + for value in self.standard_data: + value = (value,) + enc = xmlrpcplus.dumps(value) + _enc = xmlrpclib.dumps(value, allow_none=1) + self.assertEqual(enc, _enc) + params, method = xmlrpclib.loads(enc) + self.assertEqual(params, value) + self.assertEqual(method, None) + + def gendata(self): + for value in self.standard_data: + yield value + + def test_generator(self): + value = (self.gendata(),) + enc = xmlrpcplus.dumps(value) + params, method = xmlrpclib.loads(enc) + expect = (list(self.gendata()),) + self.assertEqual(params, expect) + self.assertEqual(method, None) + + long_data = [ + 2 ** 63, + -(2 ** 63), + [2**n - 1 for n in range(65)], + {"a": [2 ** 63, 5], "b": 2**63+42}, + ] + + def test_i8(self): + for value in self.long_data: + value = (value,) + enc = xmlrpcplus.dumps(value) + params, method = xmlrpclib.loads(enc) + self.assertEqual(params, value) + self.assertEqual(method, None) + + def test_overflow(self): + value = (2**64,) + with self.assertRaises(OverflowError): + xmlrpcplus.dumps(value) From bce7c414aedb5fb3c8a17a1834c4b0df395af26e Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 11/19] more xmlrpcplus unit tests --- diff --git a/tests/test_lib/test_xmlrpcplus.py b/tests/test_lib/test_xmlrpcplus.py index e4d9a64..dd21290 100644 --- a/tests/test_lib/test_xmlrpcplus.py +++ b/tests/test_lib/test_xmlrpcplus.py @@ -19,9 +19,32 @@ class TestDump(unittest.TestCase): {"a": ["b", 1, 2, None], "b": {"c": 1}}, ] - def test_standard_data(self): + def test_call(self): + method = 'my_rpc_method' + for value in self.standard_data: + value = (value, "other arg") + enc = xmlrpcplus.dumps(value, methodname=method) + _enc = xmlrpclib.dumps(value, methodname=method, allow_none=1) + self.assertEqual(enc, _enc) + params, method = xmlrpclib.loads(enc) + self.assertEqual(params, value) + self.assertEqual(method, method) + + def test_response(self): for value in self.standard_data: value = (value,) + enc = xmlrpcplus.dumps(value, methodresponse=1) + _enc = xmlrpclib.dumps(value, methodresponse=1, allow_none=1) + self.assertEqual(enc, _enc) + params, method = xmlrpclib.loads(enc) + self.assertEqual(params, value) + self.assertEqual(method, None) + + def test_just_data(self): + # xmlrpclib supports this case, so I guess we should too + # neither method call nor response + for value in self.standard_data: + value = (value, "foo", "bar") enc = xmlrpcplus.dumps(value) _enc = xmlrpclib.dumps(value, allow_none=1) self.assertEqual(enc, _enc) @@ -35,7 +58,7 @@ class TestDump(unittest.TestCase): def test_generator(self): value = (self.gendata(),) - enc = xmlrpcplus.dumps(value) + enc = xmlrpcplus.dumps(value, methodresponse=1) params, method = xmlrpclib.loads(enc) expect = (list(self.gendata()),) self.assertEqual(params, expect) @@ -51,12 +74,47 @@ class TestDump(unittest.TestCase): def test_i8(self): for value in self.long_data: value = (value,) - enc = xmlrpcplus.dumps(value) + enc = xmlrpcplus.dumps(value, methodresponse=1) params, method = xmlrpclib.loads(enc) self.assertEqual(params, value) self.assertEqual(method, None) + # and as a call + method = "foomethod" + value = tuple(self.long_data) + enc = xmlrpcplus.dumps(value, methodname=method) + params, method = xmlrpclib.loads(enc) + self.assertEqual(params, value) + self.assertEqual(method, method) def test_overflow(self): value = (2**64,) with self.assertRaises(OverflowError): xmlrpcplus.dumps(value) + + def test_fault(self): + code = 1001 + msg = "some useless error" + f1 = xmlrpcplus.Fault(code, msg) + f2 = xmlrpclib.Fault(code, msg) + value = f1 + enc = xmlrpcplus.dumps(value, methodresponse=1) + _enc = xmlrpclib.dumps(value, methodresponse=1, allow_none=1) + self.assertEqual(enc, _enc) + try: + params, method = xmlrpclib.loads(enc) + except xmlrpclib.Fault, e: + self.assertEqual(e.faultCode, code) + self.assertEqual(e.faultString, msg) + else: + raise Exception('Fault not raised') + + def test_badargs(self): + wrong_type = ["a", 0, 0.1, [], {}, True] + for value in wrong_type: + with self.assertRaises(TypeError): + xmlrpcplus.dumps(value, methodname="foo") + # responses much be singletons + value = (1, 2, 3) + with self.assertRaises(ValueError): + xmlrpcplus.dumps(value, methodresponse=1) + From 16046102480021a3e8c2b44ee3607db3c00ed1ca Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 12/19] allow_none is always on for us --- diff --git a/koji/xmlrpcplus.py b/koji/xmlrpcplus.py index 31ead4d..e8d4959 100644 --- a/koji/xmlrpcplus.py +++ b/koji/xmlrpcplus.py @@ -79,7 +79,7 @@ def dumps(params, methodname=None, methodresponse=None, encoding=None, if marshaller is not None: m = marshaller(encoding) else: - m = ExtendedMarshaller(encoding, allow_none) + m = ExtendedMarshaller(encoding) data = m.dumps(params) From 6c1d2e637adee3676ae621b2106df84284478085 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 13/19] use python six to get xmlrpc_client in unit tests --- diff --git a/tests/test_lib/test_xmlrpcplus.py b/tests/test_lib/test_xmlrpcplus.py index dd21290..867a9ad 100644 --- a/tests/test_lib/test_xmlrpcplus.py +++ b/tests/test_lib/test_xmlrpcplus.py @@ -1,6 +1,6 @@ import unittest -import xmlrpclib +from six.moves import xmlrpc_client from koji import xmlrpcplus @@ -24,9 +24,9 @@ class TestDump(unittest.TestCase): for value in self.standard_data: value = (value, "other arg") enc = xmlrpcplus.dumps(value, methodname=method) - _enc = xmlrpclib.dumps(value, methodname=method, allow_none=1) + _enc = xmlrpc_client.dumps(value, methodname=method, allow_none=1) self.assertEqual(enc, _enc) - params, method = xmlrpclib.loads(enc) + params, method = xmlrpc_client.loads(enc) self.assertEqual(params, value) self.assertEqual(method, method) @@ -34,21 +34,21 @@ class TestDump(unittest.TestCase): for value in self.standard_data: value = (value,) enc = xmlrpcplus.dumps(value, methodresponse=1) - _enc = xmlrpclib.dumps(value, methodresponse=1, allow_none=1) + _enc = xmlrpc_client.dumps(value, methodresponse=1, allow_none=1) self.assertEqual(enc, _enc) - params, method = xmlrpclib.loads(enc) + params, method = xmlrpc_client.loads(enc) self.assertEqual(params, value) self.assertEqual(method, None) def test_just_data(self): - # xmlrpclib supports this case, so I guess we should too + # xmlrpc_client supports this case, so I guess we should too # neither method call nor response for value in self.standard_data: value = (value, "foo", "bar") enc = xmlrpcplus.dumps(value) - _enc = xmlrpclib.dumps(value, allow_none=1) + _enc = xmlrpc_client.dumps(value, allow_none=1) self.assertEqual(enc, _enc) - params, method = xmlrpclib.loads(enc) + params, method = xmlrpc_client.loads(enc) self.assertEqual(params, value) self.assertEqual(method, None) @@ -59,7 +59,7 @@ class TestDump(unittest.TestCase): def test_generator(self): value = (self.gendata(),) enc = xmlrpcplus.dumps(value, methodresponse=1) - params, method = xmlrpclib.loads(enc) + params, method = xmlrpc_client.loads(enc) expect = (list(self.gendata()),) self.assertEqual(params, expect) self.assertEqual(method, None) @@ -75,14 +75,14 @@ class TestDump(unittest.TestCase): for value in self.long_data: value = (value,) enc = xmlrpcplus.dumps(value, methodresponse=1) - params, method = xmlrpclib.loads(enc) + params, method = xmlrpc_client.loads(enc) self.assertEqual(params, value) self.assertEqual(method, None) # and as a call method = "foomethod" value = tuple(self.long_data) enc = xmlrpcplus.dumps(value, methodname=method) - params, method = xmlrpclib.loads(enc) + params, method = xmlrpc_client.loads(enc) self.assertEqual(params, value) self.assertEqual(method, method) @@ -95,14 +95,14 @@ class TestDump(unittest.TestCase): code = 1001 msg = "some useless error" f1 = xmlrpcplus.Fault(code, msg) - f2 = xmlrpclib.Fault(code, msg) + f2 = xmlrpc_client.Fault(code, msg) value = f1 enc = xmlrpcplus.dumps(value, methodresponse=1) - _enc = xmlrpclib.dumps(value, methodresponse=1, allow_none=1) + _enc = xmlrpc_client.dumps(value, methodresponse=1, allow_none=1) self.assertEqual(enc, _enc) try: - params, method = xmlrpclib.loads(enc) - except xmlrpclib.Fault, e: + params, method = xmlrpc_client.loads(enc) + except xmlrpc_client.Fault as e: self.assertEqual(e.faultCode, code) self.assertEqual(e.faultString, msg) else: From 4eabd8b7da403faed1a3d7d33e869d5d2f84598a Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 14/19] py3 fixes for xmlrpcplus --- diff --git a/koji/xmlrpcplus.py b/koji/xmlrpcplus.py index e8d4959..7af122d 100644 --- a/koji/xmlrpcplus.py +++ b/koji/xmlrpcplus.py @@ -33,7 +33,7 @@ class ExtendedMarshaller(xmlrpc_client.Marshaller): MAXI8 = 2 ** 64 - 1 MINI8 = -2 ** 64 - def dump_i8(self, value, write): + def dump_int(self, value, write): # python2's xmlrpclib doesn't support i8 extension for marshalling, # but can unmarshall it correctly. if (value > self.MAXI8 or value < self.MINI8): @@ -47,8 +47,7 @@ class ExtendedMarshaller(xmlrpc_client.Marshaller): write("") write(str(int(value))) write("\n") - dispatch[types.LongType] = dump_i8 - dispatch[types.IntType] = dump_i8 + dispatch[int] = dump_int # we always want to allow None def dump_nil(self, value, write): @@ -56,6 +55,11 @@ class ExtendedMarshaller(xmlrpc_client.Marshaller): dispatch[type(None)] = dump_nil +if six.PY2: + ExtendedMarshaller.dispatch[long] = ExtendedMarshaller.dump_int + + + def dumps(params, methodname=None, methodresponse=None, encoding=None, allow_none=1, marshaller=None): """encode an xmlrpc request or response From 5bcaf7cebe6f303f0d8eb66da9ed5a99ccf18bef Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 15/19] include some unicode in unit test data --- diff --git a/tests/test_lib/test_xmlrpcplus.py b/tests/test_lib/test_xmlrpcplus.py index 867a9ad..a11d655 100644 --- a/tests/test_lib/test_xmlrpcplus.py +++ b/tests/test_lib/test_xmlrpcplus.py @@ -1,3 +1,4 @@ +# coding=utf-8 import unittest from six.moves import xmlrpc_client @@ -13,6 +14,7 @@ class TestDump(unittest.TestCase): None, True, False, + u'Hævē s°mə ŭnıčođė', [1], {"a": 1}, ["fnord"], From ae4e4e7b5160a03aac0f5f954cb75e445b8ace7c Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 16/19] unit test corner cases. xmlrpcplus coverage 100% --- diff --git a/tests/test_lib/test_xmlrpcplus.py b/tests/test_lib/test_xmlrpcplus.py index a11d655..d831e45 100644 --- a/tests/test_lib/test_xmlrpcplus.py +++ b/tests/test_lib/test_xmlrpcplus.py @@ -120,3 +120,37 @@ class TestDump(unittest.TestCase): with self.assertRaises(ValueError): xmlrpcplus.dumps(value, methodresponse=1) + def test_marshaller(self): + value = 3.14159 + value = (value,) + enc = xmlrpcplus.dumps(value, methodresponse=1, marshaller=MyMarshaller) + params, method = xmlrpc_client.loads(enc) + # MyMarshaller rounds off floats + self.assertEqual(params, (3,)) + self.assertEqual(method, None) + + def test_encoding(self): + data = [ + 45, + ["hello", "world"], + {"a": 5.5, "b": [None]}, + ] + for value in data: + value = (value,) + enc = xmlrpcplus.dumps(value, methodresponse=1, encoding='us-ascii') + _enc = xmlrpc_client.dumps(value, methodresponse=1, allow_none=1, encoding='us-ascii') + self.assertEqual(enc, _enc) + params, method = xmlrpc_client.loads(enc) + self.assertEqual(params, value) + self.assertEqual(method, None) + + +class MyMarshaller(xmlrpcplus.ExtendedMarshaller): + + dispatch = xmlrpcplus.ExtendedMarshaller.dispatch.copy() + + def dump_float_rounded(self, value, write): + value = int(value) + self.dump_int(value, write) + + dispatch[float] = dump_float_rounded From 2dfbd7d99852f45b1d130ba8e1987e8772ad104b Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 17/19] another unit test --- diff --git a/tests/test_lib/test_xmlrpcplus.py b/tests/test_lib/test_xmlrpcplus.py index d831e45..f5726ba 100644 --- a/tests/test_lib/test_xmlrpcplus.py +++ b/tests/test_lib/test_xmlrpcplus.py @@ -144,6 +144,28 @@ class TestDump(unittest.TestCase): self.assertEqual(params, value) self.assertEqual(method, None) + def test_no_i8(self): + # we shouldn't use i8 if we don't have to + data = [ + 23, + 42, + -1024, + 2 ** 31 - 1, + -2 ** 31, + [2**31 -1], + {"a": -2 ** 31, "b": 3.14}, + ] + for value in data: + value = (value,) + enc = xmlrpcplus.dumps(value, methodresponse=1, encoding='us-ascii') + _enc = xmlrpc_client.dumps(value, methodresponse=1, allow_none=1, encoding='us-ascii') + if 'i8' in enc or 'I8' in enc: + raise Exception('i8 used unnecessarily') + self.assertEqual(enc, _enc) + params, method = xmlrpc_client.loads(enc) + self.assertEqual(params, value) + self.assertEqual(method, None) + class MyMarshaller(xmlrpcplus.ExtendedMarshaller): From cea43dd45898365b10c7ba38afd471df218ee10a Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 18/19] correct range for i8 (64 bit signed) --- diff --git a/koji/xmlrpcplus.py b/koji/xmlrpcplus.py index 7af122d..6a736f4 100644 --- a/koji/xmlrpcplus.py +++ b/koji/xmlrpcplus.py @@ -30,8 +30,8 @@ class ExtendedMarshaller(xmlrpc_client.Marshaller): write("\n") dispatch[types.GeneratorType] = dump_generator - MAXI8 = 2 ** 64 - 1 - MINI8 = -2 ** 64 + MAXI8 = 2 ** 63 - 1 + MINI8 = -2 ** 63 def dump_int(self, value, write): # python2's xmlrpclib doesn't support i8 extension for marshalling, diff --git a/tests/test_lib/test_xmlrpcplus.py b/tests/test_lib/test_xmlrpcplus.py index f5726ba..70aa7f5 100644 --- a/tests/test_lib/test_xmlrpcplus.py +++ b/tests/test_lib/test_xmlrpcplus.py @@ -67,10 +67,10 @@ class TestDump(unittest.TestCase): self.assertEqual(method, None) long_data = [ - 2 ** 63, + 2 ** 63 - 1, -(2 ** 63), - [2**n - 1 for n in range(65)], - {"a": [2 ** 63, 5], "b": 2**63+42}, + [2**n - 1 for n in range(64)], + {"a": [2 ** 63 - 23, 5], "b": 2**63 - 42}, ] def test_i8(self): From a3bc0f6871663866b9f482c0dc678b4d3926fde4 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Aug 25 2017 20:48:02 +0000 Subject: [PATCH 19/19] we need xmlrpcplus in py3 too --- diff --git a/koji/Makefile b/koji/Makefile index 10ac256..d51de8d 100644 --- a/koji/Makefile +++ b/koji/Makefile @@ -2,7 +2,7 @@ PYTHON=python PACKAGE = $(shell basename `pwd`) ifeq ($(PYTHON), python3) # for python3 we fully support only basic library + CLI - PYFILES = __init__.py util.py plugin.py + PYFILES = __init__.py util.py plugin.py xmlrpcplus.py PYSCRIPTS = SUBDIRS = else