Inserting a new document with an invalid _rev succeeds (against local databases) #4712

Closed
willholley opened this Issue Dec 31, 2015 · 4 comments

Comments

Projects
None yet
3 participants
@willholley
Member

willholley commented Dec 31, 2015

Against local databases, the bulkDocs function allows insertion of documents with invalid _revs.

For example:

var db = new PouchDB("test");
db.bulkDocs([{_id:"foo2",_rev:"1-bar"}]).then(function(results) { console.log(results);});

results in:

[ { id: "foo2", ok: true, rev: "2-f126b7c8c8c80fa0d7bd8d989d38cd56" } ]

I'd expect a conflict error in this case. Presumbably, the invalid rev is being used to generate the incremental revision (2-f...) and treated like an update.

@willholley willholley added the bug label Dec 31, 2015

@willholley willholley self-assigned this Dec 31, 2015

@willholley willholley changed the title from _bulk_docs allows insertion with invalid _rev to bulk_docs allows insertion with invalid _rev Dec 31, 2015

@willholley willholley changed the title from bulk_docs allows insertion with invalid _rev to bulkDocs allows insertion with invalid _rev Dec 31, 2015

@willholley willholley removed the bug label Dec 31, 2015

@willholley

This comment has been minimized.

Show comment
Hide comment
@willholley

willholley Dec 31, 2015

Member

it seems that CouchDB 1.6.1 behaves in the same way:

 $ curl 'http://127.0.0.1:5984/revtest' -XPUT
 {"ok":true}

 $ curl 'http://127.0.0.1:5984/revtest/_bulk_docs' -H 'Content-Type: application/json' --data-binary '{"docs":[{"_id":"foobar","_rev":"1-123"}]}'
 [{"ok":true,"id":"foobar","rev":"2-9af8fdb20d72f07cc94e26da4857e3ae"}]

however, Cloudant / CouchDB 2.0 returns an error (as I would expect):

 $ curl 'http://127.0.0.1:15984/revtest' -XPUT
 {"ok":true}

 $ curl 'http://127.0.0.1:15984/revtest2/_bulk_docs' -H 'Content-Type: application/json' --data-binary     '{"docs":[{"_id":"foobar","_rev":"1-123"}]}'
 [{"id":"foobar","error":"conflict","reason":"Document update conflict."}]

@janl / @kxepal is the CouchDB 2.0 behaviour expected / correct?

Member

willholley commented Dec 31, 2015

it seems that CouchDB 1.6.1 behaves in the same way:

 $ curl 'http://127.0.0.1:5984/revtest' -XPUT
 {"ok":true}

 $ curl 'http://127.0.0.1:5984/revtest/_bulk_docs' -H 'Content-Type: application/json' --data-binary '{"docs":[{"_id":"foobar","_rev":"1-123"}]}'
 [{"ok":true,"id":"foobar","rev":"2-9af8fdb20d72f07cc94e26da4857e3ae"}]

however, Cloudant / CouchDB 2.0 returns an error (as I would expect):

 $ curl 'http://127.0.0.1:15984/revtest' -XPUT
 {"ok":true}

 $ curl 'http://127.0.0.1:15984/revtest2/_bulk_docs' -H 'Content-Type: application/json' --data-binary     '{"docs":[{"_id":"foobar","_rev":"1-123"}]}'
 [{"id":"foobar","error":"conflict","reason":"Document update conflict."}]

@janl / @kxepal is the CouchDB 2.0 behaviour expected / correct?

@willholley willholley changed the title from bulkDocs allows insertion with invalid _rev to Inserting a new document with an invalid _rev succeeds (against local databases) Dec 31, 2015

@willholley

This comment has been minimized.

Show comment
Hide comment
@willholley

willholley Dec 31, 2015

Member

This also occurs with single doc insertions:

 var db = new PouchDB("test");
 db.put({_id:"foo",_rev:"1-bar"}).then(function(results) { console.log(results);})

returns:

 {ok: true, id: "foo", rev: "2-fb61ce36c134030c8ad3923c259cd60f"}
Member

willholley commented Dec 31, 2015

This also occurs with single doc insertions:

 var db = new PouchDB("test");
 db.put({_id:"foo",_rev:"1-bar"}).then(function(results) { console.log(results);})

returns:

 {ok: true, id: "foo", rev: "2-fb61ce36c134030c8ad3923c259cd60f"}

@willholley willholley added the bug label Dec 31, 2015

willholley added a commit that referenced this issue Dec 31, 2015

@kxepal

This comment has been minimized.

Show comment
Hide comment
@kxepal

kxepal Dec 31, 2015

cc @rnewson here as well, but for me new 2.0 behaviour is correct and the right.

kxepal commented Dec 31, 2015

cc @rnewson here as well, but for me new 2.0 behaviour is correct and the right.

willholley added a commit that referenced this issue Dec 31, 2015

(#4712) - generate conflict error when using unrecognised _rev
When inserting a new document with a specific _rev, we previously
allowed the insertion instead of generating a conflict error
(as CouchDB does).

It was a bit fiddly to figure out where to add this check but I settled
on processDocs.js. This commit adds a test that when inserting a document
new to the database (and new_edits is true), the root of the revision tree
must not be missing. Normally, PouchDB will generate a new, available
root node so the lack of this indicates that the user specified a _rev
that the database doesn't recognise.

Updates to a document which the database already knows about will not
flow down this code path - instead they go to updateDoc.js which performs
its own conflict test.

willholley added a commit that referenced this issue Dec 31, 2015

(#4712) - return conflict error when inserting using unknown _rev
When inserting a new document with a specific _rev, we previously
allowed the insertion instead of generating a conflict error
(as CouchDB does).

It was a bit fiddly to figure out where to add this check but I settled
on processDocs.js. This commit adds a test that when inserting a document
new to the database (and new_edits is true), the root of the revision tree
must not be missing. Normally, PouchDB will generate a new, available
root node so the lack of this indicates that the user specified a _rev
that the database doesn't recognise.

Updates to a document which the database already knows about will not
flow down this code path - instead they go to updateDoc.js which performs
its own conflict test.

willholley added a commit that referenced this issue Dec 31, 2015

(#4712) - return conflict error when inserting using unknown _rev
When inserting a new document with a specific _rev, we previously
allowed the insertion instead of generating a conflict error
(as CouchDB does).

It was a bit fiddly to figure out where to add this check but I settled
on processDocs.js. This commit adds a test that when inserting a document
new to the database (and new_edits is true), the root of the revision tree
must not be missing. Normally, PouchDB will generate a new, available
root node so the lack of this indicates that the user specified a _rev
that the database doesn't recognise.

Updates to a document which the database already knows about will not
flow down this code path - instead they go to updateDoc.js which performs
its own conflict test.

willholley added a commit that referenced this issue Jan 1, 2016

(#4712) - return conflict error when inserting using unknown _rev
When inserting a new document with a specific _rev, we previously
allowed the insertion instead of generating a conflict error
(as CouchDB does).

It was a bit fiddly to figure out where to add this check but I settled
on processDocs.js. This commit adds a test that when inserting a document
new to the database (and new_edits is true), the root of the revision tree
must not be missing. Normally, PouchDB will generate a new, available
root node so the lack of this indicates that the user specified a _rev
that the database doesn't recognise.

Updates to a document which the database already knows about will not
flow down this code path - instead they go to updateDoc.js which performs
its own conflict test.

willholley added a commit that referenced this issue Jan 2, 2016

(#4712) - return conflict error when inserting using unknown _rev
When inserting a new document with a specific _rev, we previously
allowed the insertion instead of generating a conflict error
(as CouchDB does).

It was a bit fiddly to figure out where to add this check but I settled
on processDocs.js. This commit adds a test that when inserting a document
new to the database (and new_edits is true), the root of the revision tree
must not be missing. Normally, PouchDB will generate a new, available
root node so the lack of this indicates that the user specified a _rev
that the database doesn't recognise.

Updates to a document which the database already knows about will not
flow down this code path - instead they go to updateDoc.js which performs
its own conflict test.
@daleharvey

This comment has been minimized.

Show comment
Hide comment
@daleharvey

daleharvey Jan 2, 2016

Member

Fixed by 70271ec

Member

daleharvey commented Jan 2, 2016

Fixed by 70271ec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment