StringStream is wrongly assuming that array items cannot be empty

[ThomasLobker]

When trying to deserialized a serialized array with an empty item, neon-js is throwing an error:

Reached the end of the stream!

I believe this should not happen, it should not be illegal to have empty items in a serialized array.

For example:

// We have a serialized array from a smart contract, for example: ['test','neon','']
serialized = '800200077075626c69736880040014f8af73dc ...'

// We try to deserialize it in neon-js:
deserialized = Neon.sc.StackItem.deserialize(serialized).value;
console.log(deserialized);

This will throw an error:

(node:25781) UnhandledPromiseRejectionWarning: Error: Reached the end of the stream!
    at StringStream.read (/usr/local/src/project/node_modules/@cityofzion/neon-js/dist/index.js:24755:19)
    at Function._deserialize (/usr/local/src/project/node_modules/@cityofzion/neon-js/dist/index.js:23029:33)
    at Function._deserialize (/usr/local/src/project/node_modules/@cityofzion/neon-js/dist/index.js:23014:42)
    at Function._deserialize (/usr/local/src/project/node_modules/@cityofzion/neon-js/dist/index.js:23014:42)
    at Function.deserialize (/usr/local/src/project/node_modules/@cityofzion/neon-js/dist/index.js:23005:21)
    at init (/usr/local/src/project/index.js:62:35)
    at Object.<anonymous> (/usr/local/src/project/index.js:71:1)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
(node:25781) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:25781) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Proposed fix:

diff --git a/packages/neon-core/src/u/StringStream.ts b/packages/neon-core/src/u/StringStream.ts
index ebca4f2..f577213 100644
--- a/packages/neon-core/src/u/StringStream.ts
+++ b/packages/neon-core/src/u/StringStream.ts
@@ -51,9 +51,6 @@ export class StringStream {
    * ss.read(2); // "0203"
    */
   public read(bytes: number = 1): string {
-    if (this.isEmpty()) {
-      throw new Error("Reached the end of the stream!");
-    }
     const out = this.str.substr(this.pter, bytes * 2);
     this.pter += bytes * 2;
     return out;

[snowypowers]

Can I have a fleshed out test case for this? I don't think StringStream is responsible for this but rather the deserializer is not checking for this edge case. The stream does not even know the existence of an array.

On a side note, do you prefer that we return null for end of stream rather than throw an error when attempting to read beyond the end?

[snowypowers]

@ThomasLobker Still waiting for an example so I can debug it. My gut feeling is that the issue lies with the deserializer rather than the StringStream itself.

[ThomasLobker]

Sorry for the delay. I will make up some time next week to make a test scenario.

[hal0x2328]

@ThomasLobker Any progress on the test scenario?

[ThomasLobker]

Make the following contract in Python:

from boa.interop.Neo.Runtime import Log, Serialize, Deserialize
def Main(operation, arguments):
        if operation == 'serialize':
                return Serialize(arguments)

Run the contract and serialize some stuff: (notice the 0 as the last value in the array)

neo> sc build_run /app/serialize.py False False False 0710 05 --i                                                                                                                                           
[Param 0] String input: serialize                                                                                                                                                                           
[Param 1] Array input: ['hello', ['AWFPi7WKkU964rraC2VjSvAmUTPKeEbNvq','2c7fe2fdb008371a11dc2a9931e7c8333216c69e','5ac56b6a00527ac46a51527ac46a00c30973657269616c697a65876421006a51c368154e656f2e52756e74696d652e53657269616c697a65616c7566616a00c30b646573657269616c697a6587642e006a51c3c051876425006a51c300c368174e656f2e52756e74696d652e446573657269616c697a65616c756661006c7566',0]] 

This will be the output:

8002000568656c6c6f800400149ec6163233c8e731992adc111a3708b0fde27f2c00283263376665326664623030383337316131316463326139393331653763383333333231366336396500fd0001356163353662366130303532376163343661353135323761633436613030633330393733363537323639363136633639376136353837363432313030366135316333363831353465363536663265353237353665373436393664363532653533363537323639363136633639376136353631366337353636363136613030633330623634363537333635373236393631366336393761363538373634326530303661353163336330353138373634323530303661353163333030633336383137346536353666326535323735366537343639366436353265343436353733363537323639363136633639376136353631366337353636363130303663373536360000

Now make a neon-js project like this to deserialize that output:

const Neon = require('@cityofzion/neon-js');
serialized = '8002000568656c6c6f800400149ec6163233c8e731992adc111a3708b0fde27f2c00283263376665326664623030383337316131316463326139393331653763383333333231366336396500fd0001356163353662366130303532376163343661353135323761633436613030633330393733363537323639363136633639376136353837363432313030366135316333363831353465363536663265353237353665373436393664363532653533363537323639363136633639376136353631366337353636363136613030633330623634363537333635373236393631366336393761363538373634326530303661353163336330353138373634323530303661353163333030633336383137346536353666326535323735366537343639366436353265343436353733363537323639363136633639376136353631366337353636363130303663373536360000'
deserialized = Neon.sc.StackItem.deserialize(serialized).value;
console.log(deserialized);

And after executing I get the following message:

/home/thomas/src/test/node_modules/@cityofzion/neon-js/dist/index.js:24755
            throw new Error("Reached the end of the stream!");
            ^

Error: Reached the end of the stream!
    at StringStream.read (/home/thomas/src/test/node_modules/@cityofzion/neon-js/dist/index.js:24755:19)
    at Function._deserialize (/home/thomas/src/test/node_modules/@cityofzion/neon-js/dist/index.js:23029:33)
    at Function._deserialize (/home/thomas/src/test/node_modules/@cityofzion/neon-js/dist/index.js:23014:42)
    at Function._deserialize (/home/thomas/src/test/node_modules/@cityofzion/neon-js/dist/index.js:23014:42)
    at Function.deserialize (/home/thomas/src/test/node_modules/@cityofzion/neon-js/dist/index.js:23005:21)
    at Object.<anonymous> (/home/thomas/src/test/test.js:3:34)
    at Module._compile (internal/modules/cjs/loader.js:654:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:665:10)
    at Module.load (internal/modules/cjs/loader.js:566:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:506:12)

When I repeat all these steps and I replace the 0 with 10 all works fine and this will be the output:

[ StackItem { type: 0, value: '68656c6c6f' },
  StackItem {
    type: 128,
    value: [ [StackItem], [StackItem], [StackItem], [StackItem] ] } ]

[ThomasLobker]

@hal0x2328 @snowypowers I tested originally with version 4.1.3 and now I retested with the latest version 4.3.3 with the same results. Let me know if I can be of any further assistance

[snowypowers]

I dug into this and found the reason why. When you place the zero at the end, the serialized output actually serialized it as a null, the OpCodes produced are 0000 which can be interpreted as:

00 (OpCode to indicate type ByteArray)
00 (OpCode to indicate length of bytes to read)

Essentially, the read(0) bytes off the stream is triggering the error because the stream is empty. The issue I have here is if this is intended. Are you intending to insert a null or actually insert the bytearray of length 1 with data 00. Because you used an int instead of a string there, the case is even more ambiguous.

If the null case is the correct way, we can amend read(0) to return null. Else, we should be amending the StackItem deserializer to prevent a read(0) and fill in some defaults.

[ThomasLobker]

As far as I know, there is no distinction between null, none, false or 0 in the smart contract. I think that initially I had this issue when tried to include things like a balance or any amount in an array, where the value can be 0.

[snowypowers]

yeah the main issue I have is serializing then deserializing a StackItem of 00 byte array would return an null StackItem which is kinda wonky imo.

[snowypowers]

v4.4.0 is released and includes this fix. Please verify it on your side.