kaitai-io / kaitai_struct_java_runtime

Kaitai Struct: runtime for Java
MIT License
42 stars 18 forks source link

Serialization: Always override _io in _write() if supplied #25

Closed generalmimon closed 4 years ago

generalmimon commented 4 years ago

I think this check https://github.com/kaitai-io/kaitai_struct_java_runtime/blob/95c937e03ffecb0b0c0f1687384e4cd9df89f830/src/main/java/io/kaitai/struct/KaitaiStruct.java#L68 in _write() method declaration doesn't make sense. I was simulating this use case: open a file, fetch all seq fields and instances to the KaitaiStruct properties, then close the file because we don't need it anymore, edit the properties using the generated setters and then call r._write(new ByteBufferKaitaiStream(1024 * 1024)) and check if the data in the _io stream matches some expected data in a file. Like this:

ByteBufferKaitaiStream file = new ByteBufferKaitaiStream(io.kaitai.struct.spec.CommonSpec.SRC_DIR + "instance_std_array.bin");

InstanceStdArray r = new InstanceStdArray(file);
r._read();
file.close();
// ... edit the struct ...

struct._check();
KaitaiStream io = new ByteBufferKaitaiStream(1024 * 1024);
struct._write(io);

This is not possible, because the _io can be supplied via _write() method parameter only if it is null. When we do ByteBufferKaitaiStream.close(), the internal property ByteBuffer bb is set to null, so the KaitaiStream will become unusable (no methods will work due to NPE). But we still can't override KaitaiStruct._io property from the public scope, because the _io is not null, it's a regular KaitaiStream, though closed.

GreyCat commented 4 years ago

Seems to make sense. I don't recall the motivation of doing it the way it was done originally, so likely you're absolutely correct.