OCFS properly process empty file upload

[PVince81]

We should fix the two TUS layers to properly process uploads with 0 bytes to automatically finish:

Fixes owncloud/ocis-reva#188

• Fix scenario 1: Webdav PUT upload of empty file
• Fix scenario 2: Webdav TUS upload of empty file

Scenario 1: Webdav PUT upload of empty file

In Phoenix when creating a new file, it uses a PUT with empty payload to create the file.
The PUT goes to ocdav/put.go and gets converted to a TUS upload to the backend.
In this PR the backend has a new shortcut to automatically finish the upload.

Scenario 2: TUS upload of empty file

In Phoenix, try uploading an empty file that exists on local disk.

In this scenario, the code flow goes into tus.go and initiates the upload in the backend storage.
However it doesn't create the file automatically despite the fix for scenario 1.
I tried to debug this and somehow it seems that it doesn't even reach owncloud/upload.go for the upload and aborts earlier.
I'm a bit confused about the code path between the file upload initiation and finishing the upload in the case of TUS. Maybe it's in the storage provider.

I also tried to make it send an empty PATCH to the backend but that makes it return 400 bad request.

Also: it is likely that we'll need a similar fix for EOS.

@butonic thoughts ?

[butonic]

reproduced using these two curl commands:

Scenario 1: Webdav PUT upload of empty file

curl 'https://localhost:9200/remote.php/webdav/empty%20file' \
  -X 'PUT' \
  -H 'Content-Length: 0' \
  --insecure \
  --verbose \
  -u einstein:relativity

Scenario 2: TUS upload of empty file

curl 'https://localhost:9200/remote.php/dav/files/einstein/' \
  -X 'POST' \
  -H 'Content-Length: 0' \
  -H 'Tus-Resumable: 1.0.0' \
  -H 'Upload-Length: 0' \
  -H 'Upload-Metadata: filename YWRkcmVzc2Jvb2s=,filetype ,size MA==,mtime MTQ5OTI1MTUzMS44NjE=' \
  --insecure \
  --verbose \
  -u einstein:relativity

[butonic]

I added a few log lines and ignoered deletion of non existing files:

diff --git a/pkg/storage/fs/owncloud/upload.go b/pkg/storage/fs/owncloud/upload.go
index 8b7be82..3b44aa2 100644
--- a/pkg/storage/fs/owncloud/upload.go
+++ b/pkg/storage/fs/owncloud/upload.go
@@ -158,6 +158,9 @@ func (fs *ocfs) NewUpload(ctx context.Context, info tusd.FileInfo) (upload tusd.
 
                "LogLevel": log.GetLevel().String(),
        }
+
+       log.Debug().Interface("info", info).Msg("ocfs: built storage info")
+
        // Create binary file in the upload folder with no content
        file, err := os.OpenFile(binPath, os.O_CREATE|os.O_WRONLY, defaultFilePerm)
        if err != nil {
@@ -175,6 +178,7 @@ func (fs *ocfs) NewUpload(ctx context.Context, info tusd.FileInfo) (upload tusd.
 
        if !info.SizeIsDeferred && info.Size == 0 {
                // no need to create info file and finish directly
+               log.Debug().Interface("info", info).Msg("ocfs: finishing upload for empty file")
                u.FinishUpload(ctx)
                return u, nil
        }
@@ -366,10 +370,14 @@ func (fs *ocfs) AsTerminatableUpload(upload tusd.Upload) tusd.TerminatableUpload
 // Terminate terminates the upload
 func (upload *fileUpload) Terminate(ctx context.Context) error {
        if err := os.Remove(upload.infoPath); err != nil {
-               return err
+               if !os.IsNotExist(err) {
+                       return err
+               }
        }
        if err := os.Remove(upload.binPath); err != nil {
-               return err
+               if !os.IsNotExist(err) {
+                       return err
+               }
        }
        return nil
 }

[PVince81]

Both scenarios work now for me locally, I was testing it wrong, forgot to restart a relevant service.

I've added @butonic's change to avoid errors when the info file is not there.
Also, as discussed with @butonic, added similar changes for EOS but I can't test them.

@butonic please test the EOS changes

[butonic]

works on eos when using the /webdav endpoint. Need to check the /dav/files endpoint because it does not seem to use the correct user layout. should be fixed with owncloud/ocis#219 so does not affect this PR

[PVince81]

I've fixed the linting issue and retested with OCFS to make sure there are no issues with the error condition check.

I think this is good to go