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

[webdav] Wrong MOVE requests deletes directory #5777

Closed
dragotin opened this issue Nov 8, 2013 · 8 comments
Closed

[webdav] Wrong MOVE requests deletes directory #5777

dragotin opened this issue Nov 8, 2013 · 8 comments
Assignees
Labels
Milestone

Comments

@dragotin
Copy link
Contributor

dragotin commented Nov 8, 2013

Current master (near ownCloud 6.0.0 beta3) has the following problem:

During client development the client issued the this request:

2013-11-08 15:29:55 MOVE http://192.168.178.23/ocm/remote.php/webdav/slot4//test2
                         ← 404 application/xml 288B
Request                                                                 Response
User-Agent:        Mozilla/5.0 (Linux) csyncoC/0.90.4 neon/0.29.6
Connection:        TE
TE:                trailers
Host:              192.168.178.23
Proxy-Connection:  Keep-Alive
Destination:       http://192.168.178.23/ocm/remote.php/webdav/slot4/
Overwrite:         T
Cookie:            51d2ebccb3989=6buus2agdb19ngg60dhfjh3gddiepaoteia7q8js7ljso68tsdj0;

and the reply was

Date:            Fri, 08 Nov 2013 14:29:55 GMT
Server:          Apache/2.2.22 (Linux/SUSE)
X-Powered-By:    PHP/5.3.15
Expires:         Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control:   no-store, no-cache, must-revalidate, post-check=0, pre-check=0
Pragma:          no-cache
Content-Length:  288
Content-Type:    application/xml; charset=utf-8
XML-like data
<?xml version='1.0' encoding='utf-8'?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre_DAV_Exception_NotFound</s:exception>
  <s:message>File with name slot4/test2 could not be located</s:message>
  <s:sabredav-version>1.7.6</s:sabredav-version>
</d:error>

which seems to correct.

The problem with that is only that after this request, the directory slot4 is completely gone on the server.

At the same time, apache's error log shows

[Fri Nov 08 15:29:55 2013] [error] [client 192.168.178.23] delete hook /slot4

This is a severe problem because it means data loss, even if the WebDAV request might be bogus.

@guruz
Copy link
Contributor

guruz commented Nov 8, 2013

If I remember correctly, oC implements MOVE as COPY+DELETE.. Maybe there is some wrong error checking there.
But even then it is weird that slot4 is deleted (instead of test2...)

@dragotin
Copy link
Contributor Author

dragotin commented Nov 8, 2013

This is easy to reproduce with the MOVE of death:

 curl -n -X MOVE "http://localhost/ocm/remote.php/webdav/dir1/subdir" -H "Destination: http://localhost/ocm/remote.php/webdav/dir1/"

@ghost ghost assigned DeepDiver1975 Nov 11, 2013
@DeepDiver1975
Copy link
Member

reproduced - preparing a fix ....

@DeepDiver1975
Copy link
Member

As far as I can tell SabreDAV is deleting the destination folder because it exists
https://github.com/fruux/sabre-dav/blob/1.7.6/lib/Sabre/DAV/Server.php#L997

There is a check above to prevent source and destination being identical - the code comment is a bit unclear:
https://github.com/fruux/sabre-dav/blob/1.7.6/lib/Sabre/DAV/Server.php#L990

@evert in the master branch I see an additional check which seems to be missing in the 1.7 code base
https://github.com/fruux/sabre-dav/blob/master/lib/Sabre/DAV/Server.php#L740

Or did I miss anything? THX

@evert
Copy link

evert commented Nov 11, 2013

You're right. This was fixed on dev-master, but didn't seem like a priority to backport, because it would only affect broken clients.

@DeepDiver1975
Copy link
Member

@dragotin I'll move this to oc7

@PVince81
Copy link
Contributor

Is this still an issue ? We seem to have upgraded to Sabre 1.7.11 on master.

@dragotin
Copy link
Contributor Author

Yes, quick check with master works as expected. I think we can close this one.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants