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

Crash serializing json_serializable objects #374

Closed
lookfirst opened this issue Aug 6, 2019 · 10 comments
Closed

Crash serializing json_serializable objects #374

lookfirst opened this issue Aug 6, 2019 · 10 comments

Comments

@lookfirst
Copy link
Contributor

Included package: graphql client V2.0.1

I'm using one of my model objects as variables for a mutation. I am passing in List of these objects as a graphql variable. For some reason, in raw_operation_data.dart:toKey(), json.encoder() is crashing with this error:

image

As you can see, if I remove the toEncodable, things work just fine:

image

Any ideas?

I do have toJson and fromJson methods in the object, which are just proxies for the stuff generated by json_serializable.

@lookfirst
Copy link
Contributor Author

I just worked around this issue by pre .toJson()'ing my json_serializable model classes first...

final gpus = rig.gpus.map((gpu) => gpu.toJson()).toList();
    QueryResult result = await _client
        .mutate(MutationOptions(document: mutation, variables: {"id": rig.id, "gpus": gpus, "psu": rig.psu.toJson()}));

@micimize
Copy link
Collaborator

micimize commented Aug 6, 2019

https://api.dartlang.org/stable/2.4.0/dart-convert/jsonEncode.html

If toEncodable is omitted, it defaults to a function that returns the result of calling .toJson() on the unencodable object.

So we shoud add the default behavior to our encoder

@lookfirst
Copy link
Contributor Author

I should add, this method potentially generates an absurdly long key. What you should be doing is taking a hash of the string and using that as the key.

@micimize
Copy link
Collaborator

@lookfirst I think we could even just use object.hashCode. PRs welcome, naturally

@micimize
Copy link
Collaborator

Please try 2.1.1-beta.1 and comment if this is still an issue (didn't thoroughly test myself)

@lookfirst
Copy link
Contributor Author

@micimize Confirmed! Thanks for the hard work. One question, why not also fix the code that builds the key to be a shorter string (ie: hash)?

@micimize
Copy link
Collaborator

@lookfirst want to do it carefully - made an issue though #400. For instance, I realized I was misunderstanding hashCode recently, so if I had used it collisions would have been possible (Objects that are not equal are allowed to have the same hash code)

Would also be good to see exactly how apollo implements it, although it might be irrelevant because they do client-side query-resolution.

@lookfirst
Copy link
Contributor Author

https://pchalin.blogspot.com/2014/04/defining-equality-and-hashcode-for-dart.html

apparently quiver has hash functions.

@lookfirst
Copy link
Contributor Author

Oh man, this sent me down a rabbit hole: https://medium.com/booking-com-development/hardening-perls-hash-function-d642601f4e54

I wonder if there are any potential attacks against software as a result of using the quiver hash functions.

@lookfirst
Copy link
Contributor Author

Oh, it just keeps going... dart-lang/sdk#11617

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

No branches or pull requests

2 participants