Upload in file-drop public link flattens folder hierarchy

[skshetry]

Steps to reproduce

1. Create a files-drop link.
2. Open the files-drop link.

Expected behaviour

Some way to upload folder

Actual behaviour

No way to upload a folder. Dragging a folder uploads the contents of the folder (recurse through it and flattens them).
Also, using setValue and trying to upload folder through Nightwatch creates an empty file.

[PVince81]

@skshetry mind trying this with OC 10 ?

as far as I remember we don't support subdirectories in files drop mode so this might be expected behavior

[skshetry]

@PVince81, Folder uploads in OC 10 are not supported at all. Close this or is this a requirement (it is listed on #2222)?

Also, phoenix should not upload folder as an empty file nor should it flatten them and upload inside a same folder.

[PVince81]

not a requirement imo, it would mess up the concept and make it more complex as we'd need to avoid file name collisions even across folders.

@pmaier1 thoughts ?

[PVince81]

@pmaier1 please reopen if you think we want to change how it works in Phoenix

[skshetry]

@PVince81, then, I would not expect the contents of sub-folders getting uploaded in the root of the publicly-shared folder (at least they should be in their respective sub-folders).

Re-opening, feel free to close if you don't think, that is an issue.

[PVince81]

@skshetry you're right. We need to block this with a warning.

[pascalwengerter]

@skshetry could you retry this with all the changes that have occured in the last ~1,5 years? Should work fine now 😉

[dpakach]

@pascalwengerter, uploading folders seems to work but all the files in the folder will be flattened.
for example, if we upload a folder with the following structure

├── upload
│   ├── file1.pdf
│   ├── file.ttf
│   └── sub
│       └── file2.pdf

it will upload all 3 files file1.pdf, file.ttf, and file2.pdf will be uploaded in the same folder. Is that correct behavior?

Uploading a similar folder in the main files section keeps the folder structure of the originally uploaded folder.

[saw-jan]

@pascalwengerter
Is the current behavior the expected one?

[pascalwengerter]

@pascalwengerter
Is the current behavior the expected one?

No, should upload the folder including files & subfolders the way they were structured before

[kulmann]

Uploads have been mostly re-implemented in a recent pull request (#6202). Issue still seems to be present. Since this corrupts uploaded data / modifies uploaded data in an unexpected way I've decided to elevate this to a P1.

[JammingBen]

@pmaier1

We noticed some inconsistencies between oC10 and oCIS when uploading contents to a public drop-page:

When uploading a file that already exists, oC10 will create a new file with a suffix (e.g. file (1)). oCIS in the other hand just overwrites the existing file by creating a new version. What is the desired behavior here? Also, how do we deal with folder uploads in this case? The classic UI does not support folder uploads on public drop-pages, but the new WebUI does.

[pmaier1]

When uploading a file that already exists, oC10 will create a new file with a suffix (e.g. file (1)). oCIS in the other hand just overwrites the existing file by creating a new version. What is the desired behavior here?

Thanks for pointing at this. IMO the oC10 behavior is correct. Main reason for that is that we should cover for the case that occurs when multiple people upload files with the same name to a file drop link. In that case it does not make sense to create versions as the files most probably really are distinct except that they have the same name. Same behavior for folders then.
IIRC there was some ticket in ocis or reva repo discussion this behavior but I can't find it atm.

Apart from that, I think the versioning feature should not be exposed at all in the context of a (public) link.

[pmaier1]

IIRC there was some ticket in ocis or reva repo discussion this behavior but I can't find it atm.

Found it

• Overwrite must be forbidden on public links with drop zone cs3org/reva#2480
• Forbidden overwrite in public links with drop zone cs3org/reva#2510

The behavior is described, discussed and agreed there. cc @micbar

[JammingBen]

Thx for the input!

Then I guess Web just sends the data to oCIS, which checks and handles the duplicate case. Technically, this is a bit challenging for folders. Because with folder uploads, Web first creates all folders and then uploads resources to those. Now if oCIS stored My Folder as My Folder (1), Web would need to know that to properly upload the following files. Not sure yet how the best solution looks like... cc @kulmann

[phil-davis]

The response to the initial create of a folder is going to return some HTTP status 2nn. The response could also have the actual "allocated" resource name as a data item in the response body. Then the client can know what name the folder has, and can then create the rest of the folder structure under that folder name.

The other approach would be that if the folder already exists, then the server can respond with something like 409 "conflict", and in the response body provide a suggested alternative folder name xyz (1) for example. The problem with that is that the client has to loop possibly multiple times. By the time the client gets the response and then sends a request to create xyz (1), some other client might have done the same, and the server will now respond again with 409 and suggest xyz (2) ...

[pascalwengerter]

The response to the initial create of a folder is going to return some HTTP status 2nn. The response could also have the actual "allocated" resource name as a data item in the response body. Then the client can know what name the folder has, and can then create the rest of the folder structure under that folder name.

The other approach would be that if the folder already exists, then the server can respond with something like 409 "conflict", and in the response body provide a suggested alternative folder name xyz (1) for example. The problem with that is that the client has to loop possibly multiple times. By the time the client gets the response and then sends a request to create xyz (1), some other client might have done the same, and the server will now respond again with 409 and suggest xyz (2) ...

There is also a security concern about not disclosing what files&folders are present in whatever folder you create a dropLink for - which means the backend should "silently swallow" whatever the public uploads there and then handle renaming backend-only

[JammingBen]

I'm all in favor of letting the backend handle this, hence the first approach sounds good to me:

The response to the initial create of a folder is going to return some HTTP status 2nn. The response could also have the actual "allocated" resource name as a data item in the response body. Then the client can know what name the folder has, and can then create the rest of the folder structure under that folder name.

However, either way, the client will know that there is a resource with that exact name. I'm not sure if that is information we want to publish to the outside world.

[pascalwengerter]

I'm all in favor of letting the backend handle this, hence the first approach sounds good to me:

The response to the initial create of a folder is going to return some HTTP status 2nn. The response could also have the actual "allocated" resource name as a data item in the response body. Then the client can know what name the folder has, and can then create the rest of the folder structure under that folder name.

However, either way, the client will know that there is a resource with that exact name. I'm not sure if that is information we want to publish to the outside world.

Can't we handle subsequent (child folder) uploads based on the resourceId of the previously uploaded (parent) resource, hence not disclosing anything about potential renaming happening?

[pmaier1]

There is also a security concern about not disclosing what files&folders are present in whatever folder you create a dropLink for - which means the backend should "silently swallow" whatever the public uploads there and then handle renaming backend-only

Exactly, this is important. Clients must not have any way to infer the content of the file drop folder.

[phil-davis]

Can't we handle subsequent (child folder) uploads based on the resourceId of the previously uploaded (parent) resource, hence not disclosing anything about potential renaming happening?

That sounds promising - the response to the "create resource" request can provide a resourceId which is a "magic string" unrelated to the resource name. The client can then create child resources under that.

[JammingBen]

Good idea, AFAIK this would need to be implemented first though. The ownCloud SDK (and oCIS itself?) only works with paths and names.

[JammingBen]

I researched a bit on how things work and what needs to be done if we aim for this solution. There's quite a lot unfortunately:

• oCIS must return the resourceIds of newly created folders (is this valid though?)
• oCIS must implement a way to support file uploads via resourceId
• SDK must be adjusted to support file uploads via resourceId
• Folder upload mechanism must be adjusted in Web to match this process

[PrajwolAmatya]

This issue doesn't seem to occur in latest master. Upload folder to public link share works fine and also the issue mentioned in this comment #2443 (comment), the structure of the folder is not affected.
Checked manually by uploading the folder with following structure in a public link share.

├── upload
│   ├── file1.txt
│   ├── file2.txt
│   └── sub
│       ├── file1.pdf
│       └── file2.pdf

The folder is uploaded with the same folder structure. The folder upload is currently not supported in playwright, so only tested manually. So, closing this issue for now.
CC @ScharfViktor