diff --git a/CHANGELOG.md b/CHANGELOG.md index b8db9730c87c..efb2aca2b485 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## [5.0.0-dev4] + +[5.0.0-dev4]: https://github.com/microsoft/CCF/releases/tag/ccf-5.0.0-dev4 + +- Fix for JS execution behaviour when reusing interpreters. Storing KV handles on the global state may lead to unsafe accesses. Work around that by lazily requesting handles in the TypedKvMap for TypeScript apps. + ## [5.0.0-dev3] [5.0.0-dev3]: https://github.com/microsoft/CCF/releases/tag/ccf-5.0.0-dev3 diff --git a/js/ccf-app/src/kv.ts b/js/ccf-app/src/kv.ts index 662ebf267252..d358e19e42b4 100644 --- a/js/ccf-app/src/kv.ts +++ b/js/ccf-app/src/kv.ts @@ -41,43 +41,51 @@ import { KvMap, ccf } from "./global.js"; import { DataConverter } from "./converters.js"; export class TypedKvMap { + private _kv() { + const kvMap = + typeof this.nameOrMap === "string" + ? ccf.kv[this.nameOrMap] + : this.nameOrMap; + return kvMap; + } + constructor( - private kv: KvMap, + private nameOrMap: string | KvMap, private kt: DataConverter, private vt: DataConverter, ) {} has(key: K): boolean { - return this.kv.has(this.kt.encode(key)); + return this._kv().has(this.kt.encode(key)); } get(key: K): V | undefined { - const v = this.kv.get(this.kt.encode(key)); + const v = this._kv().get(this.kt.encode(key)); return v === undefined ? undefined : this.vt.decode(v); } getVersionOfPreviousWrite(key: K): number | undefined { - return this.kv.getVersionOfPreviousWrite(this.kt.encode(key)); + return this._kv().getVersionOfPreviousWrite(this.kt.encode(key)); } set(key: K, value: V): TypedKvMap { - this.kv.set(this.kt.encode(key), this.vt.encode(value)); + this._kv().set(this.kt.encode(key), this.vt.encode(value)); return this; } delete(key: K): void { - this.kv.delete(this.kt.encode(key)); + this._kv().delete(this.kt.encode(key)); } clear(): void { - this.kv.clear(); + this._kv().clear(); } forEach(callback: (value: V, key: K, table: TypedKvMap) => void): void { let kt = this.kt; let vt = this.vt; let typedMap = this; - this.kv.forEach(function ( + this._kv().forEach(function ( raw_v: ArrayBuffer, raw_k: ArrayBuffer, table: KvMap, @@ -87,7 +95,7 @@ export class TypedKvMap { } get size(): number { - return this.kv.size; + return this._kv().size; } } @@ -109,8 +117,7 @@ export function typedKv( kt: DataConverter, vt: DataConverter, ) { - const kvMap = typeof nameOrMap === "string" ? ccf.kv[nameOrMap] : nameOrMap; - return new TypedKvMap(kvMap, kt, vt); + return new TypedKvMap(nameOrMap, kt, vt); } /** diff --git a/tests/js-custom-authorization/custom_authorization.py b/tests/js-custom-authorization/custom_authorization.py index cfd88be27af2..6b024871ba76 100644 --- a/tests/js-custom-authorization/custom_authorization.py +++ b/tests/js-custom-authorization/custom_authorization.py @@ -899,6 +899,18 @@ def was_cached(response): expect_much_smaller(repeat2, baseline) expect_much_smaller(repeat3, baseline) + LOG.info("Testing caching of KV handles") + r = c.post("/app/increment") + assert r.status_code == http.HTTPStatus.OK, r + r = c.post("/app/increment") + assert r.status_code == http.HTTPStatus.OK, r + r = c.post("/app/increment") + assert r.status_code == http.HTTPStatus.OK, r + r = c.post("/app/increment") + assert r.status_code == http.HTTPStatus.OK, r + r = c.post("/app/increment") + assert r.status_code == http.HTTPStatus.OK, r + return network diff --git a/tests/js-interpreter-reuse/app.json b/tests/js-interpreter-reuse/app.json index 7a4ec9838908..876965b0e0b4 100644 --- a/tests/js-interpreter-reuse/app.json +++ b/tests/js-interpreter-reuse/app.json @@ -61,6 +61,19 @@ "key": "arbitrary_string_goes_here" } } + }, + "/increment": { + "post": { + "js_module": "global_handle.js", + "js_function": "increment", + "forwarding_required": "never", + "authn_policies": ["no_auth"], + "mode": "readwrite", + "openapi": {}, + "interpreter_reuse": { + "key": "increment" + } + } } } } diff --git a/tests/js-interpreter-reuse/src/global_handle.ts b/tests/js-interpreter-reuse/src/global_handle.ts new file mode 100644 index 000000000000..07c591afd0b7 --- /dev/null +++ b/tests/js-interpreter-reuse/src/global_handle.ts @@ -0,0 +1,25 @@ +import * as ccfapp from "@microsoft/ccf-app"; + +function increment() { + if (!("kvHandle" in globalThis)) { + globalThis.kvHandle = ccfapp.typedKv( + "public:cached_handle_table", + ccfapp.string, + ccfapp.uint32, + ); + } + + const k = "single_key"; + if (!globalThis.kvHandle.has(k)) { + globalThis.kvHandle.set(k, 0); + } else { + const v = globalThis.kvHandle.get(k); + globalThis.kvHandle.set(k, v + 1); + } + + const v = globalThis.kvHandle.get(k); + + return { body: { value: v } }; +} + +export { increment }; diff --git a/tests/js-interpreter-reuse/src/rollup_entry.ts b/tests/js-interpreter-reuse/src/rollup_entry.ts index aa1303bb0442..6fd965ce851f 100644 --- a/tests/js-interpreter-reuse/src/rollup_entry.ts +++ b/tests/js-interpreter-reuse/src/rollup_entry.ts @@ -1,2 +1,3 @@ export * from "./cache.js"; export * from "./di_sample"; +export * from "./global_handle";