[WIP] Prevent creating part files from the UI #7514

PVince81 · 2014-03-03T17:42:54Z

Fixes #7496

This PR prevents users to create part files from the web UI.

Since we changed an important utility method (isValidFileName), a few things need to be retested:

Test that WebDAV upload still works
Test that sync client upload still works with small files (part files)
Test that sync client upload still works with small files (part files + chunking)
Test with external storage using a backend ownCloud with this branch (sync upload must still work) 🚫 FAIL
Test with external storage using a frontend ownCloud with this branch (sync upload must still
Improve error messages ? [minor]
merge this once ownCloud 7.0 is EoL -> 9.0

isValidFileName() now flags part files are invalid names.

Added file name validity check before renaming or uploading.

PVince81 · 2014-03-03T17:43:06Z

@schiesbn @nickvergessen FYI

PVince81 · 2014-03-04T14:36:17Z

Test sync client upload with encryption enabled
Test sync client download with encryption enabled

PVince81 · 2014-03-04T15:14:39Z

Looks like uploading to a backend OC with this PR branch doesn't work, probably because it's prevent it to create a part file there. Need to find a better way.

bantu · 2014-04-20T21:22:38Z

I am not sure this patch is such a good idea. Users should be able to call their files whatever they want. Have alternatives been considered?

schiessle · 2014-04-22T09:15:40Z

I am not sure this patch is such a good idea. Users should be able to call their files whatever they want. Have alternatives been considered?

Choosing .part files was some kind of the lowest common denominator. For example web browser also use the .part extension for not finished downloads. The sync client also don't sync .part files because typically you don't want to upload unfinished downloads.

We also discussed the idea to flag files in the filecache as "hidden" instead of using part files. I even started some prove of concept implementation some time ago but it turned out that this approach is also not that easy as we thought at the beginning. You can probably still find the closed pull request with this prove of concept which should also contains a discussion about the additional problems. So for now .part files are the solution and since we don't show/sync part files we shouldn't allow users to upload such files. Because this will leave the with files in their ownCloud, counting against their quota but we no option to access them or to delete the again.

Even if we come back to the idea of the filecache flag at some point in time, we should still add this fixes for now.

PVince81 · 2014-04-28T10:41:21Z

I'm kind of stuck on this as it seems to have unwanted side-effects.

I think the main problem with uploading part files is only that it is then impossible to delete them afterwards from the UI or sync client. So maybe a part file cleaner process might be an alternative ? Note that in some cases stray part files might still lie around due to older bugs in the sabre connector.

PVince81 · 2015-02-13T17:27:34Z

Just remembered why I was blocked here: we wanted to allow part file upload to remote ownCloud servers. BUT since OC 8.0 we already detect that and disable part files in the front ownCloud through 2e57fe9

This means that I should be able to continue this task 😄

PVince81 · 2015-02-13T17:29:04Z

remove part file handling in "\OC\Connector\Sabre\ObjectTree" and make it throw a 403 if a part file is uploaded

PVince81 · 2015-02-16T17:32:29Z

That one might be risky as it would break s2s connecting from old OC 7 versions to newer OC 8.1.
Maybe we should keep that special case for a bit longer. Something to think about.

PVince81 · 2015-04-21T16:48:39Z

Well, I could maybe recycle this PR and at least keep the parts that work and defer the other ones for later instead of trying to provide a full solution (which isn't possible yet due to some blocking issues)

PVince81 · 2015-04-21T16:50:43Z

... or go with a different approach that might require more duplicate code, but with less risk of breaking those special cases...

MorrisJobke · 2015-07-03T11:09:42Z

I guess this isn't needed anymore once we move the Web UI to webdav?

MorrisJobke · 2015-07-03T11:09:45Z

cc @PVince81 ^

PVince81 · 2015-07-03T11:14:19Z

This has nothing to do with WebDAV.

The problem is that users are able to upload files called "test.part" with web UI or WebDAV.
Of course, once we move the web UI to WebDAV there will be less code paths to cover.

But currently this PR is blocked because: if we completely prevent uploading part files, then federation will not work any more because older OC versions like OC 7 don't have #13160. This means OC 7 instances would not be able to connect to OC 8.2 any more.

I suggest we keep it open for now and reconsider maybe once OC 7 is out of support.

MorrisJobke · 2015-07-03T11:26:28Z

The problem is that users are able to upload files called "test.part" with web UI or WebDAV.
Of course, once we move the web UI to WebDAV there will be less code paths to cover.

#7496 <- there @schiesbn said, that upload of part files isn't possible via webdav. I just tested it and it works :(

Then you're right. Sorry for the noise

PVince81 · 2015-07-03T11:35:30Z

I don't really remember whether part files can or cannot be uploaded via WebDAV. If they don't then with a bit of luck moving the web UI to Webdav will fix it too 😄

I'll reconsider this PR once #12353 is closed.

requires Move files app to use WebDAV instead of custom PHP endpoints #12353

MorrisJobke · 2015-07-03T11:38:06Z

I just tested it and it works :(

With "it works" I meant "I can upload .part files without any problem. ;)

PVince81 · 2015-07-03T11:47:16Z

Haha okay. 😄

PVince81 · 2015-09-01T09:05:13Z

Can be revived when 7.0 is EOL'd

MorrisJobke · 2015-11-23T11:06:31Z

@PVince81 @DeepDiver1975 Is this still needed ... looking to the new upload webdav endpoint 😉

PVince81 · 2015-11-23T12:42:29Z

There is currently no new upload webdav endpoint, still to be done.

It won't change the main issue with this PR: the fact that this PR would break compatibility when connecting to OC 7 servers. However once OC 7 are not around any more, the work can continue.

PVince81 · 2016-02-11T10:38:41Z

Since OC 7 will still be around when OC 9 is out, moving to 9.1.

See #7514 (comment) for the explanation why.

MorrisJobke · 2016-05-12T08:03:47Z

@PVince81 What is the status of this now that stable7 is EoL?

PVince81 · 2016-05-12T08:09:39Z

Needs rebasing and careful retesting with fed shares to OC 8 and up.

I'll see if I can find some time before the freeze to revive this...

PVince81 · 2016-05-20T13:13:23Z

I'm closing this as this is old and needs to be redone anyway. No point in leaving a PR open for so long.
Let's track this with its ticket: #7496

lock · 2019-08-05T20:02:43Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Vincent Petry added 2 commits March 3, 2014 18:39

Part file names are now invalid

a853d94

isValidFileName() now flags part files are invalid names.

Prevent uploading or renaming to invalid file names

89f4e54

Added file name validity check before renaming or uploading.

nickvergessen added the 3 - To develop label Aug 18, 2014

scolebrook force-pushed the master branch from 4a4ea1d to 9dea79e Compare October 10, 2014 13:32

PVince81 mentioned this pull request Nov 24, 2014

don't allow upload/creation of .part files #7496

Closed

PVince81 mentioned this pull request Feb 16, 2015

Sabre Update to 2.1 #14151

Merged

8 tasks

LukasReschke changed the title ~~Prevent creating part files from the UI~~ [WIP] Prevent creating part files from the UI Feb 24, 2015

PVince81 mentioned this pull request Apr 21, 2015

Do not trash part files, delete directly #15787

Merged

PVince81 added this to the 9.0-next milestone Sep 1, 2015

PVince81 mentioned this pull request Oct 9, 2015

Directories with name "*.part" do not get copied over DavFS. #17661

Closed

PVince81 added the comp:filesystem label Nov 20, 2015

PVince81 added the app:files label Nov 20, 2015

PVince81 modified the milestones: 9.1-next, 9.0-current Feb 11, 2016

PVince81 self-assigned this May 12, 2016

PVince81 closed this May 20, 2016

PVince81 deleted the preventcreatingpartfiles branch May 20, 2016 13:13

lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Prevent creating part files from the UI #7514

[WIP] Prevent creating part files from the UI #7514

PVince81 commented Mar 3, 2014

PVince81 commented Mar 3, 2014

PVince81 commented Mar 4, 2014

PVince81 commented Mar 4, 2014

bantu commented Apr 20, 2014

schiessle commented Apr 22, 2014

PVince81 commented Apr 28, 2014

PVince81 commented Feb 13, 2015

PVince81 commented Feb 13, 2015

PVince81 commented Feb 16, 2015

PVince81 commented Apr 21, 2015

PVince81 commented Apr 21, 2015

MorrisJobke commented Jul 3, 2015

MorrisJobke commented Jul 3, 2015

PVince81 commented Jul 3, 2015

MorrisJobke commented Jul 3, 2015

PVince81 commented Jul 3, 2015

MorrisJobke commented Jul 3, 2015

PVince81 commented Jul 3, 2015

PVince81 commented Sep 1, 2015

MorrisJobke commented Nov 23, 2015

PVince81 commented Nov 23, 2015

PVince81 commented Feb 11, 2016

MorrisJobke commented May 12, 2016

PVince81 commented May 12, 2016

PVince81 commented May 20, 2016

lock bot commented Aug 5, 2019

[WIP] Prevent creating part files from the UI #7514

[WIP] Prevent creating part files from the UI #7514

Conversation

PVince81 commented Mar 3, 2014

PVince81 commented Mar 3, 2014

PVince81 commented Mar 4, 2014

PVince81 commented Mar 4, 2014

bantu commented Apr 20, 2014

schiessle commented Apr 22, 2014

PVince81 commented Apr 28, 2014

PVince81 commented Feb 13, 2015

PVince81 commented Feb 13, 2015

PVince81 commented Feb 16, 2015

PVince81 commented Apr 21, 2015

PVince81 commented Apr 21, 2015

MorrisJobke commented Jul 3, 2015

MorrisJobke commented Jul 3, 2015

PVince81 commented Jul 3, 2015

MorrisJobke commented Jul 3, 2015

PVince81 commented Jul 3, 2015

MorrisJobke commented Jul 3, 2015

PVince81 commented Jul 3, 2015

PVince81 commented Sep 1, 2015

MorrisJobke commented Nov 23, 2015

PVince81 commented Nov 23, 2015

PVince81 commented Feb 11, 2016

MorrisJobke commented May 12, 2016

PVince81 commented May 12, 2016

PVince81 commented May 20, 2016

lock bot commented Aug 5, 2019