#571 Support large ints over xmlrpc using i8 tag
Merged by mikem. Opened by mikem.
mikem/koji xmlrpc-large-int  into  master

Download 571.patch

Python's xmlrpc decoder understands the i8 tag that comes from the ws-xmlrpc spec [1], but the encoder will not emit it. Unfortunately, 32 bits is just not enough anymore.

In these changes, Tomas and I have modified the encoder to emit i8 tags when necessary. We were already using a modified encoder in the hub code. Now, we have generalized this and moved it into the library (so that clients can send large ints to the hub too).

We have stopped using encode_int() in client code, allowing clients to send the large ints as i8. Even old hubs will be able to decode these. However, we have kept encode_int() is the few places it was used in the hub for backwards api compatibility.

Most of the new code is in the new koji.xmlrpcplus lib, which has full unit test coverage.

These changes should fix #464

[1] https://ws.apache.org/xmlrpc/types.html

This is a significant change. Reviews are welcome :sweat_smile:

A word about compatibility. As near as I can tell, the only times when this should change the result of a hub call are cases where the call would fail to encode due to an OverflowError.

All python clients should be able to decode these i8 tags. Other clients might not be able to. In such cases, they will encounter a decoding error where before the would have gotten a Fault from the hub due the overflow. So, errors either way.

We have not altered the places where the hub returned large integers as strings as a workaround, so code that expects/relies on this will not fail.

These changes are based on those from #470

@julian8628 @breilly please review

@mikem, Is there any feasibility and reasonableness to keep the old behavior as a version in request/response header?

tested current code on my vm.
It works fine.
:thumbsup:

@mikem, Is there any feasibility and reasonableness to keep the old behavior as a version in request/response header?

It's not something we've done before, and it could actually limit the fix to require the client to alter their request in order to get the new behavior. Remember, that all existing python clients (and possibly others) can already understand the i8 tags.

Note that, in the test_i8 unit test, we are encoding large ints with our new encoder and decoding them with the plain python xmlrpclib decoder.

Also, in several unit tests we assert that the encoded output is identical between our encoder and python's, for data that lacks large ints.

Thanks for the review, @julian8628 !

rebased

Commit 4cec3719 fixes this pull-request

Pull-Request has been merged by mikem@redhat.com

Metadata