neo-modules icon indicating copy to clipboard operation
neo-modules copied to clipboard

Restrict the set of possible Oracle response filters

Open fyrchik opened this issue 4 years ago • 7 comments

Summary or problem description Currently JSONPath is used for filtering JSON responses. This is problematic:

  1. It has no specification besides one blogpost https://goessner.net/articles/JsonPath/ (it is rather vague).
  2. There are not even 2 fully compatible implementations: https://cburgmer.github.io/json-path-comparison/ (the one used in neo-modules is Json.NET on the right). In the left corner you can also see a variety of corner cases.

As a result bugs like https://github.com/nspcc-dev/neo-go/issues/1903 occur. I expect similar bugs to appear for other implementations (Python one) too.

Do you have any solution you want to propose? Restrict the set of valid oracle filters. I think it is possible to do with a simple regex. I don't think we really need a lot of things besides field access. Some filtering logic can be done by the contract itself. The process could be guided by cases we need to support and available implementations for C#, Go and Python.

fyrchik avatar Apr 21 '21 13:04 fyrchik

I think it is possible to do with a simple regex.

The regex implementation in c#, go and python it's different

shargon avatar Apr 22 '21 07:04 shargon

Please check neo-project/neo#2446

erikzhang avatar Apr 22 '21 07:04 erikzhang

Before I try to find a suitable library (if possible) I'd like see if the following summary of special requirements for this non-standardised JSONPath is correct and complete. So far I have

  • Disallow root recursive decent with a wildcard ($..*)
    • Should this only hold when starting from $ root or any recursive decent with a wildcard?
  • Recursive decent on a child should include all elements (and not omit the last one, related comment)
  • Limit jsonpath nesting depth to 6 (related comment, commit)
  • Do not keep original JSON key order on returned results, but sort in descending order (related comment)

Anything I missed or misinterpreted?

ixje avatar May 03 '21 10:05 ixje

@fyrchik could you please have a quick glimpse at the above list. It looks like you have worked on the Go version, so I'm hoping you have the requirements clear. Thanks 🙏

ixje avatar May 04 '21 06:05 ixje

@ixje Sorry for the delayed response. Yes, you are right. The last one (about order) is especially important. We lose nothing and gain a lot more interoperability. It is not yet implemented and is also the only thing that prevents neo-go from being compatible with C# implementation. 2-nd and 3-rd are already merged.

Regarding first point, I think that recursive descent can be excluded completely. It is much easier to form a simple JSON on the external server, than to parse a complex one using filters or in the contract itself. Currently recursive descent is allowed, but the depth is limited.

fyrchik avatar May 07 '21 16:05 fyrchik

@ixje You may also want to look at nspcc-dev/neo-go#1916 . There are a lot of tests in pkg/services/oracle/jsonpath/jsonpath_test.go file, which cover all good/error code paths. I will re-check if they are compatible with RC2 C# node on monday.

fyrchik avatar May 07 '21 16:05 fyrchik

@fyrchik no worries. I ended up doing just that. Seems too unpredictable to run a library so better just port it.

ixje avatar May 10 '21 13:05 ixje