Skip to content

Message/PresenceMessage/Annotation size #331

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sacOO7 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Message/PresenceMessage/Annotation size #331

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5419c41
1. Updated spec for calculating Message size for @name@ and @clientId@
sacOO7 Jun 11, 2025
7fdfeb3
Updated spec for calculating Annotation, Presence, ObjectData message…
sacOO7 Jun 13, 2025
3ed8569
Updated message size spec as per review comments
sacOO7 Jun 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion textile/features.textile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1454,6 +1454,8 @@ h4. Message
** @(TM6b)@ The size of an @Object@ or @Array@ @data@ property is its string length after being JSON-stringified
** @(TM6c)@ The size of a @binary@ @data@ property is its size in bytes (of the actual binary, not the base64 representation, regardless of whether the binary protocol is in use)
** @(TM6f)@ The size of a @string@ @data@ property is its length
Copy link
Copy Markdown
Collaborator Author

@sacOO7 sacOO7 Jun 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If data has type boolean or number then what should be it's size?

Copy link
Copy Markdown
Collaborator Author

@sacOO7 sacOO7 Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same will be valid for PresenceMessage and Annotation

Copy link
Copy Markdown
Collaborator Author

@sacOO7 sacOO7 Jun 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, couldn't find ably-js impl. for calculating PresenceMessage size, seems it's handled in a generic way as a part of
message.ts#getMessageSize

function getMessageSize(msg: WireMessage) {
  let size = 0;
  if (msg.name) {
    size += msg.name.length;
  }
  if (msg.clientId) {
    size += msg.clientId.length;
  }
  if (msg.extras) {
    size += JSON.stringify(msg.extras).length;
  }
  if (msg.data) {
    size += Utils.dataSizeBytes(msg.data);     
  }
  return size;
}

// TM6b  => This seems wrong. : (
// dataSizeBytes only checks for number, boolean, string or binary

export function dataSizeBytes(data: string | number | boolean | Bufferlike): number {
  if (Platform.BufferUtils.isBuffer(data)) {
    return Platform.BufferUtils.byteLength(data);
  }
  if (typeof data === 'string') {
    return Platform.Config.stringByteSize(data);
  }
  if (typeof data === 'number') {
    return 8;
  }
  if (typeof data === 'boolean') {
    return 1;
  }
  throw new Error(
    `Expected input of Utils.dataSizeBytes to be a string, a number, a boolean or a buffer, but was: ${typeof data}`,
  );
}

Copy link
Copy Markdown
Collaborator Author

@sacOO7 sacOO7 Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calculation for msg.data seems wrong, because as per TM6b => The size of an @Object@ or @Array@ @data@ property is its string length after being JSON-stringified

Copy link
Copy Markdown
Contributor

@VeskeR VeskeR Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If data has type boolean or number then what should be it's size?

This is not an accepted data type for regular Messages or PresenceMessage.
Don't know about Annotation messages, but the message size calculation for it should be discussed in a separate conversation below.

Copy link
Copy Markdown
Contributor

@VeskeR VeskeR Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calculation for msg.data seems wrong, because as per TM6b => The size of an @object@ or @array@ @DaTa@ property is its string length after being JSON-stringified

It is calculated correctly. The message size is calculated on the encoded (and encrypted) data, as it requires data to be represented as a string or buffer:
https://github.com/ably/ably-js/blob/d55afe78340f1f16d17f7ec9eeffc15bf6563667/src/common/lib/types/message.ts#L91-L93

That said, I can see the spec is missing the part that the size calculation should take place after encoding and encryption. This is something that should be added.

Copy link
Copy Markdown
Collaborator Author

@sacOO7 sacOO7 Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If data field after encoding is either string or buffer, then we don't need TM6b right

Copy link
Copy Markdown
Contributor

@VeskeR VeskeR Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhhh, it seems like it's not that straightforward.
Based on the ably-js implementation comment, the message should be encoded and encrypted before calculating its size (which happens here). However, it should not apply protocol-specific encoding - such as representing binary data as a Base64 string in the JSON protocol (TM6c). That kind of encoding takes place at a later stage in ably-js, after message size calculation.

So currently, the spec for message size calculation kind of dances around this logic. It says that Object and Array data should be JSON-stringified in TM6b (per RSL4c3 and RSL4d3), but states that Base64 encoding should not be applied for binary data (RSL4d1 should not apply). At the same time, it doesn’t mention that the data should be encrypted (if applicable) before size calculation if client was configured to do so.

Ideally, all of these nuances should be clarified and stated explicitly in the spec for message size calculation. However, I’m not sure this should be our priority right now. Based on Simon’s comment below, it’s not a requirement to have this implemented for Annotations, and we haven’t planned to update this for Message and PresenceMessage.

Would opening a separate issue that outlines the scope of the needed changes suffice in this case?

Copy link
Copy Markdown
Collaborator Author

@sacOO7 sacOO7 Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created separate issue #336, meanwhile you can review updated spec PR.

** @(TM6g)@ The size of a @string@ @name@ property is its length
** @(TM6h)@ The size of a @string@ @clientId@ property is its length
Copy link
Copy Markdown
Member

@SimonWoolf SimonWoolf Jun 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no reason that name, clientId, data, and json-encoded-extras strings should use different calculations for their sizes. If we want to specify "UTF-8 encoded length in bytes" for a string data (which I agree is the correct interpretation, we want to bound the encoded size over the wire), we should use that same wording on the others for consistency.

(if ably-js doesn't do this that's technically a bug, though not one that matters much. if the difference changes the result then it wasn't grossy over the limit and it can just get rejected by the server, shrug)

Copy link
Copy Markdown
Collaborator Author

@sacOO7 sacOO7 Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, then it will be better to be specific for each spec point 👍
I guess we will need to update all string specific spec points.

Copy link
Copy Markdown
Collaborator Author

@sacOO7 sacOO7 Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

** @(TM6d)@ The size of the @extras@ property is the string length of its JSON representation
Copy link
Copy Markdown
Collaborator Author

@sacOO7 sacOO7 Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have Non-ASCII characters included in extras right

Copy link
Copy Markdown
Member

@SimonWoolf SimonWoolf Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we definitely can, they can contain arbitrary data, eg in a push payload

Copy link
Copy Markdown
Collaborator Author

@sacOO7 sacOO7 Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks will update for all string spec points 👍

Copy link
Copy Markdown
Collaborator Author

@sacOO7 sacOO7 Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

** @(TM6e)@ The size of a @null@ or omitted property is zero
* @(TM7)@ The SDK may expose a series of functions (static factory methods on a Message or otherwise, wherever is language idiomatic; for some languages this might just be types that can be used for type assertions, etc), that take a deserialized @JsonObject@, one of the aggregated summaries for a particular annotation type (that is, a value from the @TM2q@ @summary@ @Dict@), and outputs a strongly-typed summary entry, for ease of use by the end user (particularly in languages where manipulating plain objects is difficult).
Expand Down Expand Up @@ -1496,7 +1498,11 @@ h4. PresenceMessage
** @(TP3g)@ @timestamp@ time in milliseconds since epoch. If a presence message received from Ably does not contain a @timestamp@, it should be set to the @timestamp@ of the encapsulating @ProtocolMessage@
** @(TP3h)@ @memberKey@ string function that combines the @connectionId@ and @clientId@ ensuring multiple connected clients with the same clientId are uniquely identifiable
* @(TP4)@ @fromEncoded@ and @fromEncodedArray@ are alternative constructors that take an (already deserialized) @PresenceMessage@-like object (or array of such objects), and optionally a @channelOptions@, and return a @PresenceMessage@ (or array of such @PresenceMessages@) that's decoded and decrypted as specified in @RSL6@, using the cipher in the @channelOptions@ if the message is encrypted, with any residual transforms (ones that the library cannot decode or decrypt) left in the @encoding@ property per @RSL6b@. This is intended for users receiving messages other than from a REST or Realtime channel (for example, from a queue), to avoid them having to parse the @encoding@ string themselves. This behaviour is the same as in @(TM3)@.
* @(TP5)@ The size of the @PresenceMessage@ for "TO3l8":#TO3l8 is calculated in the same way as for @Message@; see "TM6":#TM6
* @(TP5)@ The size of the @PresenceMessage@ for "TO3l8":#TO3l8 is calculated as follows:
** @(TP5a)@ The size is the sum of the sizes of the @data@ and @clientId@ properties
Comment thread
sacOO7 marked this conversation as resolved.
Outdated
Comment thread
sacOO7 marked this conversation as resolved.
Outdated
** @(TP5b)@ The size of a @data@ property is as per "TM6b":#TM6b, "TM6c":#TM6c and "TM6f":#TM6f
** @(TP5e)@ The size of a @string@ @clientId@ property is its length
Comment thread
sacOO7 marked this conversation as resolved.
Outdated
** @(TP5f)@ The size of a @null@ or omitted property is zero

h4. ObjectMessage

Expand Down Expand Up @@ -1650,6 +1656,13 @@ h4. Annotation
** @(TAN2k)@ @type@ string: a string indicating the type of the annotation, handled opaquely by the SDK
** @(TAN2l)@ @extras@ object (optional): A JSON-encodable object for arbitrary metadata
* @(TAN3)@ @fromEncoded@ and @fromEncodedArray@ are alternative constructors that take an (already deserialized) @Annotation@-like object (or array of such objects), and optionally a @channelOptions@, and return an @Annotation@ (or array of @Annotations@) that's decoded and decrypted analagously to @TM3@ and @TP4@
* @(TAN4)@ The size of the @Annotation@ for "TO3l8":#TO3l8 is calculated as follows:
Comment thread
sacOO7 marked this conversation as resolved.
Outdated
** @(TAN4a)@ The size is the sum of the sizes of the @name@, @data@, @clientId@, and @extras@ properties
** @(TAN4b)@ The size of a @data@ property is as per "TM6b":#TM6b, "TM6c":#TM6c and "TM6f":#TM6f
** @(TAN4e)@ The size of a @string@ @name@ property is its length
** @(TAN4f)@ The size of a @string@ @clientId@ property is its length
** @(TAN4g)@ The size of the @extras@ property is the string length of its JSON representation
** @(TAN4h)@ The size of a @null@ or omitted property is zero
Comment thread
sacOO7 marked this conversation as resolved.
Outdated

h4. ProtocolMessage

Expand Down
Loading