irmen / Pyrolite

Java and .NET client interface for Pyro5 protocol
MIT License
178 stars 47 forks source link

java: Unpickler should handle `Opcodes.OBJ` #49

Closed vruusmann closed 7 years ago

vruusmann commented 7 years ago

Came across this issue when unpickling "callable class" instances.

By observing Python2.7 and Python 3.4 pickle.py files, I've managed to provide a temporary handler like this:

Unpickler unpickler = new Unpickler(){

    @Override
    protected Object dispatch(short key) throws IOException {
        switch(key){
            case Opcodes.OBJ:
                return load_obj();
            default:
                return super.dispatch(key);
        }
    }

    private Object load_obj(){
        List<Object> args = super.stack.pop_all_since_marker();
        IObjectConstructor constructor = (IObjectConstructor)args.get(0);
        args = args.subList(1, args.size());
        Object object = constructor.construct(args.toArray());
        super.stack.add(object);

        return noReturnValue();
    }
};
vruusmann commented 7 years ago

Cannot reproduce a standalone test case for the OBJ opcode. But the following Python script demonstrates a similar error condition in relation to the INST opcode:

import pickle

class Demo:
    def __call__(self):
        return "Me!"

demo = Demo()
print(demo())

con = open("demo.pkl", "wb")
pickle.dump(demo, con)
con.close()

Here's the stack trace:

Exception in thread "main" net.razorvine.pickle.InvalidOpcodeException: opcode not implemented: INST
        at net.razorvine.pickle.Unpickler.dispatch(Unpickler.java:218)
        at org.jpmml.sklearn.PickleUtil$1.dispatch(PickleUtil.java:146)
        at net.razorvine.pickle.Unpickler.load(Unpickler.java:100)
        at org.jpmml.sklearn.PickleUtil.unpickle(PickleUtil.java:198)
        at org.jpmml.sklearn.Main.run(Main.java:111)

This exception occurs when the above Python script is executed using Python 2.7(.11). However, everything works fine (ie. the INST opcode is not around) when it is executed using Python 3.4(.3).

irmen commented 7 years ago

Hmm, that's unfortunate I hoped to never encounter those opcodes in the wild. Look in the dispatch method of the unpickler for a list of currently unimplemented opcodes:

I'll have to investigate the working of the INST and OBJ opcodes and see if/how that can be translated to the Java (and .NET) library.

irmen commented 7 years ago

PERSID, BINPERSID, INST and OBJ have been implemented. EXT1/2/4 will not be implemented at this time.

irmen commented 7 years ago

@vruusmann thanks for the temporary handler you provided, it served as a nice basis. Can you verify that the new implementation is working for you before I create a new official release?

vruusmann commented 7 years ago

@irmen I can confirm that Pyrolite 4.17-SNAPSHOT addresses my current needs completely - there is no need for subclassing Unpickler anymore, and my test suite (involves unpickling ~100 rather complex data structures) passes cleanly.

Never had any problems with Pyrolite. Awesome work!

irmen commented 7 years ago

new version 4.17 has been pushed