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

Serious issue with serializing using jQuery: sub arrays do not transport #1

Open
kltm opened this issue Nov 3, 2016 · 1 comment
Open
Labels

Comments

@kltm
Copy link
Member

kltm commented Nov 3, 2016

This is a bug in jQuery at least, so maybe technically I'd want to get the fix upstream? I don't know, but I found the effects here.

While the node.js engine seems fine with taking objects and arrays when making requests without serializing them (or at least serializing them properly), it seems that jQuery (2.1.4?) is not so forgiving--it seems to be some kind of serializing bug where the "[" and "]" for the list appear in the wrong spot, on the field side instead of the value side, destroying the request as intended (this bad input can be seen as it is fine when it leaves the manager, transforms inside jQuery (I haven't caught exactly where yet), and bad when it gets to barista). In our case, since groups (provided-by) is an array at the top level, we would need to encode it to make sure is survives the trip to the browser (as above); however, for the time being, Minerva cannot decode that string (expecting json object as-is), so we are blocked. Needless to say, this was causing some really weird bugs. Would a jQuery upgrade help here?

Since I'm serializing the requests as a string, and minerva seems happy to decode them as such, they get through fine.

As a temporary workaround, we are just going to take the first provided-by argument as pass it through.

@kltm kltm added the bug label Nov 3, 2016
@kltm
Copy link
Member Author

kltm commented Nov 3, 2016

As a cheap workaround for geneontology/noctua#350, since minerva seems happy with just having a single non-array string, I've hacked it so that is just takes the first if there is a list. Ugh. But we can move forward with at least some functionality for the time being.

Will talk to @balhoff later about options.

Ugh.

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

No branches or pull requests

1 participant