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

Copying a file from within a public link folder to "/" overwrites the parent folder #1230

Closed
jasson99 opened this issue Jul 10, 2020 · 10 comments
Assignees
Labels
Priority:p2-high Escalation, on top of current planning, release blocker Type:Bug

Comments

@jasson99
Copy link
Contributor

Description

When we copy file within a public link folder to a file with unusual destination names like the same file name or with no name, then the behaviour in core and ocis are different.

Steps to reproduce:

  1. Create a user Alice
  2. User Alice creates a folder PARENT
  3. User Alice uploads a file with content some data to PARENT/testfile.txt
  4. User Alice creates a public link share with path PARENT and permissions read,update,create,delete
    [User gets a token in the response eg:mVwPFKlJKNDOSx8]

In OC:

  1. The public copies file testfile.txt to "testfile.txt" or to " " using new public webdav api
curl -X COPY -H "Destination: http://172.17.0.1/oc/remote.php/dav/public-files/mVwPFKlJKNDOSx8/testfile.txt" 'http://172.17.0.1/oc/remote.php/dav/public-files/mVwPFKlJKNDOSx8/testfile.txt' -v

HTTP/1.1 403 Forbidden

<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\Forbidden</s:exception>
  <s:message>Source and destination uri are identical.</s:message>
</d:error>



curl -X COPY -H "Destination: http://172.17.0.1/oc/remote.php/dav/public-files/mVwPFKlJKNDOSx8/" 'http://172.17.0.1/oc/remote.php/dav/public-files/mVwPFKlJKNDOSx8/testfile.txt' -v

HTTP/1.1 403 Forbidden

<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\Forbidden</s:exception>
  <s:message>Permission denied to delete a resource</s:message>
</d:error>

In Ocis:

  1. The public copies file testfile.txt to " " using new public webdav api
curl -X COPY -H "Destination: https://localhost:9200/remote.php/dav/public-files/FYJrUstxwjQM/" 'https://localhost:9200/remote.php/dav/public-files/FYJrUstxwjQM/testfile.txt' -k -v 

< HTTP/1.1 204 No Content

[Same response when the public copies file "testfile.txt" to "testfile.txt"]

  1. The content of file "PARENT/" for user "Alice" is:
curl -u Alice:1234 "http://localhost:9140/remote.php/webdav/PARENT"   
                  
some data%                             
@individual-it individual-it changed the title Different behaviour on copying file within a public link folder to a file with unusual destination names copying a file from within a public link folder to "/" overwrites the parent folder Jul 13, 2020
@butonic butonic transferred this issue from owncloud/ocis-reva Jan 18, 2021
@butonic butonic transferred this issue from owncloud/core Jan 18, 2021
@refs refs changed the title copying a file from within a public link folder to "/" overwrites the parent folder Copying a file from within a public link folder to "/" overwrites the parent folder Jan 18, 2021
@settings settings bot removed the p3-medium label Apr 7, 2021
@ScharfViktor
Copy link
Contributor

@butonic I doble checked it. it still relevant and we have test for that: second case BREAKS data

image

case1 public copies testfile.txt to testfile.txt - 204 OK Alice can see file content. nothing crime
https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiSharePublicLink2/copyFromPublicLink.feature#L177

case2 public copies testfile.txt to the folder 😨 - result: 500 error. Alice looses folder with file
https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiSharePublicLink2/copyFromPublicLink.feature#L178

@ScharfViktor ScharfViktor added the Priority:p2-high Escalation, on top of current planning, release blocker label Mar 5, 2024
@micbar micbar moved this from Qualification to Prio 2 in Infinite Scale Team Board Mar 5, 2024
@dragonchaser dragonchaser self-assigned this Mar 11, 2024
@dragonchaser dragonchaser moved this from Prio 2 to In progress in Infinite Scale Team Board Mar 11, 2024
@dragonchaser
Copy link
Member

Also the public link is still existing and can be accessed (expectation: error message "An error occured while loading the public link")

@dragonchaser
Copy link
Member

This also works the other way around:

If you have a folder inside that public share and copy a file using the folders name as target, it will also kill the folder & its contents. 🥳 trying to fix this aswell....

@dragonchaser
Copy link
Member

After discussion with @individual-it and @2403905 the overwriting of child items is expected.

But the initial issue also applies for MOVE ....

@saw-jan
Copy link
Member

saw-jan commented Mar 19, 2024

@dragonchaser I noticed this behavior:

  1. I have this folder tree
parent/child/afile.txt
  1. create public share of parent folder with edit permissions
  2. public COPY folder child to child
curl -XCOPY "https://localhost:9200/remote.php/dav/public-files/BzAzalNWGXrTnFk/child" \
-H"DESTINATION: https://localhost:9200/remote.php/dav/public-files/BzAzalNWGXrTnFk/child"
< HTTP/1.1 204 No Content

Result:
parent/child - exists
parent/child/afile.txt - removed
parent/child - is in the trashbin

Is this the expected behavior?

@individual-it
Copy link
Member

I think this request should fail because:

  1. source and destination are the same (maybe can be discussed I cannot find anything about that in the RFC)
  2. there is no overwrite header set

and even if we allow to overwrite itself the data should not be lost

@saw-jan could you check the behavior

  1. file overwriting itself
  2. with overwrite header
  3. all of the cases with webDAV on the spaces endpoint

and then create a new issue

@saw-jan
Copy link
Member

saw-jan commented Mar 19, 2024

the behavior is not a regression because it was there since ages

COPY to same FILE:

  • webdav - 204 (file content exists)
  • dav/files - 204 (file content exists)
  • dav/spaces - 204 (file content exists)

COPY to same FOLDER:

  • webdav - 204 (folder exists but folder content is moved to trashbin)
  • dav/files - 204 (folder exists but folder content is moved to trashbin)
  • dav/spaces - 500

With Overwrite: F header, all requests return 412 Precondition Failed

@phil-davis
Copy link
Contributor

I suppose that "COPY to same FOLDER" should always return 204 (indicating that it worked) and in the storage it can actually just do nothing, because the required destination data (folder/file tree) is already there (it is both the source and destination).

I guess that would be easy to implement at some reasonably high level when processing the incoming request - detect that the request is really "a silly NO-OP", just check that the source folder does exist, do not touch the storage, return 204.

@dragonchaser
Copy link
Member

IMHO this should be a new issue, can we have a chat about this tomorrow after standup?

@individual-it
Copy link
Member

issue created in #8711

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:p2-high Escalation, on top of current planning, release blocker Type:Bug
Projects
Archived in project
Development

No branches or pull requests

7 participants