OpenKMIP / PyKMIP

A Python implementation of the KMIP specification.
Apache License 2.0
272 stars 134 forks source link

Don't forget to update the __repr__ for the various objects. #637

Open KBassford opened 4 years ago

KBassford commented 4 years ago

Hi Peter,

I've been transporting some KMIP objects across DBus and I've encountered a problem that I presume is due to stuff falling by the wayside. For example:

class SymmetricKey(Key):
    def __init__(self, algorithm, length, value, masks=None,
                 name='Symmetric Key', key_wrapping_data=None):

yet below this ...

    def __repr__(self):
        algorithm = "algorithm={0}".format(self.cryptographic_algorithm)
        length = "length={0}".format(self.cryptographic_length)
        value = "value={0}".format(binascii.hexlify(self.value))
        key_wrapping_data = "key_wrapping_data={0}".format(
            self.key_wrapping_data
        )

        return "SymmetricKey({0}, {1}, {2}, {3})".format(
            algorithm,
            length,
            value,
            key_wrapping_data
        )

when it should be ...

    def __repr__(self):
        algorithm = "algorithm={0}".format(self.cryptographic_algorithm)
        length = "length={0}".format(self.cryptographic_length)
        value = "value={0}".format(binascii.hexlify(self.value))
        masks = "masks={0}.format(self.cryptographic_usage_masks)"
        names = "names={0}.format(self.names)"
        key_wrapping_data = "key_wrapping_data={0}".format(
            self.key_wrapping_data
        )

        return "SymmetricKey({0}, {1}, {2}, {3}, {4}, {5})".format(
            algorithm,
            length,
            value,
            masks
            names
            key_wrapping_data
        )

I would do a pull and correct the ones I spot myself, but I'm a little rushed for time at the moment.

PeterHamilton commented 4 years ago

Hi @KBassford, thanks for filing this issue. Yeah, the repr functions are definitely not uniformly up-to-date. The original intent was to support basic serialization to string form as opposed to a full-blown pickle object. However, we've run into various issues in the past successfully re-instantiating objects from their repr form. At this point I'm leaning away from repr and would prefer a different serialization method (like pickle), beyond the existing KMIP binary serialization obviously.

Just a heads up.

KBassford commented 4 years ago

Ouch! I've been using pydbus and one of the most annoying habits is that it tries to eval on variant types, which fails 9 times out of 10. That's why I flattened it to a repr. :(

KBassford commented 4 years ago

Hi Peter,

Folks over here were nice enough to let me work on this, so I forked the project and I am incrementally going through and fixing what I find. First bug fix is already pushed, branch name "bug-637a".