search

mcollina / msgpack5

A msgpack v5 implementation for node.js, with extension points / msgpack.org[Node]
MIT License
492 stars 76 forks source link

nil evaluation error #67

Open i-kovalyov opened 6 years ago

i-kovalyov commented 6 years ago

I founded that after decoding of 0xc0 byte (nil) from msgpack stream msgpack5 decides that stream is finished and then produces error.

I tried to replace text below from function tryDecode of decoder.js case 0xc0: return buildDecodeResult(null, 1) with case 0xc0: return buildDecodeResult(undefined, 1)

This correction fixed error.

However mapping msgpack nil to "undefined" don't looks as best solution.

I tried to fix error another way so, that 0xc0 (nil) maps to null but it's not easy. Null evaluation as end of stream is hardcoded in readable-stream. function readableAddChunk of _stream_readable.js has following code: ... if (chunk === null) { state.reading = false; onEofChunk(stream, state); } ...

Any ideas how to fix this error?

mcollina commented 6 years ago

I think we should either allow to map nil to our own msgpack.Nil object, or output the result of tryDecode in tne decoder (via an option).

i-kovalyov commented 6 years ago

Added pull request fixing this error https://github.com/mcollina/msgpack5/pull/68

Msgpack nil is still mapped to null but during stream processing null temprarily replaced to undefined then replaced back to null.

Is this solution appropriate?