Skip to content

fix #212; Let Encoder#encode() return a copy of the internal buffer #213

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.

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

Merged
merged 3 commits into from
Sep 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ on:

jobs:
nodejs:
runs-on: ubuntu-20.04
runs-on: ubuntu-latest
strategy:
matrix:
node-version:
Expand All @@ -33,7 +33,7 @@ jobs:
- run: codecov -f coverage/*.json

browser:
runs-on: ubuntu-20.04
runs-on: ubuntu-latest
strategy:
matrix:
browser: [ChromeHeadless, FirefoxHeadless]
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/fuzz.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
uses: actions/setup-node@v2
with:
cache: npm
node-version: "16"
node-version: "18"

- run: npm ci
- run: npm run test:fuzz
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# This is the revision history of @msgpack/msgpack

## NEXT

* Let `Encoder#encode()` return a copy of the internal buffer, instead of the reference of the buffer (fix #212).
* Introducing `Encoder#encodeSharedRef()` to return the shared reference to the internal buffer.

## 2.7.2 2022/02/08

https://github.com/msgpack/msgpack-javascript/compare/v2.7.1...v2.7.2
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"test:cover:purejs": "npx nyc --no-clean npm run test:purejs",
"test:cover:te": "npx nyc --no-clean npm run test:te",
"test:deno": "deno test test/deno_test.ts",
"test:fuzz": "npm exec -- jsfuzz@git+https://gitlab.com/gitlab-org/security-products/analyzers/fuzzers/jsfuzz.git --fuzzTime 60 --no-versifier test/decode.jsfuzz.js corpus",
"test:fuzz": "npm exec --yes -- jsfuzz@git+https://gitlab.com/gitlab-org/security-products/analyzers/fuzzers/jsfuzz.git --fuzzTime 60 --no-versifier test/decode.jsfuzz.js corpus",
"cover:clean": "rimraf .nyc_output coverage/",
"cover:report": "npx nyc report --reporter=text-summary --reporter=html --reporter=json",
"test:browser": "karma start --single-run",
Expand Down
20 changes: 15 additions & 5 deletions src/Encoder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,28 @@ export class Encoder<ContextType = undefined> {
private readonly forceIntegerToFloat = false,
) {}

private getUint8Array(): Uint8Array {
return this.bytes.subarray(0, this.pos);
}

private reinitializeState() {
this.pos = 0;
}

/**
* This is almost equivalent to {@link Encoder#encode}, but it returns an reference of the encoder's internal buffer and thus much faster than {@link Encoder#encode}.
*
* @returns Encodes the object and returns a shared reference the encoder's internal buffer.
*/
public encodeSharedRef(object: unknown): Uint8Array {
this.reinitializeState();
this.doEncode(object, 1);
return this.bytes.subarray(0, this.pos);
}

/**
* @returns Encodes the object and returns a copy of the encoder's internal buffer.
*/
public encode(object: unknown): Uint8Array {
this.reinitializeState();
this.doEncode(object, 1);
return this.getUint8Array();
return this.bytes.slice(0, this.pos);
}

private doEncode(object: unknown, depth: number): void {
Expand Down
2 changes: 1 addition & 1 deletion src/encode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,5 @@ export function encode<ContextType = undefined>(
options.ignoreUndefined,
options.forceIntegerToFloat,
);
return encoder.encode(value);
return encoder.encodeSharedRef(value);
}
44 changes: 43 additions & 1 deletion test/reuse-instances.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { deepStrictEqual } from "assert";
import { Encoder, Decoder } from "@msgpack/msgpack";
import { Encoder, Decoder, decode } from "@msgpack/msgpack";

const createStream = async function* (...args: any) {
for (const item of args) {
Expand Down Expand Up @@ -108,5 +108,47 @@ describe("shared instances", () => {
deepStrictEqual(a, [[object]], `#${i}`);
}
});

context("regression #212", () => {
it("runs multiple times", () => {
const encoder = new Encoder();
const decoder = new Decoder();

const data1 = {
isCommunication: false,
isWarning: false,
alarmId: "619f65a2774abf00568b7210",
intervalStart: "2022-05-20T12:00:00.000Z",
intervalStop: "2022-05-20T13:00:00.000Z",
triggeredAt: "2022-05-20T13:00:00.000Z",
component: "someComponent",
_id: "6287920245a582301475627d",
};

const data2 = {
foo: "bar",
};

const arr = [data1, data2];
const enc = arr.map((x) => [x, encoder.encode(x)] as const);

enc.forEach(([orig, acc]) => {
const des = decoder.decode(acc);
deepStrictEqual(des, orig);
});
});
});

context("Encoder#encodeSharedRef()", () => {
it("returns the shared reference", () => {
const encoder = new Encoder();

const a = encoder.encodeSharedRef(true);
const b = encoder.encodeSharedRef(false);

deepStrictEqual(decode(a), decode(b)); // yes, this is the expected behavior
deepStrictEqual(a.buffer, b.buffer);
});
});
});
});