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

BUG: Recreating deleted document can break replication when VDU function is active #1418

Open
ondra-novak opened this issue Jun 28, 2018 · 4 comments
Labels

Comments

@ondra-novak
Copy link

ondra-novak commented Jun 28, 2018

It is possible to recreate (i mean: to update a deleted document with new content which hasn't the flag _deleted set to true) a document without need to specify revision field (_rev). However this behaviour has been changed somehow in CouchDB 2+.

In CouchDB 1.X, one can recreate document with the correct revision. It is also possible to recreate document without the revision and in this case, the CouchDB 1.X uses the document's recent revision. The rest of the operation is similar to updating existing document, including how the VDU (validate_doc_update) is called.

Consider following declaration (formatted to achieve better reading)

"validate_doc_update": function(newDoc, oldDoc, userCtx) {

		log(["validation oldDoc, newDoc, userCtx", oldDoc, newDoc, userCtx]);
 		function unchanged(field) {			
      		         if (oldDoc && (field in oldDoc) && toJSON(oldDoc[field]) != toJSON(newDoc[field]))
      		         throw({forbidden : "Field can't be changed: " + field});
  		};
		unchanged("owner");

}" 

In normal operation, oldDoc contains old document, newDoc contains new document. If the document is being created, the function is called with oldDoc == null. For all other updates the function is called with oldDoc equal to the body of the document being updated.

In CouchDB 1+ this is also applied for deleted documents, when the document is recreated, regardless on whether it is recreated with _rev or without the oldDoc contains the body of the deleted document.

In CouchDB 2+ this is no longer true. When the document is being recreated, oldDoc is always null. This behaviour can break replication in situation, when the replication is not continuous and when the document is deleted and recreated between each replication round.

Expected Behavior

If the document is recreated, the oldDoc of VDU should contain the body of previously deleted document

Current Behavior

If the document is recreated, the oldDoc of VDU is set to null

Possible Solution

The recreating of the document is mistakenly handled as a creation of the new document. It should be handled as an update.

Steps to Reproduce (for bugs)

  1. Prepare a design document with VDU defined above
  2. Create two databases (db1, db2) and put that design document in the both of them
  3. In the first database (db1), create document with a field "owner". For example
{
  "_id": "bug",
  "owner": "jack"
}
  1. ensure, that owner cannot be changed.
  2. also ensure, that document cannot be deleted using DELETE (VDU blocks correctly)
  3. replicate db1 to db2
  4. in the first database (db1), delete the document "bug" by adding "_deleted":true to the document
  5. recreate document "bug" with other owner (this will work, because VDU is called with null)
{
  "_id": "bug",
  "owner": "marie"
}
  1. replicate db1 to db2
  2. Result: document is not replicated. There is also error message sitting in the LOG file
Replicator: couldn't write document `bug`, revision `4-e0cc418137015d862ef83eb6d714b21c`, to target database `http:https://localhost:5984/db2/`. Error: `forbidden`, reason: `Field can't be changed: owner`.

This bug is dangerous in a one way. There is no way to detect, that the document is being recreated so the creator can create a document which suddenly cannot be replicated. There is also no way how to prevent such situation, because the VDU cannot distinguish between creation and recreation

creation:

Log :: ["validation oldDoc, newDoc, userCtx",null,{"_id":"loc.test","owner":"jack"},{"db":"db1","name":"admin","roles":["_admin"]}]

recreation:

Log :: ["validation oldDoc, newDoc, userCtx",null,{"_id":"loc.test","owner":"marie"},{"db":"db1","name":"admin","roles":["_admin"]}]

Context

The main goal of the checking the owner in the VDU is to prevent user to steal a document owned by other user.

Your Environment

  • Version used: 2.1.1
  • Browser Name and version: not relevant
  • Operating System and version (desktop or mobile): Ubuntu 16.04
  • Link to your project: not public
@janl janl added the bug label Jul 14, 2018
@bravier
Copy link

bravier commented Oct 16, 2018

Hello,

We are facing a similar issue as the one described by @ondra-novak on the _users database.

Steps to reproduce

Let A and B be 2 CouchDB servers.

  • Create the _users database (if it does not exist yet)
    Request
    curl -X PUT http:https://$A:5984/_users
    Response
    {
      "ok": true
    }
  • Create a user document
    Request
    curl -X PUT http:https://$A:5984/_users/org.couchdb.user:[email protected] -d '{"name": "[email protected]", "type": "user", "roles": [], "password": "password"}'
    Response
    {
      "ok": true,
      "id": "org.couchdb.user:[email protected]",
      "rev": "1-19fa61446d76bcb9131a1b65c03b4c1c"
    }
  • Replicate _users from A to B
    Request
    curl -X POST http:https://$A:5984/_replicate \
     -d '{"source":"http:https://$A:5984/_users","target":"http:https://$B:5984/_users","create_target":true}' \
     -H "Content-Type: application/json"
    Response
    {
      "ok": true,
      "session_id": "...",
      "..."
    }
  • Delete the user document from A
    Request
    curl -X DELETE http:https://$A:5984/_users/org.couchdb.user:[email protected]?rev=1-19fa61446d76bcb9131a1b65c03b4c1c
    Response
    {
      "ok": true,
      "id": "org.couchdb.user:[email protected]",
      "rev": "2-43b4189bc429587a529beba79a037cad"
    }
  • Replicate _users from A to B once again
    Request
    curl -X POST http:https://$A:5984/_replicate \
     -d '{"source":"http:https://$A:5984/_users","target":"http:https://$B:5984/_users","create_target":true}' \
     -H "Content-Type: application/json"
    Response
    {
      "ok": true,
      "session_id": "...",
      "..."
    }
  • Re-create the same user document again on A
    Request
    curl -X PUT http:https://$A:5984/_users/org.couchdb.user:[email protected] -d '{"name": "[email protected]", "type": "user", "roles": [], "password": "password"}'
    Response
    {
      "ok": true,
      "id": "org.couchdb.user:[email protected]",
      "rev": "3-1a2c0e3876f314e08d80b1d236481a2d"
    }
  • Replicate _users from A to B
    Request
    curl -X POST http:https://$A:5984/_replicate \
     -d '{"source":"http:https://$A:5984/_users","target":"http:https://$B:5984/_users","create_target":true}' \
     -H "Content-Type: application/json"
    Response
    {
      "ok": true,
      "session_id": "...",
      "..."
    }

Expected behavior

The user document might be replicated from A to B and the number of documents on A/_users and B/_users might be equal to 2 (_design/_auth document + our user).

Current behavior

The document is not replicated and the number of documents on A/_users is equal to 2 but the number of documents on B/_users is equal to 1 (_design/_auth only).

Environment

  • Server A
    • OS: Fedora 28
    • CouchDB: 2.1.2
  • Server B
    • OS: CentOS 7.5.1804
    • CouchDB: 2.1.2

This issue is quite impacting for us as it silently breaks replication and can lead to data loss.

Thanks in advance for helping us on this bug.

Do not hesitate to ask me for more infos if needed.

bravier pushed a commit to adrienverge/coucharchive that referenced this issue Oct 16, 2018
This commit handles document creation when the document is not created
by replication because of the bug described below.

**Bug**

A bug exists in CouchDB when replicating a document that where
created, replicated, deleted, re-created (with the same ID)
and replicated again.

We expect CouchDB to create the document on the last replication as it
would normally do but it doesn't.

Another user already opened an issue on the CouchDB issue tracker.

See: apache/couchdb#1418

I have tested it locally and the bug reproduction procedure is available
on the issue.

To confirm this fix works I am just using `coucharchive` on the last
replication.
bravier pushed a commit to adrienverge/coucharchive that referenced this issue Oct 17, 2018
This commit handles document creation when the document is not created
by replication because of the bug described below.

**Bug**

A bug exists in CouchDB when replicating a document that where
created, replicated, deleted, re-created (with the same ID)
and replicated again.

We expect CouchDB to create the document on the last replication as it
would normally do but it doesn't.

Another user already opened an issue on the CouchDB issue tracker.

See: apache/couchdb#1418

I have tested it locally and the bug reproduction procedure is available
on the issue.

To confirm this fix works I am just using `coucharchive` on the last
replication.
bravier pushed a commit to adrienverge/coucharchive that referenced this issue Oct 17, 2018
This commit handles document creation when the document is not created
by replication because of the bug described below.

**Bug**

A bug exists in CouchDB when replicating a document that where
created, replicated, deleted, re-created (with the same ID)
and replicated again.

We expect CouchDB to create the document on the last replication as it
would normally do but it doesn't.

Another user already opened an issue on the CouchDB issue tracker.

See: apache/couchdb#1418

I have tested it locally and the bug reproduction procedure is available
on the issue.

To confirm this fix works I am just using `coucharchive` on the last
replication.
adrienverge pushed a commit to adrienverge/coucharchive that referenced this issue Oct 17, 2018
This commit handles document creation when the document is not created
by replication because of the bug described below.

**Bug**

A bug exists in CouchDB when replicating a document that where
created, replicated, deleted, re-created (with the same ID)
and replicated again.

We expect CouchDB to create the document on the last replication as it
would normally do but it doesn't.

Another user already opened an issue on the CouchDB issue tracker.

See: apache/couchdb#1418

I have tested it locally and the bug reproduction procedure is available
on the issue.

To confirm this fix works I am just using `coucharchive` on the last
replication.
@kocolosk
Copy link
Member

kocolosk commented Mar 5, 2019

So I know this was filed a long time ago, but - what a fascinating bug report!

I understand what's happening in @ondra-novak 's initial report, but it's not clear to me that @bravier is experiencing the same issue. In Ondra's case the validation failure happens because the replicator skips over the deletion, which means the target sees the old version (before the deletion) and the new version, and their owner fields don't match.

In @bravier 's case there's a replication after every edit, so it's definitely a different situation. I'm wondering if in that situation oldDoc really is present in the replicated edit path, and because the deletion did not keep the name property in the tombstone it fails to validate when the re-created doc shows up as an extension of the edit path. It's almost the converse of @ondra-novak 's report. Worth double-checking.

At a higher level ... I'm concerned a bit about the reliance on the presence of oldDoc in the VDU when a document was created without specifying any revision. Yes, in that case CouchDB will look to extend an existing edit branch, but I always thought of this as a performance optimization that was not intended to have user-facing consequences. Clearly it does, and folks are relying on it for reasonable use cases.

Separately, at some point we blocked the ability to supply the _rev of a deleted leaf edit when every leaf of the document is deleted. It's possible that this happened in 39df1d5. Oddly enough if some other branch of the document is not deleted we will let the operation against the deleted leaf go through. That seems like an unintentional inconsistency.

I don't have a great answer about how to proceed here, but figured I should write down a few details that I picked up because I happened to be looking at this part of the codebase for #1957 and @wohali pointed me to this ticket as an interesting edge case.

@adrienverge
Copy link
Contributor

This is an interesting edge case indeed!

I also experience the bug on CouchDB 2. My understanding is that it happens in this particular case:

  • step A: document is non-deleted, and correctly replicated ✔️
                 first rev       latest rev on source
                    ↘           ↙
    SOURCE SERVER    o----o----o
    TARGET SERVER    o----o----o
                                ↖
                                 latest rev on target
    
  • step B: document is deleted, and correctly replicated ✔️
                 first rev            tombstone rev (latest rev on source)
                    ↘                ↙
    SOURCE SERVER    o----o----o----x
    TARGET SERVER    o----o----o----x
                                     ↖
                                      tombstone rev (latest rev on target)
    
  • step C: document is re-created on source, but not replicated ⚠️
                                tombstone rev
                 first rev     (last common ancestor)     latest rev on source
                    ↘               ↓                    ↙
    SOURCE SERVER    o----o----o----x----o----o----o----o
    TARGET SERVER    o----o----o----x
                                     ↖
                                      tombstone rev (latest rev on target)
    

@komorebi-san
Copy link

I have got the same issue as well for the database.

Currently, i dont allow to delete but only remarked as "deleted". Haha

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

6 participants