Fix channel's payload.data deserialization in RSocketMachine #277
Conversation
| @@ -784,13 +784,14 @@ class RSocketMachineImpl<D, M> implements RSocketMachine<D, M> { | |||
| } | |||
|
|
|||
| _handleRequestChannel(streamId: number, frame: RequestChannelFrame): void { | |||
| const payload = this._deserializePayload(frame); | |||
There was a problem hiding this comment.
In all _handleXXXX methods, the frame is deserialized first. So in this method also, the payload should be deserialized first, before using it.
| const existingSubscription = this._subscriptions.get(streamId); | ||
| if (existingSubscription) { | ||
| //Likely a duplicate REQUEST_CHANNEL frame, ignore per spec | ||
| return; | ||
| } | ||
|
|
||
| const payloads = new Flowable(subscriber => { | ||
| const payloads = new Flowable<Payload<D, M>>(subscriber => { |
There was a problem hiding this comment.
I believe this bug (bug is explained in another code comment) was introduced mainly due to the missing type of this Flowable. so I added the type now.
| const framesToPayloads = new FlowableProcessor(payloads, frame => | ||
| this._deserializePayload(frame), | ||
| ); | ||
| const framesToPayloads = new FlowableProcessor(payloads); | ||
| this._receivers.set(streamId, framesToPayloads); |
There was a problem hiding this comment.
Bug:
this._receivers is of type Map<number, ISubject<Payload<D, M>>>.
The subject is of type Payload<D, M>. i.e. deserialized payload. The subscribers of this subject can use the payload directly, its already deserliazed. The bug is, in the subscriber the payload is deserialized again and it's causing deserialization errors.
| } | ||
| const data = this._serializers.data.serialize(payload.data); | ||
| const metadata = this._serializers.metadata.serialize(payload.metadata); |
There was a problem hiding this comment.
A minor bug fix: don't try to serialize metadata when metadata is not there. serialize it only when it exists.
Signed-off-by: Palani C <palani@canva.com>
viglucci
added
needs triage
Motivation:
In the current
RSocketMachine._handleRequestChannelimplementation, the payload is deserialized twice. So it leads to deserialization error when channels are used with serializers inRSocketServer.This bug only affects 'channel' interaction model, not other interaction models.
Modifications:
The bug and the fix are explained in the code comments.
This PR only fixes the 0.x branch.
Result:
Added tests to test the deserialization and serialization path of the channel.
Fixes #217