[Bug]: Behavior change for errors on file uploads

lennartdohmann commented 3 weeks ago

⚠️ This issue respects the following points: ⚠️

[X] This is a bug, not a question or a configuration/webserver/proxy issue.
[X] This issue is not already reported on Github OR Nextcloud Community Forum (I've searched it).
[X] Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
[X] I agree to follow Nextcloud's Code of Conduct.

Bug description

The PR linked here has caused a change in behavior for us (G DATA CyberDefense AG with our antivirus app) with undesirable consequences for the end customer.

Our app hooks into the upload process during file uploads to check a file for unwanted content during upload. If malware is found, the upload is blocked and the user receives a corresponding self defined error message.

This is exactly where we have been throwing this exception so far.

Before the PR, this led to us getting this error message when uploading via the HTTP API:

<?xml version=“1.0” encoding=“utf-8”?>
<d:error xmlns:d=“DAV:” xmlns:s=“http://sabredav.org/ns”>
  <s:exception>OCA\DAV\Connector\Sabre\Exception\UnsupportedMediaType</s:exception>
  <s:message>Virus EICAR-Test-File#462103 is detected in the file. Upload cannot be completed.</s:message>
</d:error>

When uploading via the UI, the customer was shown our defined error message.

After merging this PR, the error message via the API is now as follows for us and the end customer:

<?xml version=“1.0” encoding=“utf-8”?>
<d:error xmlns:d=“DAV:” xmlns:s=“http://sabredav.org/ns”>
        <s:exception>Internal Server Error</s:exception>
        <s:message>
                The server was unable to complete your request.         If this happens again, please send the technical details below to the server administrator.            More details can be found in the server log.                    </s:message>

        <s:technical-details>
                <s:remote-address>172.19.0.1</s:remote-address>
                <s:request-id>OpoP5Qqe8mqIJ7YQf4m0</s:request-id>

                </s:technical-details>
</d:error>

When debugging, we noticed that a new listener is called on 'exception' and this is called. This listener calls sendResponse what causes an exit() here. This exit causes that our custom error message is no longer displayed to the end customer.

Steps to reproduce

Install Nextcloud
Install G DATA Antivirus
Configure G DATA Antivirus app (you can create your own test credentials on the app settings page)
Upload an EICAR file to the Nextcloud instance (https://www.eicar.org/download-anti-malware-testfile/)
The expected behavior would be our custom error message that the upload got blocked due to a found virus like before

Expected behavior

The code after this try/catch block previously caused our own error message to come through, but due to the exit in the Emit() as described above, this code is no longer executed.

Nextcloud Server version

30

Operating system

Debian/Ubuntu

PHP engine version

PHP 8.1

Web server

Apache (supported)

Database engine version

SQlite

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?

[X] Default user-backend (database)
[ ] LDAP/ Active Directory
[ ] SSO - SAML
[ ] Other

Configuration report

{
    "system": {
        "skeletondirectory": "",
        "htaccess.RewriteBase": "\/",
        "memcache.local": "\\OC\\Memcache\\APCu",
        "apps_paths": [
            {
                "path": "\/var\/www\/html\/apps",
                "url": "\/apps",
                "writable": false
            },
            {
                "path": "\/var\/www\/html\/custom_apps",
                "url": "\/custom_apps",
                "writable": true
            }
        ],
        "upgrade.disable-web": true,
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": {
            "0": "localhost",
            "2": "192.168.5.80"
        },
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "dbtype": "sqlite3",
        "version": "30.0.0.14",
        "overwrite.cli.url": "http:\/\/localhost",
        "installed": true,
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "loglevel": 0,
        "mail_smtpmode": "smtp",
        "mail_smtphost": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpport": "25",
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_domain": "***REMOVED SENSITIVE VALUE***"
    }
}

List of activated Apps

Enabled:
  - activity: 3.0.0
  - bruteforcesettings: 3.0.0
  - circles: 30.0.0-dev
  - cloud_federation_api: 1.13.0
  - comments: 1.20.1
  - contactsinteraction: 1.11.0
  - dashboard: 7.10.0
  - dav: 1.31.1
  - federatedfilesharing: 1.20.0
  - federation: 1.20.0
  - files: 2.2.0
  - files_downloadlimit: 3.0.0
  - files_pdfviewer: 3.0.0
  - files_reminders: 1.3.0
  - files_sharing: 1.22.0
  - files_trashbin: 1.20.1
  - files_versions: 1.23.0
  - gdatavaas: 0.0.0
  - logreader: 3.0.0
  - lookup_server_connector: 1.18.0
  - nextcloud_announcements: 2.0.0
  - notifications: 3.0.0
  - oauth2: 1.18.1
  - password_policy: 2.0.0
  - photos: 3.0.2
  - privacy: 2.0.0
  - provisioning_api: 1.20.0
  - recommendations: 3.0.0
  - related_resources: 1.5.0
  - serverinfo: 2.0.0
  - settings: 1.13.0
  - sharebymail: 1.20.0
  - support: 2.0.0
  - survey_client: 2.0.0
  - systemtags: 1.20.0
  - text: 4.1.0
  - theming: 2.5.0
  - twofactor_backupcodes: 1.19.0
  - updatenotification: 1.20.0
  - user_status: 1.10.0
  - viewer: 3.0.0
  - webhook_listeners: 1.1.0-dev
  - workflowengine: 2.12.0
Disabled:
  - admin_audit: 1.20.0
  - encryption: 2.18.0
  - files_external: 1.22.0
  - firstrunwizard: 3.0.0 (installed 3.0.0)
  - suspicious_login: 8.0.0
  - twofactor_nextcloud_notification: 4.0.0
  - twofactor_totp: 12.0.0-dev
  - user_ldap: 1.21.0
  - weather_status: 1.10.0 (installed 1.10.0)

Nextcloud Signing status

No response

Nextcloud Logs

No response

Additional info

No response

lennartdohmann commented 3 weeks ago

@unglaublicherdude

lennartdohmann commented 3 weeks ago

This is the new error message we are not able to customize for the end user.

NewErrorMessage

kesselb commented 3 weeks ago

30.0.1 will remove the exit in sendResponse: https://github.com/nextcloud/server/commit/337df1f7600c32c91543dcfef9d476b4f49e85cc

lennartdohmann commented 3 weeks ago

One more question: Only for sever version 30? I think the ‘exit()’ has been already backported into all major versions in these PR's, or am I seeing this wrong? Or will this change also be backported from 30.0.1 onwards? Thanks!

kesselb commented 3 weeks ago

The backports for 25, 26 and 27 were corrected before merging. For 28 and 29 a follow-up exists to remove the exit again together with other fixes.

nextcloud / server