Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON Validation #1

Open
kyoukaya opened this issue Sep 19, 2019 · 4 comments
Open

JSON Validation #1

kyoukaya opened this issue Sep 19, 2019 · 4 comments
Labels
Important High priority issue

Comments

@kyoukaya
Copy link
Owner

kyoukaya commented Sep 19, 2019

The JSON data that the servers send us is an absolute nightmare. Thankfully, the client seems to send sanely formatted JSON.

These are the known quirks of the server sent JSON:

  • The ordering on keys in JSON objects may change. This may not be much of an issue to a user, but it'll be hard to verify the correctness of marshaled data.
  • Integers may be encoded in a string, e.g., "123", empty string, e.g., "", if the value is effectively nil, or as a JSON number, e.g., 123.
  • Floats may be encoded in a string, e.g., "1.23", empty string, e.g., "", if the value is effectively nil, or as a JSON number, e.g., 1.23.
  • The empty value of a string in the JSON may be the empty string "", or null.
  • In the case where an object is used as a value, when the object is effectively nil, it may be represented as an empty JSON array [].
  • JSON objects are used as a key value map.

The current solution to this problem is to use custom types in the packet definitions to facilitate the marshaling and unmarshaling of JSON data, which is far from ideal.

My current idea to remove the custom types from the packet definitions themselves and at runtime, create a dynamic struct based on the static packet definition but with the custom types in them and marshal the data into that.

@kyoukaya kyoukaya added the Important High priority issue label Sep 19, 2019
@kyoukaya
Copy link
Owner Author

1e1aaf8 currently implements an overhaul of the definition and validation system. There are still some huge caveats, primarily performance due to usage of github.com/iancoleman/orderedmap to create a reference structure for which we can coerce the values of our structs to while marshaling the data.
The performance hit is huge, and particularly noticeable on SIndex/index and SFriend/teamGuns. Will probably look at hand rolling a JSON parser to create the reference structure.

Additionally, capturing the exact format of float values, e.g., the amount of decimal points, in the original JSON is also not possible with this system. Though it does not seem to be throwing too many errors and many floats can be left as strings in the definitions, though this is not ideal.

@kyoukaya
Copy link
Owner Author

Current implementation is failing to handle JSON arrays with mixed types in them such as in SOuthouse/establish_build. []interface{} should be marshaled correctly if a custom JSON parser is to be implemented. Mixed type arrays should hopefully be rare enough to not warrant too many []interface{} in the definitions.

Original: {"build_tmp_data":[80,200001,240000,"build_coin",240]}
Output:   {"build_tmp_data":[80.0,200001.0,240000.0,"build_coin",240.0]}

@kyoukaya kyoukaya changed the title JSON handling JSON Validation Sep 29, 2019
@kyoukaya
Copy link
Owner Author

While we can validate that all keys and values in the JSON data we receive have a representation in our struct definitions, there's no way to ascertain if there is a field in our struct that is not longer being sent in the corresponding JSON data for a certain packet. This could lead to definition bloat over time but I don't see any way of concretely determining if a field is no longer being used unless the client is reversed.

@kyoukaya
Copy link
Owner Author

kyoukaya commented Oct 1, 2019

SEquipDevelopMulti has an entirely different structure depending on the usage of quick builds. It probably isn't worth it to design for varying packet structures based on what is sent in the client packet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Important High priority issue
Projects
None yet
Development

No branches or pull requests

1 participant