nats.net icon indicating copy to clipboard operation
nats.net copied to clipboard

NatsMsg Build should not try to deserialize 0 length payloadBuffer

Open sgwong opened this issue 7 months ago • 2 comments

Proposed change

In the NatsMsg<T> Build function. It should check the payloadBuffer length and only deserialize when its not empty. else, it should return default value.

      if (headers?.Error == null)
        {
            try
            {
                if (payloadBuffer.Length == 0)
                    data = default;
                else
                    data = serializer.Deserialize(payloadBuffer);
            }
            catch (Exception e)
            {
                headers ??= new NatsHeaders();
                headers.Error = new NatsDeserializeException(payloadBuffer.ToArray(), e);
                data = default;
            }
        }

Use case

I am facing error on latest nats with my msgpack deserializer. I am not sure that is changes from new nats server or the nats.net client. I have to add checking for 0 length buffer as below. I think it should check the buffer length before calling the deserialize function. Not sure there is any use case that some serializer need to construct other return value for 0 length buffer instead of default?

public T? Deserialize(in ReadOnlySequence<byte> buffer)
    {
        if (buffer.Length == 0)
            return default;
        return MessagePackSerializer.Deserialize<T>(buffer);
    }

Contribution

The changes are in this proposal. I am not planning to submit PR for this because changes are trivial.

sgwong avatar Jul 01 '24 02:07 sgwong