[Bug]: Bulk download (zip generation) from S3 primary storage downloads truncated files

[Ziyann]

⚠️ This issue respects the following points: ⚠️

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

Bug description

When downloading a large folder from S3 primary storage, the resulting ZIP file contains some corrupted (truncated) files - usually, only a few, with varying original sizes (I've seen it happening with both smaller and larger files). It's not always the same files, and the resulting ZIP file is valid, it's opening and extracting fine, even passing the CRC check, it's only the few files inside that are truncated (as if they were packed that way to begin with).

The number of files in the folder doesn't seem to matter; size seems to be more of an issue. It can happen even if I upload a single 2.6 GB file to a folder, and download that folder, but is more prevalent with bigger ones. Downloading the files directly one by one works, so they're uploaded correctly, but downloading the whole folder most often results in some truncated files.

My instance is located on a Google Compute Engine VM, with the primary storage being a Google Cloud Storage bucket in the same region. Because of this, the bucket access is very fast, and I can upload and download at 30-50 MB/s via Nextcloud (and the VM itself can access the bucket much faster). The VM has several gigabytes of free RAM and disk space when the issue happens, and there are no logs whatsoever (only a HTTP 200/OK log line for the download request from Nginx, with the truncated size).

I've verified that this only happens with S3 primary storage. Downloads from a local external storage from the same instance work fine (with the same folder contents, tested multiple times), so I think web server issues can be ruled out (unless this is some kind of receive timeout issue caused by S3? But it seems to be downloading just fine, before it stops, and again, the ZIP itself isn't corrupted, just some files inside). I don't have a separate proxy.

Steps to reproduce

1. Set up Nextcloud with an S3 primary storage
2. Create a local folder with a few files with various sizes (1 MB to a few GB, so the total folder size will be at least a few GB)
3. Upload the entire folder to Nextcloud
4. Download the entire folder as a ZIP (right click on it and select download)
5. Compare the downloaded folder with the original's contents
6. If there are differences, download the corrupt file directly from Nextcloud (in non-bulk mode)
7. Verify that the file isn't corrupt when downloaded alone

Expected behavior

Bulk downloads from S3 shouldn't produce a ZIP with truncated files inside, despite showing a successful download.

Installation method

Community Docker image

Nextcloud Server version

27

Operating system

Debian/Ubuntu

PHP engine version

PHP 8.2

Web server

Nginx

Database engine version

PostgreSQL

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

{
    "system": {
        "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
            }
        ],
        "memcache.distributed": "\\OC\\Memcache\\Redis",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "password": "***REMOVED SENSITIVE VALUE***",
            "port": 6379
        },
        "overwriteprotocol": "https",
        "trusted_proxies": "***REMOVED SENSITIVE VALUE***",
        "objectstore": {
            "class": "\\OC\\Files\\ObjectStore\\S3",
            "arguments": {
                "bucket": "XXXXXXXXX",
                "region": "europe-west1",
                "hostname": "storage.googleapis.com",
                "port": "443",
                "objectPrefix": "urn:oid:",
                "autocreate": false,
                "use_ssl": true,
                "use_path_style": false,
                "legacy_auth": false,
                "key": "***REMOVED SENSITIVE VALUE***",
                "secret": "***REMOVED SENSITIVE VALUE***"
            }
        },
        "filelocking.enabled": false,
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "cloud.XXXXXXXXX.com"
        ],
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "trashbin_retention_obligation": "auto, 30",
        "versions_retention_obligation": "auto, 30",
        "dbtype": "pgsql",
        "version": "27.1.4.1",
        "overwrite.cli.url": "https:\/\/cloud.XXXXXXXXX.com",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "",
        "dbtableprefix": "oc_",
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "installed": true,
        "mail_smtpmode": "smtp",
        "mail_sendmailmode": "smtp",
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "mail_smtphost": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpport": "465",
        "mail_smtpauth": 1,
        "mail_smtpname": "***REMOVED SENSITIVE VALUE***",
        "mail_smtppassword": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpsecure": "ssl",
        "updater.secret": "***REMOVED SENSITIVE VALUE***"
    }
}

List of activated Apps

Enabled:
  - activity: 2.19.0
  - cloud_federation_api: 1.10.0
  - contactsinteraction: 1.8.0
  - dav: 1.27.0
  - federatedfilesharing: 1.17.0
  - files: 1.22.0
  - files_external: 1.19.0
  - files_pdfviewer: 2.8.0
  - files_rightclick: 1.6.0
  - files_sharing: 1.19.0
  - files_trashbin: 1.17.0
  - files_versions: 1.20.0
  - logreader: 2.12.0
  - lookup_server_connector: 1.15.0
  - nextcloud_announcements: 1.16.0
  - notifications: 2.15.0
  - oauth2: 1.15.1
  - password_policy: 1.17.0
  - privacy: 1.11.0
  - provisioning_api: 1.17.0
  - recommendations: 1.6.0
  - related_resources: 1.2.0
  - serverinfo: 1.17.0
  - settings: 1.9.0
  - sharebymail: 1.17.0
  - survey_client: 1.15.0
  - text: 3.8.0
  - theming: 2.2.0
  - twofactor_backupcodes: 1.16.0
  - updatenotification: 1.17.0
  - user_oidc: 1.3.5
  - viewer: 2.1.0
  - workflowengine: 2.9.0
Disabled:
  - admin_audit: 1.17.0
  - bruteforcesettings: 2.7.0
  - circles: 27.0.1 (installed 27.0.1)
  - comments: 1.17.0 (installed 1.17.0)
  - dashboard: 7.7.0 (installed 7.7.0)
  - encryption: 2.15.0
  - federation: 1.17.0 (installed 1.17.0)
  - files_reminders: 1.0.0 (installed 1.0.0)
  - firstrunwizard: 2.16.0 (installed 2.16.0)
  - photos: 2.3.0 (installed 2.3.0)
  - support: 1.10.0 (installed 1.10.0)
  - suspicious_login: 5.0.0
  - systemtags: 1.17.0 (installed 1.17.0)
  - twofactor_totp: 9.0.0
  - user_ldap: 1.17.0
  - user_status: 1.7.0 (installed 1.7.0)
  - weather_status: 1.7.0 (installed 1.7.0)

Nextcloud Signing status

No errors have been found.

Nextcloud Logs

No response

Additional info

services:
  redis:
    image: redis:alpine
    restart: always

  nextcloud:
    image: nextcloud:fpm-alpine
    restart: always
    volumes:
      - ./php-config/zz-pm.conf:/usr/local/etc/php-fpm.d/zz-pm.conf:ro
      - ./php-config/zz-logging.ini:/usr/local/etc/php/conf.d/zz-logging.ini:ro
      - ./nextcloud:/var/www/html
    tmpfs:
      - /tmp
    environment:
      - REDIS_HOST=redis
      - OVERWRITEPROTOCOL=https
      - POSTGRES_HOST=XXXXXXXXX
      - POSTGRES_DB=XXXXXXXXX
      - POSTGRES_USER=XXXXXXXXX
      - POSTGRES_PASSWORD=XXXXXXXXX
      - OBJECTSTORE_S3_HOST=storage.googleapis.com
      - OBJECTSTORE_S3_BUCKET=XXXXXXXXX
      - OBJECTSTORE_S3_REGION=europe-west1
      - OBJECTSTORE_S3_KEY=XXXXXXXXX
      - OBJECTSTORE_S3_SECRET=XXXXXXXXX
      - OBJECTSTORE_S3_PORT=443
      - OBJECTSTORE_S3_SSL=true
    depends_on:
      - redis

  nextcloud-cron:
    image: nextcloud:fpm-alpine
    restart: always
    volumes:
      - ./nextcloud:/var/www/html
    entrypoint: /cron.sh
    depends_on:
      - redis

  nginx:
    image: nginx:alpine
    restart: always
    ports:
      - 80:80
      - 443:443
    volumes:
      - ./nginx/nginx.conf:/etc/nginx/nginx.conf:ro
      - ./certbot/www:/var/www/certbot:ro
      - ./certbot/conf:/etc/nginx/ssl:ro
      - ./nextcloud:/var/www/nextcloud:ro
    depends_on:
      - nextcloud

  certbot:
    image: certbot/certbot:latest
    volumes:
      - ./certbot/www:/var/www/certbot:rw
      - ./certbot/conf:/etc/letsencrypt:rw

worker_processes 4;

events {
    worker_connections  1024;
}

http {
        include /etc/nginx/mime.types;
        default_type application/octet-stream;

        sendfile on;
        tcp_nopush on;
        tcp_nodelay on;
        keepalive_timeout 65;
        types_hash_max_size 2048;

        upstream php-handler {
                server nextcloud:9000;
                #server unix:/run/php/php8.2-fpm.sock;
        }

# Set the `immutable` cache control options only for assets with a cache busting `v` argument
        map $arg_v $asset_immutable {
                "" "";
                default "immutable";
        }


        server {
                listen 80;
                listen [::]:80;
                server_name cloud.XXXXXXXXX.com;

                # Prevent nginx HTTP Server Detection
                server_tokens off;

                location /.well-known/acme-challenge/ {
                        root /var/www/certbot;
                }
                client_max_body_size 512M;

                location /put.php {
                        fastcgi_pass php-handler;
                }

                # Enforce HTTPS
                location / {
                        return 301 https://$server_name$request_uri;
                }
        }

        server {
                listen 443      ssl;
                listen [::]:443 ssl;
                #http2 on;
                server_name cloud.XXXXXXXXX.com;

                # Path to the root of your installation
                root /var/www/nextcloud;

                # Use Mozilla's guidelines for SSL/TLS settings
                # https://mozilla.github.io/server-side-tls/ssl-config-generator/
                ssl_certificate     /etc/nginx/ssl/live/cloud.XXXXXXXXX.com/fullchain.pem;
                ssl_certificate_key /etc/nginx/ssl/live/cloud.XXXXXXXXX.com/privkey.pem;

                # Prevent nginx HTTP Server Detection
                server_tokens off;

                # HSTS settings
                # WARNING: Only add the preload option once you read about
                # the consequences in https://hstspreload.org/. This option
                # will add the domain to a hardcoded list that is shipped
                # in all major browsers and getting removed from this list
                # could take several months.
                add_header Strict-Transport-Security "max-age=15768000; includeSubDomains; preload" always;

                # set max upload size and increase upload timeout:
                client_max_body_size 512M;
                client_body_timeout 300s;
                fastcgi_buffers 64 4K;

                # Enable gzip but do not remove ETag headers
                gzip on;
                gzip_vary on;
                gzip_comp_level 4;
                gzip_min_length 256;
                gzip_proxied expired no-cache no-store private no_last_modified no_etag auth;
                gzip_types application/atom+xml text/javascript application/javascript application/json application/ld+json application/manifest+json application/rss+xml application/vnd.geo+json application/vnd.ms-fontobject application/wasm application/x-font-ttf application/x-web-app-manifest+json application/xhtml+xml application/xml font/opentype image/bmp image/svg+xml image/x-icon text/cache-manifest text/css text/plain text/vcard text/vnd.rim.location.xloc text/vtt text/x-component text/x-cross-domain-policy;

                # Pagespeed is not supported by Nextcloud, so if your server is built
                # with the `ngx_pagespeed` module, uncomment this line to disable it.
                #pagespeed off;

                # The settings allows you to optimize the HTTP2 bandwidth.
                # See https://blog.cloudflare.com/delivering-http-2-upload-speed-improvements/
                # for tuning hints
                client_body_buffer_size 10M;

                # HTTP response headers borrowed from Nextcloud `.htaccess`
                add_header Referrer-Policy                   "no-referrer"       always;
                add_header X-Content-Type-Options            "nosniff"           always;
                add_header X-Frame-Options                   "SAMEORIGIN"        always;
                add_header X-Permitted-Cross-Domain-Policies "none"              always;
                add_header X-Robots-Tag                      "noindex, nofollow" always;
                add_header X-XSS-Protection                  "1; mode=block"     always;

                # Remove X-Powered-By, which is an information leak
                fastcgi_hide_header X-Powered-By;

                # Add .mjs as a file extension for javascript
                # Either include it in the default mime.types list
                # or include you can include that list explicitly and add the file extension
                # only for Nextcloud like below:
                include mime.types;
                types {
                        text/javascript mjs;
                }

                # Specify how to handle directories -- specifying `/index.php$request_uri`
                # here as the fallback means that Nginx always exhibits the desired behaviour
                # when a client requests a path that corresponds to a directory that exists
                # on the server. In particular, if that directory contains an index.php file,
                # that file is correctly served; if it doesn't, then the request is passed to
                # the front-end controller. This consistent behaviour means that we don't need
                # to specify custom rules for certain paths (e.g. images and other assets,
                                # `/updater`, `/ocs-provider`), and thus
                # `try_files $uri $uri/ /index.php$request_uri`
                # always provides the desired behaviour.
                index index.php index.html /index.php$request_uri;

                # Rule borrowed from `.htaccess` to handle Microsoft DAV clients
                location = / {
                        if ( $http_user_agent ~ ^DavClnt ) {
                                return 302 /remote.php/webdav/$is_args$args;
                        }

                }

                location = /robots.txt {
                        allow all;
                        log_not_found off;
                        access_log off;
                }

                # Make a regex exception for `/.well-known` so that clients can still
                # access it despite the existence of the regex rule
                # `location ~ /(\.|autotest|...)` which would otherwise handle requests
                # for `/.well-known`.
                location ^~ /.well-known {
                        # The rules in this block are an adaptation of the rules
                        # in `.htaccess` that concern `/.well-known`.

                        location = /.well-known/carddav { return 301 /remote.php/dav/; }
                        location = /.well-known/caldav  { return 301 /remote.php/dav/; }

                        location /.well-known/acme-challenge    { try_files $uri $uri/ =404; }
                        location /.well-known/pki-validation    { try_files $uri $uri/ =404; }

                        # Let Nextcloud's API for `/.well-known` URIs handle all other
                        # requests by passing them to the front-end controller.
                        return 301 /index.php$request_uri;
                }

                # Rules borrowed from `.htaccess` to hide certain paths from clients
                location ~ ^/(?:build|tests|config|lib|3rdparty|templates|data)(?:$|/)  { return 404; }
                location ~ ^/(?:\.|autotest|occ|issue|indie|db_|console)                { return 404; }

                # Ensure this block, which passes PHP files to the PHP process, is above the blocks
                # which handle static assets (as seen below). If this block is not declared first,
                # then Nginx will encounter an infinite rewriting loop when it prepends `/index.php`
                # to the URI, resulting in a HTTP 500 error response.
                location ~ \.php(?:$|/) {
                        # Required for legacy support
                        rewrite ^/(?!index|remote|public|cron|core\/ajax\/update|status|ocs\/v[12]|updater\/.+|ocs-provider\/.+|.+\/richdocumentscode\/proxy) /index.php$request_uri;

                        fastcgi_split_path_info ^(.+?\.php)(/.*)$;
                        set $path_info $fastcgi_path_info;

                        try_files $fastcgi_script_name =404;

                        include fastcgi_params;
                        fastcgi_param SCRIPT_FILENAME /var/www/html/$fastcgi_script_name;
                        fastcgi_param PATH_INFO $path_info;
                        fastcgi_param HTTPS on;

                        fastcgi_param modHeadersAvailable true;         # Avoid sending the security headers twice
                        fastcgi_param front_controller_active true;     # Enable pretty urls
                        fastcgi_pass php-handler;

                        fastcgi_intercept_errors on;
                        fastcgi_request_buffering off;

                        fastcgi_send_timeout 600s;
                        fastcgi_read_timeout 600s;

                        fastcgi_max_temp_file_size 0;
                }

                # Serve static files
                location ~ \.(?:css|js|mjs|svg|gif|png|jpg|ico|wasm|tflite|map|ogg|flac)$ {
                        try_files $uri /index.php$request_uri;
                        add_header Cache-Control "public, max-age=15778463, $asset_immutable";
                        access_log off;     # Optional: Don't log access to assets

                        location ~ \.wasm$ {
                                default_type application/wasm;
                        }
                }

                location ~ \.woff2?$ {
                        try_files $uri /index.php$request_uri;
                        expires 7d;         # Cache-Control policy borrowed from `.htaccess`
                        access_log off;     # Optional: Don't log access to assets
                }

                # Rule borrowed from `.htaccess`
                location /remote {
                        return 301 /remote.php$request_uri;
                }

                location / {
                        try_files $uri $uri/ /index.php$request_uri;
                }
        }
}

[Ziyann]

I found that I'm hitting this issue because of the way ZipStreamer is reading the data:
https://github.com/nextcloud/3rdparty/blob/v27.1.4/deepdiver/zipstreamer/src/ZipStreamer.php#L359

while (!feof($stream) && $data = fread($stream, self::STREAM_CHUNK_SIZE)) {
  ...
}

So, if fread returns an empty string, then even if feof is false, indicating that it's not the end of the stream yet, it will still abort reading the file (S3 stream in this case).

If I change it like so:

while (!feof($stream)) {
  $data = fread($stream, self::STREAM_CHUNK_SIZE);
  ...
}

Then my issue is fixed. The behavior was changed here: DeepDiver1975/PHPZipStreamer@22515e3. However, I believe it's incorrect, and the file size should be passed to the function (as it is known by that point) instead, to do the required checks instead of relying on fread return values, which, as we see, can silently produce truncated files.