Wrap the entire dav PUT in a read lock

[icewind1991]

Acquire a read lock in the beforeMethod hook to ensure we have the file read locked when precondition checks are made.

Fixes #16569

cc @DeepDiver1975 @PVince81

[MorrisJobke]

Tested and works. Also fixes owncloud/files_texteditor#61 owncloud/files_texteditor#93

[MorrisJobke]

👍

[Henni]

@MorrisJobke does this really solve the frontend issue owncloud/files_texteditor#61 for you?

[MorrisJobke]

@MorrisJobke does this really solve the frontend issue owncloud/files_texteditor#61 for you?

Sorry. Wrong paste. It fixes owncloud/files_texteditor#93

[PVince81]

Breaks chunk upload:

{"reqId":"lfpmolA5MBbqxHztmKvS","remoteAddr":"127.0.0.1","app":"webdav","message":"Exception: {\"Message\":\"HTTP\\\/1.1 400 expected filesize 5242880 got 4866048\",\"Code\":0,\"Trace\":\"#0 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/private\\\/connector\\\/sabre\\\/file.php(100): OC\\\\Connector\\\\Sabre\\\\File->createFileChunked(Resource id #255)\\n#1 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/private\\\/connector\\\/sabre\\\/directory.php(115): OC\\\\Connector\\\\Sabre\\\\File->put(Resource id #255)\\n#2 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/3rdparty\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(1053): OC\\\\Connector\\\\Sabre\\\\Directory->createFile('data2.dat-chunk...', Resource id #255)\\n#3 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/3rdparty\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/CorePlugin.php(513): Sabre\\\\DAV\\\\Server->createFile('data2.dat-chunk...', Resource id #255, NULL)\\n#4 [internal function]: Sabre\\\\DAV\\\\CorePlugin->httpPut(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#5 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/3rdparty\\\/sabre\\\/event\\\/lib\\\/EventEmitterTrait.php(105): call_user_func_array(Array, Array)\\n#6 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/3rdparty\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(469): Sabre\\\\Event\\\\EventEmitter->emit('method:PUT', Array)\\n#7 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/3rdparty\\\/sabre\\\/dav\\\/lib\\\/DAV\\\/Server.php(254): Sabre\\\\DAV\\\\Server->invokeMethod(Object(Sabre\\\\HTTP\\\\Request), Object(Sabre\\\\HTTP\\\\Response))\\n#8 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/apps\\\/files\\\/appinfo\\\/remote.php(84): Sabre\\\\DAV\\\\Server->exec()\\n#9 \\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/remote.php(132): require_once('\\\/srv\\\/www\\\/htdocs...')\\n#10 {main}\",\"File\":\"\\\/srv\\\/www\\\/htdocs\\\/owncloud\\\/lib\\\/private\\\/connector\\\/sabre\\\/file.php\",\"Line\":340}","level":4,"time":"2015-08-06T15:58:13+00:00","method":"PUT","url":"\/owncloud\/remote.php\/webdav\/data2.dat-chunking-3130333669-25-0"}

This file size is 126679286.
Works correctly on master with locking enabled.

@icewind1991 when touching webdav upload code please always test with both non-chunked upload and chunked upload.

[ghost]

🚀 Test PASSed.🚀

[icewind1991]

chunked problems should all be fixed now (and covered with unit tests)

[MorrisJobke]

I tested this with and without file locking based on the webdav interface.

Worked so far 👍

[PVince81]

@MorrisJobke the problem was based on file chunking, so must also test with the sync client.

[PVince81]

With locking

• TEST: sync client chunk upload
• TEST: sync client chunk upload + overwrite
• TEST: webdav no-chunk upload
• TEST: webdav no-chunk upload + overwrite
• TEST: smashbox basic sync

Without locking

• TEST: sync client chunk upload
• TEST: sync client chunk upload + overwrite
• TEST: webdav no-chunk upload
• TEST: webdav no-chunk upload + overwrite
• TEST: smashbox basic sync

[PVince81]

Note that I am testing with encryption enabled too.
I'll continue testing later, I seem to have stumbled on an encryption issue. Need to check whether it happens on master or only on this branch.

[PVince81]

• BUG: encryption (no locking), sync client upload big file (120MB). Wait. Touch the file to trigger a reupload+overwrite. => file cannot be deleted/downloaded in web UI due to key issues, getting 403. This works properly on master.

[icewind1991]

BUG: encryption (no locking), sync client upload big file (120MB). Wait. Touch the file to trigger a reupload+overwrite. => file cannot be deleted/downloaded in web UI due to key issues, getting 403. This works properly on master.

I can't reproduce this, either manually or automated (#18901) and I have a hard time seeing how these changes can introduce an issue with locking disabled

[PVince81]

Can you rebase ?

[scrutinizer-notifier]

A new inspection was created.

[PVince81]

Seems the chunk upload with encryption works now. Let me retest the other cases too.

[PVince81]

Also I can confirm that the hooks still work as I could see versions created for both chunked and non-chunked overwritten files.

I can't test the smashbox stuff, it's broken on master: #19031

[icewind1991]

@PVince81 @DeepDiver1975 can this be merged?

[PVince81]

Ok to merge from me 👍

But please have a look at the failing smashbox tests on master.