From 19976ca7aad2dc41a79a7f707fe18d88d0342ac1 Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Mon, 17 Jul 2023 11:17:13 -0700 Subject: [PATCH 1/8] Make opinionated judgements about string/symbol decoding --- src/scval.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/scval.js b/src/scval.js index 3111cc56..6ffc285b 100644 --- a/src/scval.js +++ b/src/scval.js @@ -263,7 +263,7 @@ export function nativeToScVal(val, opts = {}) { * - map -> key-value object of any of the above (via recursion) * - bool -> boolean * - bytes -> Uint8Array - * - string, symbol -> string|Uint8Array + * - string, symbol -> string * * If no conversion can be made, this just "unwraps" the smart value to return * its underlying XDR value. @@ -315,14 +315,19 @@ export function scValToNative(scv) { case xdr.ScValType.scvBytes().value: return scv.value(); + // these are "presented" as strings so we should treat them as such. if the + // user encoded non-printable bytes in their string value, that's on them. + // + // TODO: Should we make a judgement call about UTF-8 here? We could use e.g. + // TextDecoder instead. case xdr.ScValType.scvString().value: + // these are limited to [a-zA-Z0-9_]+, so we can safely make ascii strings case xdr.ScValType.scvSymbol().value: { const v = scv.value(); // string|Buffer - if (Buffer.isBuffer(v)) { - // trying to avoid bubbling up problematic Buffer type - return Uint8Array.from(v); + if (Buffer.isBuffer(v) || ArrayBuffer.isView(v)) { + return Array.from(v).map(char => String.fromCharCode(char)); } - return v; // string + return v; // string already? } // these can be converted to bigint From 7903fbc4253afbb932b24f7a2eef7d424dade9d4 Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Mon, 17 Jul 2023 11:21:03 -0700 Subject: [PATCH 2/8] Use utf8 decoding for strings --- src/scval.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/scval.js b/src/scval.js index 6ffc285b..70f9b3de 100644 --- a/src/scval.js +++ b/src/scval.js @@ -315,19 +315,27 @@ export function scValToNative(scv) { case xdr.ScValType.scvBytes().value: return scv.value(); - // these are "presented" as strings so we should treat them as such. if the - // user encoded non-printable bytes in their string value, that's on them. + // these are "presented" as strings so we should treat them as such (in + // other words, string = bytes with a hint that it's text). if the user + // encoded non-printable bytes in their string value, that's on them. // - // TODO: Should we make a judgement call about UTF-8 here? We could use e.g. - // TextDecoder instead. + // note that we assume a utf8 encoding, which is ascii-compatible. for other + // encodings, you should probably use bytes anyway. case xdr.ScValType.scvString().value: + const v = scv.value(); // string|Buffer + if (Buffer.isBuffer(v) || ArrayBuffer.isView(v)) { + return new TextDecoder(v); + } + return v; // string already + // these are limited to [a-zA-Z0-9_]+, so we can safely make ascii strings case xdr.ScValType.scvSymbol().value: { const v = scv.value(); // string|Buffer if (Buffer.isBuffer(v) || ArrayBuffer.isView(v)) { + // we don't need the add'l complexity of utf8 since the charset is known return Array.from(v).map(char => String.fromCharCode(char)); } - return v; // string already? + return v; // string already } // these can be converted to bigint From c810cb5dcd0b776fa08ca8baec6a152ca6747110 Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Mon, 17 Jul 2023 11:39:45 -0700 Subject: [PATCH 3/8] Fixup linting and tests --- src/scval.js | 5 +++-- test/unit/scval_test.js | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/scval.js b/src/scval.js index 70f9b3de..2bc6750b 100644 --- a/src/scval.js +++ b/src/scval.js @@ -321,12 +321,13 @@ export function scValToNative(scv) { // // note that we assume a utf8 encoding, which is ascii-compatible. for other // encodings, you should probably use bytes anyway. - case xdr.ScValType.scvString().value: + case xdr.ScValType.scvString().value: { const v = scv.value(); // string|Buffer if (Buffer.isBuffer(v) || ArrayBuffer.isView(v)) { - return new TextDecoder(v); + return new TextDecoder().decode(v); } return v; // string already + } // these are limited to [a-zA-Z0-9_]+, so we can safely make ascii strings case xdr.ScValType.scvSymbol().value: { diff --git a/test/unit/scval_test.js b/test/unit/scval_test.js index 00825f3b..631dda6f 100644 --- a/test/unit/scval_test.js +++ b/test/unit/scval_test.js @@ -110,8 +110,8 @@ describe('parsing and building ScVals', function () { ], [xdr.ScVal.scvString('hello there!'), 'hello there!'], [xdr.ScVal.scvSymbol('hello'), 'hello'], - [xdr.ScVal.scvString(Buffer.from('hello')), Buffer.from('hello')], // ensure no conversion - [xdr.ScVal.scvSymbol(Buffer.from('hello')), Buffer.from('hello')], // ensure no conversion + [xdr.ScVal.scvString(Buffer.from('hello')), 'hello'], // ensure conversion + [xdr.ScVal.scvSymbol(Buffer.from('hello')), 'hello'], // ensure conversion [ new StellarBase.Address(kp.publicKey()).toScVal(), (actual) => actual.toString() === kp.publicKey() From ca0c37b9686594f27facb238194ad8132d72bf89 Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Mon, 17 Jul 2023 11:45:01 -0700 Subject: [PATCH 4/8] Fixup string building --- src/scval.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scval.js b/src/scval.js index 2bc6750b..5bbac3f7 100644 --- a/src/scval.js +++ b/src/scval.js @@ -334,7 +334,7 @@ export function scValToNative(scv) { const v = scv.value(); // string|Buffer if (Buffer.isBuffer(v) || ArrayBuffer.isView(v)) { // we don't need the add'l complexity of utf8 since the charset is known - return Array.from(v).map(char => String.fromCharCode(char)); + return Array.from(v).map(char => String.fromCharCode(char)).join(''); } return v; // string already } From 0321faabe2963a4b521eff358a8276d7b5f9a39a Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Mon, 17 Jul 2023 12:22:30 -0700 Subject: [PATCH 5/8] Return bytearray in non-utf8 case --- src/scval.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/scval.js b/src/scval.js index 5bbac3f7..ba1154b9 100644 --- a/src/scval.js +++ b/src/scval.js @@ -320,11 +320,16 @@ export function scValToNative(scv) { // encoded non-printable bytes in their string value, that's on them. // // note that we assume a utf8 encoding, which is ascii-compatible. for other - // encodings, you should probably use bytes anyway. + // encodings, you should probably use bytes anyway. if it cannot be decoded, + // the raw bytes are returned. case xdr.ScValType.scvString().value: { const v = scv.value(); // string|Buffer if (Buffer.isBuffer(v) || ArrayBuffer.isView(v)) { - return new TextDecoder().decode(v); + try { + return new TextDecoder().decode(v); + } catch (e) { + return new Uint8Array(v.buffer); // copy of bytes + } } return v; // string already } From 2d82e563b4f7fa98e0e7fb08468d0e90acf1623b Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Mon, 17 Jul 2023 12:26:42 -0700 Subject: [PATCH 6/8] Fixup yarn fmt --- src/scval.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/scval.js b/src/scval.js index ba1154b9..9fbff5f7 100644 --- a/src/scval.js +++ b/src/scval.js @@ -339,7 +339,9 @@ export function scValToNative(scv) { const v = scv.value(); // string|Buffer if (Buffer.isBuffer(v) || ArrayBuffer.isView(v)) { // we don't need the add'l complexity of utf8 since the charset is known - return Array.from(v).map(char => String.fromCharCode(char)).join(''); + return Array.from(v) + .map((char) => String.fromCharCode(char)) + .join(''); } return v; // string already } From cdf64422b9ccf8a65279d763891265a39bf2144b Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Mon, 17 Jul 2023 12:54:05 -0700 Subject: [PATCH 7/8] Improve performance with normal for loop --- src/scval.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/scval.js b/src/scval.js index 9fbff5f7..b8f1fe79 100644 --- a/src/scval.js +++ b/src/scval.js @@ -263,10 +263,12 @@ export function nativeToScVal(val, opts = {}) { * - map -> key-value object of any of the above (via recursion) * - bool -> boolean * - bytes -> Uint8Array - * - string, symbol -> string + * - symbol -> string + * - string -> string IF the underlying buffer can be decoded as ascii/utf8, + * and a raw Uint8Array of the contents in any error case * - * If no conversion can be made, this just "unwraps" the smart value to return - * its underlying XDR value. + * If no viable conversion can be determined, this just "unwraps" the smart + * value to return its underlying XDR value. * * @param {xdr.ScVal} scv - the input smart contract value * @@ -339,9 +341,11 @@ export function scValToNative(scv) { const v = scv.value(); // string|Buffer if (Buffer.isBuffer(v) || ArrayBuffer.isView(v)) { // we don't need the add'l complexity of utf8 since the charset is known - return Array.from(v) - .map((char) => String.fromCharCode(char)) - .join(''); + const result = new Array(v.length); + for (let i = 0; i < result.length; i += 1) { + result[i] = String.fromCharCode(v[i]); + } + return result.join(''); } return v; // string already } From 810beb32558dd4649312e31a2ef44ae20f0b73b9 Mon Sep 17 00:00:00 2001 From: George Kudrayvtsev Date: Mon, 17 Jul 2023 13:06:37 -0700 Subject: [PATCH 8/8] Unify code paths and clean up comments --- src/scval.js | 46 +++++++++++++++++----------------------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/src/scval.js b/src/scval.js index b8f1fe79..57d68e1a 100644 --- a/src/scval.js +++ b/src/scval.js @@ -252,20 +252,19 @@ export function nativeToScVal(val, opts = {}) { } /** - * Given a smart contract value, attempt to convert to a native type. - * + * Given a smart contract value, attempt to convert it to a native type. * Possible conversions include: * - * - void -> null - * - u32, i32 -> number - * - u64, i64, u128, i128, u256, i256 -> bigint - * - vec -> array of any of the above (via recursion) + * - void -> `null` + * - u32, i32 -> `number` + * - u64, i64, u128, i128, u256, i256 -> `bigint` + * - vec -> `Array` of any of the above (via recursion) * - map -> key-value object of any of the above (via recursion) - * - bool -> boolean - * - bytes -> Uint8Array - * - symbol -> string - * - string -> string IF the underlying buffer can be decoded as ascii/utf8, - * and a raw Uint8Array of the contents in any error case + * - bool -> `boolean` + * - bytes -> `Uint8Array` + * - symbol -> `string` + * - string -> `string` IF the underlying buffer can be decoded as ascii/utf8, + * `Uint8Array` of the raw contents in any error case * * If no viable conversion can be determined, this just "unwraps" the smart * value to return its underlying XDR value. @@ -317,13 +316,16 @@ export function scValToNative(scv) { case xdr.ScValType.scvBytes().value: return scv.value(); - // these are "presented" as strings so we should treat them as such (in - // other words, string = bytes with a hint that it's text). if the user + // Symbols are limited to [a-zA-Z0-9_]+, so we can safely make ascii strings + // + // Strings, however, are "presented" as strings and we treat them as such + // (in other words, string = bytes with a hint that it's text). If the user // encoded non-printable bytes in their string value, that's on them. // - // note that we assume a utf8 encoding, which is ascii-compatible. for other - // encodings, you should probably use bytes anyway. if it cannot be decoded, + // Note that we assume a utf8 encoding (ascii-compatible). For other + // encodings, you should probably use bytes anyway. If it cannot be decoded, // the raw bytes are returned. + case xdr.ScValType.scvSymbol().value: case xdr.ScValType.scvString().value: { const v = scv.value(); // string|Buffer if (Buffer.isBuffer(v) || ArrayBuffer.isView(v)) { @@ -336,20 +338,6 @@ export function scValToNative(scv) { return v; // string already } - // these are limited to [a-zA-Z0-9_]+, so we can safely make ascii strings - case xdr.ScValType.scvSymbol().value: { - const v = scv.value(); // string|Buffer - if (Buffer.isBuffer(v) || ArrayBuffer.isView(v)) { - // we don't need the add'l complexity of utf8 since the charset is known - const result = new Array(v.length); - for (let i = 0; i < result.length; i += 1) { - result[i] = String.fromCharCode(v[i]); - } - return result.join(''); - } - return v; // string already - } - // these can be converted to bigint case xdr.ScValType.scvTimepoint().value: case xdr.ScValType.scvDuration().value: