jangko / msgpack4nim

MessagePack serializer/deserializer implementation for Nim / msgpack.org[Nim]
http://msgpack.org/
119 stars 21 forks source link

Non-generic streams (make MsgStream a proper Stream) #71

Closed snej closed 1 year ago

snej commented 1 year ago

I found a way to make MsgStream a proper subclass of Stream and initialize it correctly. It’s a bit of a kludge but it works; really it’s just a workaround of a design flaw in the streams module.

That means the implementation of the stream API can be removed, since it’s all inherited now.

It also means that all the functions that operate on streams can be regular non-generic procs that take a Stream parameter, instead of needing to make them generic over a ByteStream type.

In doing this I found a place where MsgStream had different semantics from a regular Stream: the readStr function here would raise an IOError if it couldn’t read the entire string; the standard one doesn’t. I think raising an error is better behavior, but it’s nonstandard. I resolved this by defining a new function readExactString(Stream) that has the MsgStream behavior, and changing all calls to readString to readExactString.

Note: this means there is a bug in the current master branch: If StringStream or FileStream is used to unpack, instead of MsgStream, truncated input won’t necessarily be detected. The unpack call might incorrectly return output containing a truncated string, instead of raising an exception as it should.