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

MultipartUpload for uploading chunks to s3 may break other object store implementations #39088

Closed
5 of 8 tasks
EchedelleLR opened this issue Jun 30, 2023 · 16 comments · Fixed by #39432
Closed
5 of 8 tasks
Assignees
Labels
1. to develop Accepted and waiting to be taken care of 26-feedback bug

Comments

@EchedelleLR
Copy link

EchedelleLR commented Jun 30, 2023

⚠️ This issue respects the following points: ⚠️

Bug description

Currently, it is not possible to upload large files through the web interface (more than 10 MiB) using the web client starting by Nextcloud 26.

Other clients work. Everything works in Nextcloud 25.

An error is showed as the ObjectStore not supporting MultiPartUploads.

Steps to reproduce

  1. Install Nextcloud
  2. Install StorJ app
  3. Configure it as ObjectStore

Expected behavior

Being able to upload big files.

Installation method

Community Manual installation with Archive

Nextcloud Server version

26

Operating system

Debian/Ubuntu

PHP engine version

PHP 8.1

Web server

Apache (supported)

Database engine version

MariaDB

Is this bug present after an update or on a fresh install?

Fresh Nextcloud Server install

Are you using the Nextcloud Server Encryption module?

Encryption is Disabled

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

No response

List of activated Apps

No response

Nextcloud Signing status

{
  "reqId": "EQyBldlkEuO3u4jdtVqY",
  "level": 3,
  "time": "2023-06-30T10:56:49+00:00",
  "remoteAddr": "*.*.*.*",
  "user": "echedeylr",
  "app": "webdav",
  "method": "MKCOL",
  "url": "/remote.php/dav/uploads/echedeylr/web-file-upload-07010de993dce4c44dc3cc1b29e57d38-1688122602929",
  "message": "Object store does not support multipart upload",
  "userAgent": "Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Firefox/102.0",
  "version": "26.0.3.2",
  "exception": {
    "Exception": "OCP\\Files\\GenericFileException",
    "Message": "Object store does not support multipart upload",
    "Code": 0,
    "Trace": [
      {
        "function": "startChunkedWrite",
        "class": "OC\\Files\\ObjectStore\\ObjectStoreStorage",
        "type": "->"
      },
      {
        "file": "/var/www/cloud/lib/private/Files/Storage/Wrapper/Wrapper.php",
        "line": 524,
        "function": "call_user_func_array"
      },
      {
        "function": "__call",
        "class": "OC\\Files\\Storage\\Wrapper\\Wrapper",
        "type": "->"
      },
      {
        "file": "/var/www/cloud/lib/private/Files/Storage/Wrapper/Wrapper.php",
        "line": 524,
        "function": "call_user_func_array"
      },
      {
        "function": "__call",
        "class": "OC\\Files\\Storage\\Wrapper\\Wrapper",
        "type": "->"
      },
      {
        "file": "/var/www/cloud/lib/private/Files/Storage/Wrapper/Wrapper.php",
        "line": 524,
        "function": "call_user_func_array"
      },
      {
        "file": "/var/www/cloud/apps/dav/lib/Upload/ChunkingV2Plugin.php",
        "line": 133,
        "function": "__call",
        "class": "OC\\Files\\Storage\\Wrapper\\Wrapper",
        "type": "->"
      },
      {
        "file": "/var/www/cloud/3rdparty/sabre/event/lib/WildcardEmitterTrait.php",
        "line": 89,
        "function": "afterMkcol",
        "class": "OCA\\DAV\\Upload\\ChunkingV2Plugin",
        "type": "->"
      },
      {
        "file": "/var/www/cloud/3rdparty/sabre/dav/lib/DAV/Server.php",
        "line": 482,
        "function": "emit",
        "class": "Sabre\\DAV\\Server",
        "type": "->"
      },
      {
        "file": "/var/www/cloud/3rdparty/sabre/dav/lib/DAV/Server.php",
        "line": 253,
        "function": "invokeMethod",
        "class": "Sabre\\DAV\\Server",
        "type": "->"
      },
      {
        "file": "/var/www/cloud/3rdparty/sabre/dav/lib/DAV/Server.php",
        "line": 321,
        "function": "start",
        "class": "Sabre\\DAV\\Server",
        "type": "->"
      },
      {
        "file": "/var/www/cloud/apps/dav/lib/Server.php",
        "line": 366,
        "function": "exec",
        "class": "Sabre\\DAV\\Server",
        "type": "->"
      },
      {
        "file": "/var/www/cloud/apps/dav/appinfo/v2/remote.php",
        "line": 35,
        "function": "exec",
        "class": "OCA\\DAV\\Server",
        "type": "->"
      },
      {
        "file": "/var/www/cloud/remote.php",
        "line": 172,
        "args": [
          "/var/www/cloud/apps/dav/appinfo/v2/remote.php"
        ],
        "function": "require_once"
      }
    ],
    "File": "/var/www/cloud/lib/private/Files/ObjectStore/ObjectStoreStorage.php",
    "Line": 638,
    "message": "Object store does not support multipart upload",
    "exception": [],
    "CustomMessage": "Object store does not support multipart upload"
  },
  "id": "649f14d4a11ba"
}

Nextcloud Logs

No response

Additional info

  It seems that the #27034 has been merged to master and 26.0.0 recently.

  However, this feature also needs the client to support it (since it has limitations). As per my trial, the newest desktop version didn't implement it yet. I have submitted an issue here: https://github.com/nextcloud/desktop/issues/5554. Please upvote that issue if you need this feature.

_Originally posted by @ZE3kr in https://github.com/nextcloud/server/issues/19414#issuecomment-1489598976_

PR: #27034

@solracsf solracsf added bug 0. Needs triage Pending check for reproducibility or if it fits our roadmap 26-feedback labels Jun 30, 2023
@kesselb
Copy link
Contributor

kesselb commented Jun 30, 2023

Please update your comment to use our usual template for bug reports: https://github.com/nextcloud/server/issues/new?assignees=&labels=bug%2C0.+Needs+triage&projects=&template=BUG_REPORT.yml&title=%5BBug%5D%3A+

Also provide more details about your StorJ setup.
There is an app for Nextcloud, but you mounted it as s3 bucket?

The error is emitted here:

if (!$this->objectStore instanceof IObjectStoreMultiPartUpload) {
throw new GenericFileException('Object store does not support multipart upload');
}

That means, the provided object store instance does not implement the given interface. But our default object store backend for s3 does implement it, and therefore we need to know which object store you are using?

@EchedelleLR
Copy link
Author

I am using the native StorJ app in the store. Currently not able to link it here from mobile.

The issue doesn't happen in Nextcloud 25 and it does happen in Nextcloud 26 just in the web interface.

I am using the object store from its extension.

Related to the issue, I created it using the GitHub feature of creating an issue quoting other directly which didn't show me the default template.

I already reported the issue time before in another issue but it seems that got hidden because the original author had an issue with their web server config and was determined to not be the same.

@EchedelleLR
Copy link
Author

I already filled it.

@kesselb
Copy link
Contributor

kesselb commented Jun 30, 2023

https://apps.nextcloud.com/apps/storj is not released for Nextcloud 26.

Please append the configuration report (don't forget to remove sensitive data).
There should be a log record for the given error with additional information (e.g., a stack trace to know which path the code used).

@kesselb kesselb changed the title #27034 may be breaking S3-compatible storages MultipartUpload for uploading chunks to s3 may break other object store implementations Jun 30, 2023
@kesselb
Copy link
Contributor

kesselb commented Jun 30, 2023

cc @juliushaertl

ObjectStoreStorage implements IChunkedFileWrite and therefore provides startChunkedWrite, putChunkedWritePart, completeChunkedWrite and cancelChunkedWrite.

But ObjectStoreStorage checks, if the concrect object store backend (Azure, S3 or Swift) implements IObjectStoreMultiPartUpload, which is only the case for S3.

Isn't there something missing to route requests to the default chunking plugin if IObjectStoreMultiPartUpload is not implemented?

@EchedelleLR
Copy link
Author

EchedelleLR commented Jun 30, 2023

https://apps.nextcloud.com/apps/storj is not released for Nextcloud 26.

I understand the implications but two things first:

  • it doesn't fail with clients, just with the default web client
  • you use "Not tested" instead of "Not supported" which is a confusing nomenclature. It seems to imply that upstream didn't report supporting or not that Nextcloud version, implying that feature support is not validated at Nextcloud upstream side but by app upstream side and is based on trust.

Given the first implication, I thought that the second was not something to be taken into account since StorJ (based on what I expected) should not be dependent on client support. If a client change is causing the issue and this affects the way files in background are uploaded, why is a plugin issue?

That was my first motivation to make the report. I apologize for the initial format and not re-checking the template. I tried to provide what I considered basic information (and failed on it).

If there is a scope issue of what should be from app upstream development and what Nextcloud upstream development, I would like to know as it is not very clear for me.

@kesselb
Copy link
Contributor

kesselb commented Jun 30, 2023

Don't worry ;)

Thank you for your bug report. We have the required information now.

The main point for reporting a bug is to provide a clear path how to reproduce a problem. This might be a stack trace (the best option when available) or a description. For UI bugs, screenshots are often helpful. The template is there to make it easier. Too bad it won't work in "Reference in new issue".

@kesselb kesselb added 1. to develop Accepted and waiting to be taken care of and removed needs info 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Jun 30, 2023
@EchedelleLR
Copy link
Author

EchedelleLR commented Jun 30, 2023

BTW, I forgot to mention that the error is not showed in screen to the user end in the web client. Just nothing happens.

I found the error while digging the error in real time and checking the log.

I remember seeing a web console error but could not reproduce it last time.

@github-project-automation github-project-automation bot moved this to 🧭 Planning evaluation (don't pick) in 📝 Office team Jul 4, 2023
@AndyScherzinger AndyScherzinger moved this from 🧭 Planning evaluation (don't pick) to 📄 To do (~10 entries) in 📝 Office team Jul 4, 2023
@AndyScherzinger AndyScherzinger moved this from 📄 To do (~10 entries) to 🧭 Planning evaluation (don't pick) in 📝 Office team Jul 4, 2023
@kesselb
Copy link
Contributor

kesselb commented Jul 7, 2023

Index: apps/dav/lib/Upload/ChunkingV2Plugin.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/apps/dav/lib/Upload/ChunkingV2Plugin.php b/apps/dav/lib/Upload/ChunkingV2Plugin.php
--- a/apps/dav/lib/Upload/ChunkingV2Plugin.php	(revision 3d2fb2934799f34616a75ad0ad19b6d07596aadf)
+++ b/apps/dav/lib/Upload/ChunkingV2Plugin.php	(date 1688727607153)
@@ -278,6 +278,9 @@
 		if (!$this->uploadFolder->getStorage()->instanceOfStorage(IChunkedFileWrite::class)) {
 			throw new StorageInvalidException('Storage does not support chunked file writing');
 		}
+		if ($this->uploadFolder->getStorage()->instanceOfStorage(ObjectStoreStorage::class) && !$this->uploadFolder->getStorage()->getObjectStore() instanceof IObjectStoreMultiPartUpload) {
+			throw new StorageInvalidException('Storage does not support multi part uploads');
+		}
 
 		if ($checkUploadMetadata) {
 			if ($this->uploadId === null || $this->uploadPath === null) {

@EchedeyLR mind to try the patch?

@EchedelleLR
Copy link
Author

EchedelleLR commented Jul 7, 2023

As far as I can see, this only adds a check to throw a previous exception to the error I have (I mean, is a previous part of the code) (I would change the message or would integrate it with the previous statement creating a more complex one in the official code).

Would this fallback to the default chunked mode or how?

@kesselb
Copy link
Contributor

kesselb commented Jul 7, 2023

Would this fallback to the default chunked mode or how?

That's the plan, please let me know if it works for your setup as well.
I updated the patch, to use a different error message.

@EchedelleLR
Copy link
Author

@kesselb it is working as far as I could test.

Upload is now allowed from the web client.

Let me see it it finishes properly to confirm that there is nothing wrong.

@juliusknorr juliusknorr moved this from 🧭 Planning evaluation (don't pick) to 📄 To do (~10 entries) in 📝 Office team Jul 10, 2023
@juliusknorr
Copy link
Member

The patch seems reasonable. @kesselb Would you be up for opening a PR once confirmed?

@cpuch
Copy link

cpuch commented Jul 18, 2023

Hello, I'm facing the issue on Swift Object Storage. I'm trying to a upload a 530Mo ISO image from my web browser.

Let me know if you want more details

occ config:list system

{
    "system": {
        "debug": true,
        "dbtype": "mysql",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "",
        "dbtableprefix": "nc_",
        "mysql.utf8mb4": true,
        "objectstore": {
            "class": "OC\\Files\\ObjectStore\\Swift",
            "arguments": {
                "autocreate": true,
                "user": {
                    "name": "***REMOVED SENSITIVE VALUE***",
                    "password": "***REMOVED SENSITIVE VALUE***",
                    "domain": {
                        "name": "Default"
                    }
                },
                "scope": {
                    "project": {
                        "name": "7714518278425330",
                        "domain": {
                            "name": "Default"
                        }
                    }
                },
                "tenantName": "service",
                "serviceName": "swift",
                "region": "GRA",
                "url": "https:\/\/auth.cloud.ovh.net\/v3\/",
                "bucket": "nextcloud"
            }
        },
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "cloud.season-of-mist.com"
        ],
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "version": "27.0.0.8",
        "overwrite.cli.url": "https:\/\/cloud.season-of-mist.com",
        "htaccess.RewriteBase": "\/",
        "installed": true,
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "skeletondirectory": "",
        "mail_smtpmode": "smtp",
        "mail_smtpsecure": "tls",
        "mail_sendmailmode": "smtp",
        "mail_smtphost": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpport": "587",
        "mail_smtpauth": 1,
        "mail_smtpauthtype": "LOGIN",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpname": "***REMOVED SENSITIVE VALUE***",
        "mail_smtppassword": "***REMOVED SENSITIVE VALUE***",
        "mail_template_class": "OCA\\CustomEmailTemplate\\MyClass",
        "memcache.local": "\\OC\\Memcache\\Redis",
        "memcache.distributed": "\\OC\\Memcache\\Redis",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "port": 6379,
            "timeout": 0,
            "dbindex": 0,
            "password": "***REMOVED SENSITIVE VALUE***"
        },
        "filelocking.enabled": true,
        "enable_previews": false,
        "theme": "",
        "logtimezone": "Europe\/Paris",
        "simpleSignUpLink.shown": false,
        "maintenance": false,
        "loglevel": 0,
        "default_phone_region": "FR",
        "trashbin_retention_obligation": "60, 300",
        "session_keepalive": true,
        "check_for_working_htaccess": true,
        "app_install_overwrite": [
            "sharepoint"
        ],
        "updater.secret": "***REMOVED SENSITIVE VALUE***"
    }
}

Logging

[webdav] Error: OCP\Files\GenericFileException: Object store does not support multipart upload at <<closure>>

 0. <<closure>>
    OC\Files\ObjectStore\ObjectStoreStorage->startChunkedWrite()
 1. /var/www/html/cloud.season-of-mist.com/lib/private/Files/Storage/Wrapper/Wrapper.php line 524
    call_user_func_array()
 2. <<closure>>
    OC\Files\Storage\Wrapper\Wrapper->__call()
 3. /var/www/html/cloud.season-of-mist.com/lib/private/Files/Storage/Wrapper/Wrapper.php line 524
    call_user_func_array()
 4. <<closure>>
    OC\Files\Storage\Wrapper\Wrapper->__call()
 5. /var/www/html/cloud.season-of-mist.com/lib/private/Files/Storage/Wrapper/Wrapper.php line 524
    call_user_func_array()
 6. /var/www/html/cloud.season-of-mist.com/apps/dav/lib/Upload/ChunkingV2Plugin.php line 133
    OC\Files\Storage\Wrapper\Wrapper->__call()
 7. /var/www/html/cloud.season-of-mist.com/3rdparty/sabre/event/lib/WildcardEmitterTrait.php line 89
    OCA\DAV\Upload\ChunkingV2Plugin->afterMkcol()
 8. /var/www/html/cloud.season-of-mist.com/3rdparty/sabre/dav/lib/DAV/Server.php line 482
    Sabre\DAV\Server->emit()
 9. /var/www/html/cloud.season-of-mist.com/3rdparty/sabre/dav/lib/DAV/Server.php line 253
    Sabre\DAV\Server->invokeMethod()
10. /var/www/html/cloud.season-of-mist.com/3rdparty/sabre/dav/lib/DAV/Server.php line 321
    Sabre\DAV\Server->start()
11. /var/www/html/cloud.season-of-mist.com/apps/dav/lib/Server.php line 364
    Sabre\DAV\Server->exec()
12. /var/www/html/cloud.season-of-mist.com/apps/dav/appinfo/v2/remote.php line 35
    OCA\DAV\Server->exec()
13. /var/www/html/cloud.season-of-mist.com/remote.php line 172
    require_once("/var/www/html/c ... p")

MKCOL /remote.php/dav/uploads/admin/web-file-upload-e014249aae71df1d8caf084bb779c882-1689693092074
from 178.16.171.35 by admin at 2023-07-18T17:11:32+02:00

php-fpm log

- -  18/Jul/2023:17:11:31 +0200 "MKCOL /remote.php" 500
- -  18/Jul/2023:17:11:32 +0200 "DELETE /remote.php" 204
- -  18/Jul/2023:17:11:33 +0200 "GET /index.php" 200

@cpuch
Copy link

cpuch commented Aug 7, 2023

Any updates on this issue with Swift object store ?

@kesselb
Copy link
Contributor

kesselb commented Aug 7, 2023

Fixed in:

Nextcloud 27.0.2
Nextcloud 26.0.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of 26-feedback bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants