Delete Custom Dav Properties on file delete

[micbar]

Steps to reproduce

1. Create custom DAV Property
2. Create File with DAV Property, gets written into oc_properties table
3. Delete the file

Expected behaviour

All DAV Properties for this fileID should be deleted

Actual behaviour

The orphaned properties remain in the database.

Server configuration

Operating system:
MAC
Web server:
Apache2
Database:
MySQL
PHP version:
7.1
ownCloud version: (see ownCloud admin page)
10.0.8

Updated from an older ownCloud or fresh install:
fresh
Where did you install ownCloud from:
git
Signing status (ownCloud 9.0 and above):

List of activated apps:

• activity: 2.3.7
• comments: 0.3.0
• dav: 0.3.2
• enterprise_key: 0.1.4
• federatedfilesharing: 0.3.1
• federation: 0.1.0
• files: 1.5.1
• files_external: 0.7.1
• files_lifecycle: 1.0.0
• files_sharing: 0.10.1
• files_texteditor: 2.2.1
• files_trashbin: 0.9.1
• files_versions: 1.3.0
• market: 0.2.5
• notifications: 0.3.2
• provisioning_api: 0.5.0
• systemtags: 0.3.0
• updatenotification: 0.2.1

Possible Solution

• Needs to be discussed
• Could be solved by introducing foreign keys with cascade delete

[micbar]

@PVince81 @DeepDiver1975

[ownclouders]

GitMate.io thinks possibly related issues are #27274 (delete), #13824 (Delete File), #2438 (files deleted on sync), #19140 (Consolidate deleted files in deleted folders), and #2716 (Cannot permanently delete deleted files).

[PVince81]

@micbar did you delete over Webdav or using PHP API ?

If you delete over Webdav on the files endpoint, the Sabre engine should already trigger our custom properties plugin and delete the properties.

It is know that if the deletion happens outside of it, the Sabre code is never initialized outside of Sabre routes so it has no chance to clean up.

[PVince81]

Possible solutions:

• foreign keys + cascading delete (also see Introduce foreign keys / cascading delete #13143)
• have a background job that cleans up orphaned properties

Whatever we do we'll need either a bkg job or a repair job to clean up existing installs out there, and that before telling the DB to add a foreign key (if we choose to go that route)

[micbar]

@PVince81
On 10.0.8 I do the follwing.

1. Create File in the WebUI (Text Editor)
2. See that a property has been created
3. Delete File in the WebUI
4. Property does not get deleted

[PVince81]

Argh, so it means that even the Sabre delete routine doesn't work

[micbar]

Same happens when deleting via Sync Client.

[micbar]

In Files Lifecycle, we access the custom properties via an Entity Mapper.
We could also use an event listener and delete via the Mapper.

[micbar]

@DeepDiver1975 suggested to use the Mapper everywhere.

[PVince81]

I think we need to move the custom properties code to a lower layer. Sabre layer is too high as it's unreachable from PHP API.

We need to move the custom properties to the PHP API, maybe the Node API or something.
Then rewire Sabre to use this API for accessing properties.
The API in question would internally use a Mapper.

For now the quick fix is to write a background job to clean up, as we did for many other orphaned entries cases.

[PVince81]

Assigning @sharidas to write a cleanup background job.

[sharidas]

I have done 3 tests here. One on master branch, second on stable10 branch and third on 10.0.8 branch. Summary is that on 10.0.8 branch this issue is found and it is not found on master branch and stable10 branch.
Below are the results captured from console:

Master branch

• Create an admin user
• Create a file a.txt
• Create a proppatch.xml file in the location where the curl command is executed. The proppatch.xml file has the content <?xml version="1.0" encoding="utf-8" ?><propertyupdate xmlns='DAV:'><set><prop><t:valnspace xmlns:t='http://example.com/neon/litmus/'><foo xmlns='http://bar'/></t:valnspace></prop></set></propertyupdate>
• Execute the curl command as shown below:

➜  owncloud3 git:(master) ✗ curl -k -D - -u admin:admin -X PROPPATCH -H "Content-Type: text/xml" --data-binary "@proppatch.xml" http://localhost/testing3/remote.php/webdav/a.txt
HTTP/1.1 207 Multi-Status
Date: Thu, 08 Nov 2018 10:02:35 GMT
Server: Apache/2.4.29 (Ubuntu)
Set-Cookie: ocwwacfgm4cp=cmc8ftvdaenblakrui8bpliqpa; path=/testing3; HttpOnly
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-store, no-cache, must-revalidate
Pragma: no-cache
Set-Cookie: oc_sessionPassphrase=0b2iFwmkUNoHK7A2rGQnqSvHhmCSSNfpJIwQAjKTPsub5jxZLSOztDnIpg6yIjccE%2FuW%2FDar4BuIHZTwIMdJxHeUNefW%2B8r7G8Lh9AE%2Ba%2FWtmZ4%2Bf1naJnDYcjNXHxAz; path=/testing3; HttpOnly
Content-Security-Policy: default-src 'none';
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
X-Robots-Tag: none
X-Download-Options: noopen
X-Permitted-Cross-Domain-Policies: none
Set-Cookie: ocwwacfgm4cp=l3mi6uod16f6cfsseqfc0bt9pp; path=/testing3; HttpOnly
Set-Cookie: cookie_test=test; expires=Thu, 08-Nov-2018 11:02:35 GMT; Max-Age=3600
Vary: Brief,Prefer
Content-Length: 346
Content-Type: application/xml; charset=utf-8

<?xml version="1.0"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns"><d:response><d:href>/testing3/remote.php/webdav/a.txt</d:href><d:propstat><d:prop><x1:valnspace xmlns:x1="http://example.com/neon/litmus/"/></d:prop><d:status>HTTP/1.1 200 OK</d:status></d:propstat></d:response></d:multistatus>%                                                                                               ➜  owncloud3 git:(master) ✗

• Verify the oc_properties table:

mysql> select * from oc_properties;                                                                                                                                                                               +----+--------+--------------------------------------------+---------------+
| id | fileid | propertyname                               | propertyvalue |
+----+--------+--------------------------------------------+---------------+
|  1 |     15 | {http://example.com/neon/litmus/}valnspace | Object        |
+----+--------+--------------------------------------------+---------------+
1 row in set (0.00 sec)

mysql>

• Delete the file a.txt from web UI and the oc_properties table content is as shown below:

mysql> select * from oc_properties;
Empty set (0.00 sec)

mysql>

Stable10 branch

• Create admin user
• Create a file a.txt
• Now try to execute the proppatch.xml file as shown below ( same procedure done on master branch ):

➜  owncloud3 git:(stable10) ✗ curl -k -D - -u admin:admin -X PROPPATCH -H "Content-Type: text/xml" --data-binary "@proppatch.xml" http://localhost/testing3/remote.php/webdav/a.txt
HTTP/1.1 207 Multi-Status
Date: Thu, 08 Nov 2018 10:08:57 GMT
Server: Apache/2.4.29 (Ubuntu)
Set-Cookie: ocs4y7c9js3p=ijfsn8vfbdmfkvleecpudqtqn5; path=/testing3; HttpOnly
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-store, no-cache, must-revalidate
Pragma: no-cache
Set-Cookie: oc_sessionPassphrase=8LHx3eWH8lD5VYyq95RWHldO3V3Ic3eTz1DtTGm1%2BRcg%2BoRHuYRdcxwCtRw%2BUYmCrmthccss0tlwT9tYKZrVMGNs3zcykorargaoT7s7xpn%2FVUu5X3CcNiXv5W291BQm; path=/testing3; HttpOnly
Content-Security-Policy: default-src 'none';
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
X-Robots-Tag: none
X-Download-Options: noopen
X-Permitted-Cross-Domain-Policies: none
Set-Cookie: ocs4y7c9js3p=98ca6d2ccodql5sv95jq2261g8; path=/testing3; HttpOnly
Set-Cookie: cookie_test=test; expires=Thu, 08-Nov-2018 11:08:57 GMT; Max-Age=3600
Vary: Brief,Prefer
Content-Length: 346
Content-Type: application/xml; charset=utf-8

<?xml version="1.0"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns"><d:response><d:href>/testing3/remote.php/webdav/a.txt</d:href><d:propstat><d:prop><x1:valnspace xmlns:x1="http://example.com/neon/litmus/"/></d:prop><d:status>HTTP/1.1 200 OK</d:status></d:propstat></d:response></d:multistatus>%                                                                                               ➜  owncloud3 git:(stable10) ✗

• verify the oc_properties table:

mysql> select * from oc_properties;
+----+--------+--------------------------------------------+---------------+
| id | fileid | propertyname                               | propertyvalue |
+----+--------+--------------------------------------------+---------------+
|  1 |     14 | {http://example.com/neon/litmus/}valnspace | Object        |
+----+--------+--------------------------------------------+---------------+
1 row in set (0.00 sec)

mysql>

• Now delete the file a.txt using web UI and verify the oc_properties table again:

mysql> select * from oc_properties;
+----+--------+--------------------------------------------+---------------+
| id | fileid | propertyname                               | propertyvalue |
+----+--------+--------------------------------------------+---------------+
|  1 |     14 | {http://example.com/neon/litmus/}valnspace | Object        |
+----+--------+--------------------------------------------+---------------+
1 row in set (0.00 sec)

mysql>

10.0.8 Version

• Create admin user
• Create a.txt file
• Execute the curl command with proppatch.xml file as shown below:

➜  owncloud3 git:(ef478a9735) ✗ curl -k -D - -u admin:admin -X PROPPATCH -H "Content-Type: text/xml" --data-binary "@proppatch.xml" http://localhost/testing3/remote.php/webdav/a.txt
HTTP/1.1 207 Multi-Status
Date: Thu, 08 Nov 2018 10:14:04 GMT
Server: Apache/2.4.29 (Ubuntu)
Set-Cookie: ocxd70yjioq0=6ba42cavcl5uiqeanv40gc0occ; path=/testing3; HttpOnly
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-store, no-cache, must-revalidate
Pragma: no-cache
Set-Cookie: oc_sessionPassphrase=es85jRJLrAEXDr9lGj1HaBMRTRVLc1WX7LcsYAmYexrc%2BiweLG1%2FkW1kNlTpj5FLbupDama%2BEhwDA8%2BW%2Bra%2FeDfG0A1k5ovcghzRNQalNcNJbL7Ovq%2FhTftis9eLXME9; path=/testing3; HttpOnly
Content-Security-Policy: default-src 'none';
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
X-Robots-Tag: none
X-Download-Options: noopen
X-Permitted-Cross-Domain-Policies: none
Set-Cookie: ocxd70yjioq0=99h15aqpnecdeolkv0097m8ljc; path=/testing3; HttpOnly
Set-Cookie: cookie_test=test; expires=Thu, 08-Nov-2018 11:14:04 GMT; Max-Age=3600
Vary: Brief,Prefer
Content-Length: 346
Content-Type: application/xml; charset=utf-8

<?xml version="1.0"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:oc="http://owncloud.org/ns"><d:response><d:href>/testing3/remote.php/webdav/a.txt</d:href><d:propstat><d:prop><x1:valnspace xmlns:x1="http://example.com/neon/litmus/"/></d:prop><d:status>HTTP/1.1 200 OK</d:status></d:propstat></d:response></d:multistatus>%                                                                                               ➜  owncloud3 git:(ef478a9735) ✗

• Verify oc_properties table:

mysql> select * from oc_properties;
+----+--------+--------------------------------------------+---------------+
| id | fileid | propertyname                               | propertyvalue |
+----+--------+--------------------------------------------+---------------+
|  1 |     14 | {http://example.com/neon/litmus/}valnspace | Object        |
+----+--------+--------------------------------------------+---------------+
1 row in set (0.00 sec)

mysql>

• Delete a.txt from the web UI and verify the oc_properties table

mysql> select * from oc_properties;
+----+--------+--------------------------------------------+---------------+
| id | fileid | propertyname                               | propertyvalue |
+----+--------+--------------------------------------------+---------------+
|  1 |     14 | {http://example.com/neon/litmus/}valnspace | Object        |
+----+--------+--------------------------------------------+---------------+
1 row in set (0.00 sec)

mysql>

• It is reproducible here.

[PVince81]

@sharidas thanks for the research. So this means the bug about "deleting over Webdav doesn't remove oc_properties" is already fixed on stable10, it's likely fixed on 10.0.10.

We still need to look into a way to clean up stray properties when they're removed using PHP APIs.