From b39baec8c55b0ca126c613d4f3a01d6aeb70fa02 Mon Sep 17 00:00:00 2001 From: Rajaram Gaunker Date: Thu, 8 Jun 2017 01:17:02 -0700 Subject: [PATCH 1/6] v8: add a js class for Serializer/Dserializer Calling Serializer/Deserlizer without new crashes node. Adding a js class which just inherits cpp bindings. Fixes: https://github.com/nodejs/node/issues/13326 --- lib/v8.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/v8.js b/lib/v8.js index e5b31564ad63c7..9c1cdccb80839e 100644 --- a/lib/v8.js +++ b/lib/v8.js @@ -15,11 +15,15 @@ 'use strict'; const { Buffer } = require('buffer'); -const { Serializer, Deserializer } = process.binding('serdes'); +const serdesBindings = process.binding('serdes'); const { copy } = process.binding('buffer'); const { objectToString } = require('internal/util'); const { FastBuffer } = require('internal/buffer'); +class Serializer extends serdesBindings.Serializer {} + +class Deserializer extends serdesBindings.Deserializer {} + const { cachedDataVersionTag, setFlagsFromString, From 90c50a6c2af6bded9071dc022284a80a2db3bf71 Mon Sep 17 00:00:00 2001 From: Rajaram Gaunker Date: Thu, 8 Jun 2017 01:17:02 -0700 Subject: [PATCH 2/6] v8: add a js class for Serializer/Dserializer Calling Serializer/Deserlizer without new crashes node. Adding a js class which just inherits cpp bindings. Fixes: https://github.com/nodejs/node/issues/13326 --- test/parallel/test-v8-serdes.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/parallel/test-v8-serdes.js b/test/parallel/test-v8-serdes.js index d3a879fc225353..2e7d164d6889db 100644 --- a/test/parallel/test-v8-serdes.js +++ b/test/parallel/test-v8-serdes.js @@ -131,3 +131,19 @@ const objects = [ assert.deepStrictEqual(v8.deserialize(buf), expectedResult); } + +{ + try { + v8.Serializer(); + } catch (e) { + const m = "Class constructor Serializer cannot be invoked without 'new'"; + assert.strictEqual(e.message, m); + } + + try { + v8.Deserializer(); + } catch (e) { + const m = "Class constructor Deserializer cannot be invoked without 'new'"; + assert.strictEqual(e.message, m); + } +} From e09951d1f013799e70e7c0b49d751aa7ba0e1f25 Mon Sep 17 00:00:00 2001 From: Rajaram Gaunker Date: Thu, 8 Jun 2017 09:55:03 -0700 Subject: [PATCH 3/6] v8: add a js class for Serializer/Dserializer Calling Serializer/Deserlizer without new crashes node. Adding a js class which just inherits cpp bindings. Added refression tests. Fixes: https://github.com/nodejs/node/issues/13326 --- test/parallel/test-v8-serdes.js | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/test/parallel/test-v8-serdes.js b/test/parallel/test-v8-serdes.js index 2e7d164d6889db..5625d4986081ae 100644 --- a/test/parallel/test-v8-serdes.js +++ b/test/parallel/test-v8-serdes.js @@ -133,17 +133,19 @@ const objects = [ } { - try { - v8.Serializer(); - } catch (e) { - const m = "Class constructor Serializer cannot be invoked without 'new'"; - assert.strictEqual(e.message, m); - } - - try { - v8.Deserializer(); - } catch (e) { - const m = "Class constructor Deserializer cannot be invoked without 'new'"; - assert.strictEqual(e.message, m); - } + assert.throws( + () => { + v8.Serializer(); + }, + Error, + "Class constructor Serializer cannot be invoked without 'new'" + ); + + assert.throws( + () => { + v8.Derializer(); + }, + Error, + "Class constructor Deserializer cannot be invoked without 'new'" + ); } From 9643c6c20f6752bd40da023f2ec217e029954f49 Mon Sep 17 00:00:00 2001 From: Rajaram Gaunker Date: Thu, 8 Jun 2017 11:21:30 -0700 Subject: [PATCH 4/6] v8: add a js class for Serializer/Deserializer Calling Serializer/Deserializer without new crashes node. Adding a js class which just inherits cpp bindings. Added regression tests. Fixes: https://github.com/nodejs/node/issues/13326 --- test/parallel/test-v8-serdes.js | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/test/parallel/test-v8-serdes.js b/test/parallel/test-v8-serdes.js index 5625d4986081ae..806cbc5fc3414d 100644 --- a/test/parallel/test-v8-serdes.js +++ b/test/parallel/test-v8-serdes.js @@ -134,18 +134,12 @@ const objects = [ { assert.throws( - () => { - v8.Serializer(); - }, - Error, - "Class constructor Serializer cannot be invoked without 'new'" + () => { v8.Serializer(); }, + /^TypeError: Class constructor Serializer cannot be invoked without 'new'$/ ); assert.throws( - () => { - v8.Derializer(); - }, - Error, - "Class constructor Deserializer cannot be invoked without 'new'" + () => { v8.Deserializer(); }, + /^TypeError: Class constructor Deserializer cannot be invoked without 'new'$/ ); } From 955a1bbb9ded4621b618ab201c45a2c7bc89d866 Mon Sep 17 00:00:00 2001 From: Rajaram Gaunker Date: Fri, 9 Jun 2017 10:38:55 -0700 Subject: [PATCH 5/6] v8: add a js class for Serializer/Deserializer Calling Serializer/Deserializer without new crashes node. Adding a js class which just inherits cpp bindings. Added regression tests. Fixes: https://github.com/nodejs/node/issues/13326 --- lib/v8.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/v8.js b/lib/v8.js index 9c1cdccb80839e..7cdf4598c4fdcd 100644 --- a/lib/v8.js +++ b/lib/v8.js @@ -15,14 +15,20 @@ 'use strict'; const { Buffer } = require('buffer'); -const serdesBindings = process.binding('serdes'); +const { + Serializer: _Serializer, + Deserializer: _Deserializer + } = process.binding('serdes'); const { copy } = process.binding('buffer'); const { objectToString } = require('internal/util'); const { FastBuffer } = require('internal/buffer'); -class Serializer extends serdesBindings.Serializer {} +// Calling exposed c++ functions directly throws exception as it expected to be +// called with new operator and caused an assert to fire. +// Creating JS wrapper so that it gets caught at JS layer. +class Serializer extends _Serializer { } -class Deserializer extends serdesBindings.Deserializer {} +class Deserializer extends _Deserializer { } const { cachedDataVersionTag, From 1de429ce02a8334dd6ae3b5713133c950812d8a4 Mon Sep 17 00:00:00 2001 From: Rajaram Gaunker Date: Fri, 9 Jun 2017 17:40:03 -0700 Subject: [PATCH 6/6] v8: add a js class for Serializer/Deserializer Calling Serializer/Deserializer without new crashes node. Adding a js class which just inherits cpp bindings. Added regression tests. Fixes: https://github.com/nodejs/node/issues/13326 --- lib/v8.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/v8.js b/lib/v8.js index 7cdf4598c4fdcd..ba95c98dadc1ee 100644 --- a/lib/v8.js +++ b/lib/v8.js @@ -16,9 +16,9 @@ const { Buffer } = require('buffer'); const { - Serializer: _Serializer, - Deserializer: _Deserializer - } = process.binding('serdes'); + Serializer: _Serializer, + Deserializer: _Deserializer +} = process.binding('serdes'); const { copy } = process.binding('buffer'); const { objectToString } = require('internal/util'); const { FastBuffer } = require('internal/buffer');