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

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

Open
5 of 8 tasks
lherbolt opened this issue Oct 9, 2023 · 4 comments
Open
5 of 8 tasks

Comments

@lherbolt
Copy link

lherbolt commented Oct 9, 2023

⚠️ This issue respects the following points: ⚠️

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:

Steps to reproduce

  1. install nextcloud
  2. create millions of files in data folder
  3. 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)
  • 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

@szaimen szaimen transferred this issue from nextcloud/server Oct 9, 2023
@joshtrichards joshtrichards changed the title [Bug]: Updater recurses into datadirectory even though it's later excluded Oct 9, 2023
@joshtrichards
Copy link
Member

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

@caco3
Copy link

caco3 commented Nov 3, 2023

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
Copy link
Author

lherbolt commented Nov 5, 2023

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

@joshtrichards joshtrichards changed the title Updater recurses into datadirectory even though it's later excluded Updater recurses into datadirectory even though it's later excluded in createBackup() Nov 16, 2023
@joshtrichards
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants