dCache / oncrpc4j

Pure Java implementation of ONCRPC/SUNRPC
Other
30 stars 30 forks source link

null handling for opaque (byte[]) data #18

Open radai-rosenblatt opened 9 years ago

radai-rosenblatt commented 9 years ago

if you leave an opaque value null, you get something like this:

java.lang.NullPointerException
    at org.dcache.xdr.Xdr.xdrEncodeDynamicOpaque(Xdr.java:554)
    at org.dcache.oncrpc4j.rpcgen.Value.xdrEncode(Value.java:33)
    at org.dcache.xdr.RpcCall.acceptedReply(RpcCall.java:238)
    at org.dcache.xdr.RpcCall.reply(RpcCall.java:225)

im not sure if anything can be done, since the specs (rfcs 1014, 1832 and 4056) dont say anything about nulls. and variable-length fields are encoded starting with the length as an unsigned number ...

im opening this just to verify that this should be treated as user error.

radai-rosenblatt commented 9 years ago

note - this does not apply to unions, just "plain" variable length opaque fields

kofemann commented 9 years ago

I think null values for a vector can be treated as a zero length array. The rpcgen code have to be updated as well to generate corresponding encoders.

radai-rosenblatt commented 9 years ago

0 length array and null are not equivalent. the closesnt approximation i've been able to come up with is this:

struct Key {
    opaque data<16>;
};

union Value switch (bool notNull) {
    case TRUE:
        opaque data<1024>;
    case FALSE:
        void;
};

program BLOB_STORAGE {
    version BLOB_STORAGE_VERS {
        void put(Key key, Value value) = 1;
        Value get(Key key) = 2;
    } = 1;
} = 118;

this leaves the Value.data field actually null (but requires Value.notNull = false to work). the issue here is that:

1 oncrpc4j doesnt generate any setters for this at all. so user code ends up looking like:

Value res = new Value();
if (value != null) {
    res.notNull = true;
    res.data = value;
}

2 on top of getters/setters would be nice if oncrpc4j could recognize the boolean flag has no meaning and just have getData()/setData() - that return/accept null and update the flag accordingly. i realize this "optimization" would only work for non-primitive fields covered under a boolean union variable.

i'll open a pull request with the above test scenario

radai-rosenblatt commented 9 years ago

i've opened a pull request https://github.com/dCache/oncrpc4j/pull/19 with the blob store code above (as a test)