Skip to content

Commit

Permalink
Address review feedback
Browse files Browse the repository at this point in the history 
- Remove default value for `visitedObjects` parameter
- Move circular reference check from the `object` value encoder  into `wirteTypeValue` to ensure that all the ciruclar reference check is applied on all object references
  • Loading branch information
@tortmayr
tortmayr committed Jun 16, 2022
1 parent dbf0060 commit 8e9f4de
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 36 deletions.
14 changes: 12 additions & 2 deletions packages/core/src/common/message-rpc/rpc-message-encoder.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import { expect } from 'chai';
import { Uint8ArrayReadBuffer, Uint8ArrayWriteBuffer } from './uint8-array-message-buffer';
import { RpcMessageDecoder, RpcMessageEncoder } from './rpc-message-encoder';
import { EncodingError, RpcMessageDecoder, RpcMessageEncoder } from './rpc-message-encoder';

describe('PPC Message Codex', () => {
describe('RPC Message Encoder & Decoder', () => {
Expand All @@ -30,7 +30,7 @@ describe('PPC Message Codex', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
jsonMangled.registeredTags = (encoder as any).registeredTags;

encoder.writeTypedValue(writer, encoder);
encoder.writeTypedValue(writer, encoder, new WeakSet());

const written = writer.getCurrentContents();

Expand All @@ -41,5 +41,15 @@ describe('PPC Message Codex', () => {

expect(decoded).deep.equal(jsonMangled);
});
it('should fail with an EncodingError when trying to encode the object ', () => {
const x = new Set();
const y = new Set();
x.add(y);
y.add(x);
const encoder = new RpcMessageEncoder();

const writer = new Uint8ArrayWriteBuffer();
expect(() => encoder.writeTypedValue(writer, x, new WeakSet())).to.throw(EncodingError);
});
});
});
72 changes: 38 additions & 34 deletions packages/core/src/common/message-rpc/rpc-message-encoder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,28 +382,19 @@ export class RpcMessageEncoder {
this.registerEncoder(ObjectType.Object, {
is: value => typeof value === 'object',
write: (buf, object, visitedObjects, recursiveEncode) => {
try {
if (visitedObjects.has(object)) {
throw new EncodingError('Object to encode contains circular references!');
}
visitedObjects.add(object);

const properties = Object.keys(object);
const relevant = [];
for (const property of properties) {
const value = object[property];
if (typeof value !== 'function') {
relevant.push([property, value]);
}
const properties = Object.keys(object);
const relevant = [];
for (const property of properties) {
const value = object[property];
if (typeof value !== 'function') {
relevant.push([property, value]);
}
}

buf.writeLength(relevant.length);
for (const [property, value] of relevant) {
buf.writeString(property);
recursiveEncode?.(buf, value, visitedObjects);
}
} finally {
visitedObjects.delete(object);
buf.writeLength(relevant.length);
for (const [property, value] of relevant) {
buf.writeString(property);
recursiveEncode?.(buf, value, visitedObjects);
}
}
});
Expand Down Expand Up @@ -504,42 +495,55 @@ export class RpcMessageEncoder {
buf.writeUint8(RpcMessageType.Notification);
buf.writeUint32(requestId);
buf.writeString(method);
this.writeArray(buf, args);
this.writeArray(buf, args, new WeakSet());
}

request(buf: WriteBuffer, requestId: number, method: string, args: any[]): void {
buf.writeUint8(RpcMessageType.Request);
buf.writeUint32(requestId);
buf.writeString(method);
this.writeArray(buf, args);
this.writeArray(buf, args, new WeakSet());
}

replyOK(buf: WriteBuffer, requestId: number, res: any): void {
buf.writeUint8(RpcMessageType.Reply);
buf.writeUint32(requestId);
this.writeTypedValue(buf, res);
this.writeTypedValue(buf, res, new WeakSet());
}

replyErr(buf: WriteBuffer, requestId: number, err: any): void {
buf.writeUint8(RpcMessageType.ReplyErr);
buf.writeUint32(requestId);
this.writeTypedValue(buf, err);
this.writeTypedValue(buf, err, new WeakSet());
}

writeTypedValue(buf: WriteBuffer, value: any, visitedObjects = new WeakSet()): void {
for (let i: number = this.encoders.length - 1; i >= 0; i--) {
if (this.encoders[i][1].is(value)) {
buf.writeUint8(this.encoders[i][0]);
this.encoders[i][1].write(buf, value, visitedObjects, (innerBuffer, innerValue, _visitedObjects) => {
this.writeTypedValue(innerBuffer, innerValue, _visitedObjects);
});
return;
writeTypedValue(buf: WriteBuffer, value: any, visitedObjects: WeakSet<object>): void {
if (value && typeof value === 'object') {
if (visitedObjects.has(value)) {
throw new EncodingError('Object to encode contains circular references!');
}
visitedObjects.add(value);
}

try {
for (let i: number = this.encoders.length - 1; i >= 0; i--) {
if (this.encoders[i][1].is(value)) {
buf.writeUint8(this.encoders[i][0]);
this.encoders[i][1].write(buf, value, visitedObjects, (innerBuffer, innerValue, _visitedObjects) => {
this.writeTypedValue(innerBuffer, innerValue, _visitedObjects);
});
return;
}
}
throw new EncodingError(`No suitable value encoder found for ${value}`);
} finally {
if (value && typeof value === 'object') {
visitedObjects.delete(value);
}
}
throw new EncodingError(`No suitable value encoder found for ${value}`);
}

writeArray(buf: WriteBuffer, value: any[], visitedObjects = new WeakSet()): void {
writeArray(buf: WriteBuffer, value: any[], visitedObjects: WeakSet<object>): void {
buf.writeLength(value.length);
for (let i = 0; i < value.length; i++) {
this.writeTypedValue(buf, value[i], visitedObjects);
Expand Down

0 comments on commit 8e9f4de

Please sign in to comment.