Json smart contract parser by shargon · Pull Request #785 · neo-project/neo

shargon · 2019-05-26T18:03:19Z

Related to #779

TODO: max length for items

JSON parsing libraries have been observed to differ as to whether or not they make the ordering of object members visible to calling software.

An implementation may set limits on the size of texts that it accepts. An implementation may set limits on the maximum depth of nesting. An implementation may set limits on the range and precision of numbers. An implementation may set limits on the length and character contents of strings.

codecov-io · 2019-05-26T18:06:40Z

Codecov Report

Merging #785 into master will increase coverage by 0.28%.
The diff coverage is 76.47%.

@@            Coverage Diff             @@
##           master     #785      +/-   ##
==========================================
+ Coverage   36.82%   37.11%   +0.28%     
==========================================
  Files         174      175       +1     
  Lines       12262    12296      +34     
==========================================
+ Hits         4516     4564      +48     
+ Misses       7746     7732      -14

Impacted Files	Coverage Δ
neo/SmartContract/InteropService.cs	`15.43% <12.5%> (-0.06%)`	⬇️
neo/VM/JsonParser.cs	`96.15% <96.15%> (ø)`
neo/IO/Json/JObject.cs	`68.5% <0%> (+2.36%)`	⬆️
neo/IO/Json/JArray.cs	`57.95% <0%> (+3.4%)`	⬆️
neo/IO/Json/JNumber.cs	`47.05% <0%> (+31.37%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2401a11...b92ec8c. Read the comment docs.

codecov-io · 2019-05-26T18:06:41Z

Codecov Report

Merging #785 into master will increase coverage by 0.92%.
The diff coverage is 79.79%.

@@            Coverage Diff             @@
##           master     #785      +/-   ##
==========================================
+ Coverage   36.82%   37.75%   +0.92%     
==========================================
  Files         174      176       +2     
  Lines       12262    12439     +177     
==========================================
+ Hits         4516     4696     +180     
+ Misses       7746     7743       -3

Impacted Files	Coverage Δ
neo/IO/Json/JNumber.cs	`79.78% <100%> (+64.1%)`	⬆️
neo/SmartContract/InteropService.NEO.cs	`12.8% <13.33%> (+0.02%)`	⬆️
neo/IO/Caching/OrderedDictionary.cs	`47.45% <47.45%> (ø)`
neo/IO/Json/JBoolean.cs	`65% <65%> (-20.37%)`	⬇️
neo/IO/Json/JString.cs	`79.31% <90%> (+19.69%)`	⬆️
neo/IO/Json/JObject.cs	`83.84% <93.1%> (+17.7%)`	⬆️
neo/IO/Json/JArray.cs	`58.51% <95.45%> (+3.96%)`	⬆️
neo/SmartContract/JsonSerializer.cs	`97.82% <97.82%> (ø)`
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2401a11...427780b. Read the comment docs.

erikzhang

Neo.Json.Serialize - serialize a StackItem object to a JSON string.
Neo.Json.Deserialize - deserialize a JSON string to a StackItem object.

{"a": [1, "2"]}

It will be converted to a Map, whose key is a ByteArray, and value is an Array with 2 elements. The first element is an Integer, and second element is a ByteArray.

igormcoelho

looks good.

neo.UnitTests/UT_JsonParser.cs

igormcoelho · 2019-05-27T00:16:46Z

neo.UnitTests/UT_JsonParser.cs

+            var map = (VM.Types.Map)items;
+
+            Assert.IsTrue(map.TryGetValue(new VM.Types.ByteArray(Encoding.UTF8.GetBytes("test")), out var value));
+            Assert.AreEqual(((VM.Types.Integer)value).GetBigInteger(), 123);


parsing int to BigInt may be an issue... let's see.

neo.UnitTests/UT_JsonParser.cs

igormcoelho · 2019-05-27T00:19:10Z

neo/SmartContract/InteropService.cs

        public static readonly uint System_Runtime_GetTime = Register("System.Runtime.GetTime", Runtime_GetTime, 0_00000250);
        public static readonly uint System_Runtime_Serialize = Register("System.Runtime.Serialize", Runtime_Serialize, 0_00100000);
        public static readonly uint System_Runtime_Deserialize = Register("System.Runtime.Deserialize", Runtime_Deserialize, 0_00500000);
+        public static readonly uint System_Runtime_ParseJon = Register("System.Runtime.ParseJson", Runtime_ParseJson, 100_00000000/*TODO: Compute gast cost*/);


I think we should abolish fixed prices on Neo 3.0. One problem less, and one problem we cannot solve without market and consensus nodes.

neo/VM/JsonParser.cs

igormcoelho · 2019-05-27T00:22:39Z

I approved what is now... but I agree Erik's interface is better:
Neo.Json.Serialize - serialize a StackItem object to a JSON string.
Neo.Json.Deserialize - deserialize a JSON string to a StackItem object.

neo/SmartContract/InteropService.NEO.cs

erikzhang · 2019-05-27T09:34:15Z

neo/SmartContract/InteropService.NEO.cs

+
+        private static bool Json_Deserialize(ApplicationEngine engine)
+        {
+            var json = Encoding.UTF8.GetString(engine.CurrentContext.EvaluationStack.Pop().GetByteArray());


Maybe we need to handle the illegal characters consistently in smart contracts.

JObject.Parse should deal with this, what do you propose?

Yes. We should deal with this in JObject.Parse().

neo/VM/JsonParser.cs

erikzhang · 2019-05-27T14:05:55Z

I am concerned that different languages handle JSON differently and may lead to inconsistencies in smart contracts. We must impose strict restrictions on JObject.Parse().

shargon · 2019-05-27T14:37:37Z

Yes, we should be able to parse True as a valid boolean too

erikzhang · 2019-05-27T14:42:37Z

True is not a valid value in JSON, I think.

shargon · 2019-05-27T14:50:49Z

So, do you think maybe Python will not produce exactly the same output as us? We can solve this in the specification, and unit tests.

erikzhang · 2019-05-27T14:55:24Z

Yes, we must ensure that it is handled in strict accordance with the JSON specification.

https://tools.ietf.org/html/rfc7159

shargon · 2019-05-27T15:37:57Z

I will do with our json classes

shargon · 2019-05-29T10:01:26Z

@neo-project/core do you think that there are any pending unit test? we need to ensure the deterministic behaviour

erikzhang · 2019-05-29T10:05:39Z

Yes. See the followings:

JSON parsing libraries have been observed to differ as to whether or not they make the ordering of object members visible to calling software.

For example:

PUSH {"b": 1, "a": 2}
SYSCALL Neo.Json.Deserialize
VALUES

Some implementation may return [1,2], the others may return [2,1]. We must ensure the deterministic behaviour of this.

An implementation may set limits on the size of texts that it accepts. An implementation may set limits on the maximum depth of nesting. An implementation may set limits on the range and precision of numbers. An implementation may set limits on the length and character contents of strings.

And these limits must be set.

shargon · 2019-05-29T10:16:16Z

I think other clients should avoid libraries and do their own implementation, according to this, that we should specify in the specifications.

erikzhang · 2019-05-29T11:58:24Z

We must use OrderedDictionary for maintaining JObject's properties.

shargon · 2019-05-29T12:05:02Z

This doesn't work?
https://docs.microsoft.com/en-us/dotnet/api/system.collections.specialized.ordereddictionary?view=netframework-4.8

erikzhang · 2019-05-29T12:06:34Z

This is a non-generic version of OrderedDictionary. I've created a generic one.

erikzhang · 2019-05-29T18:41:27Z

I'm afraid you didn't really use OrderedDictionary in the code. @shargon

shargon · 2019-05-29T22:34:27Z

i forget to commit this file xD

* release-2.10.0 * Update v2.10.0.md (neo-project#785) * Update v2.10.0.md * Update v2.10.0.md * Update and rename v2.10.0.md to v2.10.0 * Update v2.10.0 * update exchange * Introduction for gas related rpc (neo-project#788) Introduction for gas related rpc * add plugin * Update v2.10.0.md * update English files

* release-2.10.0 * Update v2.10.0.md (neo-project#785) * Update v2.10.0.md * Update v2.10.0.md * Update and rename v2.10.0.md to v2.10.0 * Update v2.10.0 * update exchange * add new api * Add new api/gettransactionheight.md * fix fix * Update claimgas.md * Update importkey.md

ixje · 2020-07-14T13:31:45Z

Some implementation may return [1,2], the others may return [2,1]. We must ensure the deterministic behaviour of this.

An implementation may set limits on the size of texts that it accepts. An implementation may set limits on the maximum depth of nesting. An implementation may set limits on the range and precision of numbers. An implementation may set limits on the length and character contents of strings.

And these limits must be set.

@shargon is it me or is there no code for the following restrictions?

An implementation may set limits on the size of texts that it accepts

An implementation may set limits on the length and character contents of strings.

shargon · 2020-07-14T13:50:33Z

An implementation may set limits on the size of texts that it accepts

The vm has this limmit. Outside the VM you can use any length.

According to the character content. We force strict utf8 now

neo/src/neo/IO/Json/JObject.cs

Line 83 in d5b962c

return Parse(Utility.StrictUTF8.GetBytes(value), max_nest);

ixje · 2020-07-14T14:26:11Z

Then I'm good. Thanks! :-)

Json api

b92ec8c

shargon requested a review from erikzhang May 26, 2019 18:03

erikzhang reviewed May 26, 2019

View reviewed changes

igormcoelho previously approved these changes May 27, 2019

View reviewed changes

Serialize and Deserialize

9eb2b20

shargon dismissed igormcoelho’s stale review via 9eb2b20 May 27, 2019 08:08

Number overflow protection

83059fb

erikzhang reviewed May 27, 2019

View reviewed changes

shargon and others added 11 commits May 27, 2019 12:19

Erik's review

2a006ee

Clean code

acfad5e

Fix max js integer

56d9da3

Prevent non deterministic code

add4f50

Throw with decimal values

25c82c4

improve

cffbf32

rename

d453aaa

rename

8bbb8d5

simplify

ad0ba8d

simplify

edfa53d

Add MIN_SAFE_INTEGER

fcc87d2

shargon and others added 2 commits May 27, 2019 18:45

Json changes

0a9d5d9

readability

9a911ee

Fix JObject.Parse()

64073ba

shargon added 2 commits May 29, 2019 12:09

Ensure that is not possible to parse invalid numbers

545da2e

Two more tests

840e4ee

shargon and others added 2 commits May 29, 2019 12:42

More unit test

a78832b

Add generic OrderedDictionary

36ae0c6

Use ordered dictionary for properties

d275d4a

Fix Ordered dictionary

adc84ac

erikzhang previously approved these changes May 30, 2019

View reviewed changes

internal

47984c2

erikzhang dismissed their stale review via 47984c2 May 30, 2019 08:04

erikzhang previously approved these changes May 30, 2019

View reviewed changes

Null in uppercase

427780b

shargon dismissed erikzhang’s stale review via 427780b May 30, 2019 08:52

erikzhang approved these changes May 30, 2019

View reviewed changes

shargon merged commit e365f00 into neo-project:master May 30, 2019

shargon deleted the json-api branch May 30, 2019 10:40

roman-khimov mentioned this pull request Jun 11, 2020

Json.Serialize doesn't support Buffer type #1697

Closed

Conversation

shargon commented May 26, 2019 • edited by erikzhang Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented May 26, 2019

Codecov Report

Uh oh!

codecov-io commented May 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

erikzhang left a comment

Choose a reason for hiding this comment

Uh oh!

igormcoelho left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

igormcoelho May 27, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

igormcoelho May 27, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

igormcoelho commented May 27, 2019

Uh oh!

Uh oh!

Uh oh!

erikzhang May 27, 2019

Choose a reason for hiding this comment

Uh oh!

shargon May 27, 2019

Choose a reason for hiding this comment

Uh oh!

erikzhang May 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

erikzhang commented May 27, 2019

Uh oh!

shargon commented May 27, 2019

Uh oh!

erikzhang commented May 27, 2019

Uh oh!

shargon commented May 27, 2019

Uh oh!

erikzhang commented May 27, 2019

Uh oh!

shargon commented May 27, 2019

Uh oh!

shargon commented May 29, 2019

Uh oh!

erikzhang commented May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shargon commented May 29, 2019

Uh oh!

erikzhang commented May 29, 2019

Uh oh!

shargon commented May 29, 2019

Uh oh!

erikzhang commented May 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erikzhang commented May 29, 2019

Uh oh!

shargon commented May 29, 2019

Uh oh!

ixje commented Jul 14, 2020

Uh oh!

shargon commented Jul 14, 2020

Uh oh!

ixje commented Jul 14, 2020

Uh oh!

Reviewers

shargon commented May 26, 2019 •

edited by erikzhang

Loading

codecov-io commented May 26, 2019 •

edited

Loading

erikzhang May 27, 2019 •

edited

Loading

erikzhang commented May 29, 2019 •

edited

Loading

erikzhang commented May 29, 2019 •

edited

Loading