ably / ably-go

Go client library SDK for Ably realtime messaging service
https://ably.com/download
Apache License 2.0
81 stars 31 forks source link

ably-go unnecessarily base64 encodes messages when sending over messagepack #583

Open amnonbc opened 1 year ago

amnonbc commented 1 year ago

See https://github.com/ably/ably-go/blob/6e1a824207c011de54259b3eea12f6aef6618198/ably/proto_message.go#L92-L94

The spec states:

(RSL4c) When using MessagePack Message encoding
(RSL4c1) a binary Message payload is encoded as MessagePack binary type
(RSL4c2) a string Message payload is encoded as MessagePack string type
(RSL4c3) a JSON Message payload is stringified either as a JSON Object or Array and encoded as MessagePack string type and the encoding attribute is set to “json”
(RSL4c4) All messages received will deliver payloads in the format they were sent in i.e. binary, string, or a structured type containing the parsed JSON
(RSL4d) When using JSON Message encoding
(RSL4d1) a binary Message payload is encoded as Base64 and represented as a JSON string the encoding attribute is set to “base64”
(RSL4d2) a string Message payload is represented as a JSON string
(RSL4d3) a JSON Message payload is stringified either as a JSON Object or Array and represented as a JSON string and the encoding attribute is set to “json”
(RSL4d4) All messages received will be decoded based on the encoding field and deliver payloads in the format they were sent in i.e. binary, string, or a structured type containing the parsed JSON

So when we are using JSON message encoding we must Base64 encode binary message payloads (RSL4d1). But when we are sending using MessagePack, we should send a binary payload as MessagePack binary type.

This bug does not prevent messages from being delivered. But the extra base64 encoding creates extra CPU load on the client.

┆Issue is synchronized with this Jira Task by Unito

sync-by-unito[bot] commented 1 year ago

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-3248