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

Horizon should be able to update a document that has not been created by horizon #569

Closed
alf opened this issue Jun 7, 2016 · 10 comments · Fixed by #614
Closed

Horizon should be able to update a document that has not been created by horizon #569

alf opened this issue Jun 7, 2016 · 10 comments · Fixed by #614

Comments

@alf
Copy link

alf commented Jun 7, 2016

If you're reporting a bug please include the following:

  • 1.1.1
  • 1.1.1

If you create a document using the rethinkdb admin interface and then try to update that document using horizon, you get hit by the following error message: socket.js:62 Uncaught Error: Object field '$hz_v$' may not be undefined.

If you update the document using the rethinkdb admin interface and add the '$hz_v$' field with a numeric value, the problem goes away.

I would prefer it if horizon would add the field if missing instead of refusing to change the document.

@yankeeinlondon
Copy link

I'm having the same problem, here's an image of the stack trace if it's helpful:

2016-06-08_18-14-42

This feels like a significant issue for a app server fronting a real-time database (where having multiple clients is probably a common feature). I hope we can see this turned around quickly. If I were a bit deeper in the codebase I'd try and help with a PR but I'm not there yet.

@yankeeinlondon
Copy link

yankeeinlondon commented Jun 9, 2016

To make matters worse, this error signals a "disconnect" from the database. In my case my app will immediately attempt a re-connection which then I guess must retry the failing query. This repeats until the browser runs out of memory / tab crashes:

2016-06-09_10-32-38

The stack trace for subsequent errors are interestingly non-titled (aka, "undefined") and stack traces look a bit different on exit but as you go down the stack you see the same sequence originating:

2016-06-09_10-26-49

@deontologician
Copy link
Contributor

ping @Tryneus , and I'll open a new issue for the client side problem of nulling out error messages since that seems unrelated to this particular error

@danielmewes danielmewes added this to the Next Major Release milestone Jun 9, 2016
@thurt
Copy link

thurt commented Jun 20, 2016

I just ran into this error as well.

If you update the document using the rethinkdb admin interface and add the '$hz_v$' field with a numeric value, the problem goes away.

What is the $hz_v$ field for? and is it a safe workaround to add the field with a numeric value from admin tools, as you suggest?

@danielmewes
Copy link
Member

@thurt It's used to detect when a Horizon document changes between reading it and rewriting it.
Horizon makes use of this when checking validation functions in security rules.

If you insert a new document via ReQL, it's save to set the $hz_v$ field to any integer (normally 0).

If you modify an existing document through ReQL, I recommend also incrementing the $hz_v$ field as part of the update (you can use a function with update or replace to increment the value atomically). If you don't do this, there's a risk that a Horizon validation function will verify against the old state of the document, allow a write to go through, and then replace the new modified document.
For most simple validation functions this isn't an issue, but in theory it could lead to security issues in your Horizon application if you're doing anything complex in the validation (especially if you enforce invariants that depend on both the old and new value in a single validator function).

@thurt
Copy link

thurt commented Jun 21, 2016

@danielmewes thank you for the explanation! it sounds like a simple kind of revision control.

Does this mean the value of $hz_v$ is always checked before committing a write operation from horizon client? and if it is a different value than expected, does it throws an error?

I will try to start poking around the code base some more. Is there a place I can look to understand this behavior?

@danielmewes
Copy link
Member

Does this mean the value of $hz_v$ is always checked before committing a write operation from horizon client? and if it is a different value than expected, does it throws an error?

Yes.

Is there a place I can look to understand this behavior?

@Tryneus ^^^^^

@alf
Copy link
Author

alf commented Jun 21, 2016

@alf
Copy link
Author

alf commented Jun 21, 2016

@danielmewes Is it by design that horizon refuses to touch documents without a version field? If it is then maybe there could be an option to make horizon add the field on demand?

@danielmewes
Copy link
Member

@alf No, we are planning to add a default handler so that documents without version fields can still be updated through Horizon. That's why we're keeping this issue open.

@deontologician deontologician modified the milestones: Next Major Release, Medium term plans, Release 2.x.x Jun 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants