Updater recurses into datadirectory even though it's later excluded in `createBackup()`

lherbolt commented 1 year 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 updater.phar recurse into data directory which is later excluded. This is prolonging the creation of the backup while running update.

I believe there it comes from this:

    /**
         * Gets the recursive directory iterator over the Nextcloud folder
         *
         * @param string $folder
         * @return \RecursiveIteratorIterator
         */
        private function getRecursiveDirectoryIterator($folder = null) {
                if ($folder === null) {
                        $folder = $this->baseDir . '/../';
                }
                return new \RecursiveIteratorIterator(
                        new \RecursiveDirectoryIterator($folder, \RecursiveDirectoryIterator::SKIP_DOTS),
                        \RecursiveIteratorIterator::CHILD_FIRST
                );
        }

Above creates iterator over all files in nextcloud folder, it;s called from the backup fucntion public function createBackup() bellow. Once the iterator is constructed the data .well-knonw and .rnd are excluded. It the data folder has significant amount files (in ma case is preview of images) it takes significant amount of time to create the iterator of files which are excluded later on

                /**
                 * @var string $path
                 * @var \SplFileInfo $fileInfo
                 */
                foreach ($this->getRecursiveDirectoryIterator($currentDir) as $path => $fileInfo) {
                        $fileName = explode($currentDir, $path)[1];
                        $folderStructure = explode('/', $fileName, -1);

                        // Exclude the exclusions
                        if (isset($folderStructure[0])) {
                                if (array_search($folderStructure[0], $excludedElements) !== false) {
                                        continue;
                                }

excludedElements:

                $excludedElements = [
                        '.rnd',
                        '.well-known',
                        'data',
                ];

I guess usage of something like would be better:

<?php

$Directory = new RecursiveDirectoryIterator('path/to/project/');
$Iterator = new RecursiveIteratorIterator($Directory);
$Regex = new RegexIterator($Iterator, '/^.+\.php$/i', RecursiveRegexIterator::GET_MATCH);

?>

I am not php coder so it's taken from:

https://www.php.net/manual/en/class.recursivedirectoryiterator.php

Steps to reproduce

install nextcloud
create millions of files in data folder
update

Expected behavior

the data folder and others are excluded before creating the iterator

Installation method

Community Manual installation with Archive

Nextcloud Server version

25

Operating system

Other

PHP engine version

PHP 8.0

Web server

Nginx

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)
[X] LDAP/ Active Directory
[ ] SSO - SAML
[ ] Other

Configuration report

[root@http-server-2 nextcloud]# sudo -u  nextcloud ./occ config:list system

{
    "system": {
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "data.herbolt.com"
        ],
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "dbtype": "mysql",
        "version": "26.0.5.1",
        "overwrite.cli.url": "https:\/\/data.herbolt.com",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "",
        "dbtableprefix": "oc_",
        "mysql.utf8mb4": true,
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "installed": true,
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpmode": "smtp",
        "mail_sendmailmode": "smtp",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpsecure": "ssl",
        "mail_smtpauthtype": "PLAIN",
        "mail_smtphost": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpport": "587",
        "app.mail.verify-tls-peer": false,
        "memcache.local": "\\OC\\Memcache\\APCu",
        "maintenance": false,
        "loglevel": 4,
        "theme": "",
        "default_phone_region": "CZ",
        "app_install_overwrite": [
            "ldap_contacts_backend"
        ],
        "ldapProviderFactory": "OCA\\User_LDAP\\LDAPProviderFactory",
        "remember_login_cookie_lifetime": 1296000,
        "session_lifetime": 604800
    }
}
[root@http-server-2 nextcloud]#

List of activated Apps

[root@http-server-2 nextcloud]# sudo -u  nextcloud ./occ app:list
Enabled:
  - analytics: 4.11.0
  - audioplayer: 3.4.0
  - bookmarks: 13.1.1
  - calendar: 4.5.2
  - cloud_federation_api: 1.9.0
  - contacts: 5.4.2
  - contactsinteraction: 1.7.0
  - dashboard: 7.6.0
  - dav: 1.25.0
  - federatedfilesharing: 1.16.0
  - files: 1.21.1
  - files_external: 1.18.0
  - files_pdfviewer: 2.7.0
  - files_rightclick: 1.5.0
  - files_sharing: 1.18.0
  - files_trashbin: 1.16.0
  - files_versions: 1.19.1
  - firstrunwizard: 2.15.0
  - forms: 3.3.1
  - logreader: 2.11.0
  - lookup_server_connector: 1.14.0
  - maps: 1.0.2
  - nextcloud_announcements: 1.15.0
  - notes: 4.8.1
  - notifications: 2.14.0
  - oauth2: 1.14.1
  - password_policy: 1.16.0
  - photos: 2.2.0
  - previewgenerator: 5.3.0
  - privacy: 1.10.0
  - provisioning_api: 1.16.0
  - recommendations: 1.5.0
  - related_resources: 1.1.0
  - serverinfo: 1.16.0
  - settings: 1.8.0
  - sharebymail: 1.16.0
  - spreed: 16.0.6
  - survey_client: 1.14.0
  - text: 3.7.2
  - theming: 2.1.1
  - twofactor_backupcodes: 1.15.0
  - updatenotification: 1.16.0
  - user_ldap: 1.16.0
  - user_status: 1.6.0
  - viewer: 1.10.0
  - weather_status: 1.6.0
  - workflow_script: 1.11.2
  - workflowengine: 2.8.0
Disabled:
  - activity: 2.18.0 (installed 2.17.0)
  - admin_audit: 1.16.0
  - bruteforcesettings: 2.6.0
  - circles: 26.0.0 (installed 25.0.0)
  - comments: 1.16.0 (installed 1.11.0)
  - encryption: 2.14.0
  - federation: 1.16.0 (installed 1.11.0)
  - ldap_contacts_backend: 1.6.0 (installed 1.6.0)
  - ldap_write_support: 1.8.0 (installed 1.8.0)
  - support: 1.9.0 (installed 1.4.0)
  - suspicious_login: 4.4.0
  - systemtags: 1.16.0 (installed 1.11.0)
  - twofactor_totp: 8.0.0

Nextcloud Signing status

No response

Nextcloud Logs

No response

Additional info

No response

joshtrichards commented 1 year ago

Interesting find, @lherbolt! That does appear sub-optimal (at least based on a cursory look on my part).

caco3 commented 1 year ago

There is already a function RecursiveDirectoryIteratorWithoutData(): https://github.com/caco3/updater/blob/27447ce4580e590134918a32ab9c3420dce8f42e/index.php#L44

Maybe it would be enough to just use that function?

lherbolt commented 12 months ago

I guess btw, the same is happening when deleting the data after update:

https://github.com/caco3/updater/blob/27447ce4580e590134918a32ab9c3420dce8f42e/index.php#L819

joshtrichards commented 11 months ago

Related: #397 (same solution will likely be used for both)

nextcloud / updater