From d58d6199d980ce234b04aa2e892061d622507b5b Mon Sep 17 00:00:00 2001 From: J0WI Date: Sat, 27 Mar 2021 20:57:27 +0100 Subject: [PATCH 01/27] Cleaner normalizePath regex Signed-off-by: J0WI --- lib/private/Files/Filesystem.php | 8 ++++---- tests/lib/Files/FilesystemTest.php | 27 ++++++++++++++++++++++----- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/lib/private/Files/Filesystem.php b/lib/private/Files/Filesystem.php index bf94be273f273..f7029640a00a4 100644 --- a/lib/private/Files/Filesystem.php +++ b/lib/private/Files/Filesystem.php @@ -831,10 +831,10 @@ public static function normalizePath($path, $stripTrailingSlash = true, $isAbsol $path = '/' . $path; $patterns = [ - '/\\\\/s', // no windows style slashes - '/\/\.(\/\.)?\//s', // remove '/./' - '/\/{2,}/s', // remove sequence of slashes - '/\/\.$/s', // remove trailing /. + '#\\\\#s', // no windows style '\\' slashes + '#/\.(/\.)*/#s', // remove '/./' + '#\//+#s', // remove sequence of slashes + '#/\.$#s', // remove trailing '/.' ]; do { diff --git a/tests/lib/Files/FilesystemTest.php b/tests/lib/Files/FilesystemTest.php index 5859bc2c7725b..8f34860d85a12 100644 --- a/tests/lib/Files/FilesystemTest.php +++ b/tests/lib/Files/FilesystemTest.php @@ -140,11 +140,26 @@ public function normalizePathData() { ['/foo/.bar', '/foo/.bar/'], ['/foo/.bar/', '/foo/.bar/', false], ['/foo/.bar/tee', '/foo/.bar/tee'], - - ['/foo/bar', '/.///././//./foo/.///././//./bar/./././.'], - ['/foo/bar/', '/.///././//./foo/.///././//./bar/./././.', false], - ['/foo/bar', '/.///././//./foo/.///././//./bar/././././'], - ['/foo/bar/', '/.///././//./foo/.///././//./bar/././././', false], + ['/foo/bar.', '/foo/bar./'], + ['/foo/bar./', '/foo/bar./', false], + ['/foo/bar./tee', '/foo/bar./tee'], + ['/foo/.bar.', '/foo/.bar./'], + ['/foo/.bar./', '/foo/.bar./', false], + ['/foo/.bar./tee', '/foo/.bar./tee'], + + ['/foo/bar', '/.////././//./foo/.///././//./bar/././/./.'], + ['/foo/bar/', '/.////././//./foo/.///././//./bar/./././.', false], + ['/foo/bar', '/.////././//./foo/.///././//./bar/././/././'], + ['/foo/bar/', '/.////././//./foo/.///././//./bar/././/././', false], + ['/foo/.bar', '/.////././//./foo/./././/./.bar/././/././'], + ['/foo/.bar/', '/.////././//./foo/./././/./.bar/././/././', false], + ['/foo/.bar/tee./', '/.////././//./foo/./././/./.bar/tee././/././', false], + ['/foo/bar.', '/.////././//./foo/./././/./bar./././/././'], + ['/foo/bar./', '/.////././//./foo/./././/./bar./././/././', false], + ['/foo/bar./tee./', '/.////././//./foo/./././/./bar./tee././/././', false], + ['/foo/.bar.', '/.////././//./foo/./././/./.bar./././/././'], + ['/foo/.bar./', '/.////././//./foo/./././/./.bar./././././', false], + ['/foo/.bar./tee./', '/.////././//./foo/./././/./.bar./tee././././', false], // Windows paths ['/', ''], @@ -186,7 +201,9 @@ public function normalizePathData() { // normalize does not resolve '..' (by design) ['/foo/..', '/foo/../'], + ['/foo/../bar', '/foo/../bar/.'], ['/foo/..', '\\foo\\..\\'], + ['/foo/../bar', '\\foo\\..\\bar'], ]; } From 0cdcda421073e865cb8bf8b80e4e60bc34469b3d Mon Sep 17 00:00:00 2001 From: Daniel Fuchs Date: Sat, 15 May 2021 10:26:24 +0200 Subject: [PATCH 02/27] fixed path to the img directory, from the accessability app. hack-y fix for #26578 Signed-off-by: Daniel Fuchs --- apps/accessibility/css/dark.scss | 3 +++ apps/accessibility/css/fontdyslexic.scss | 3 +++ apps/accessibility/css/highcontrast.scss | 3 +++ apps/accessibility/css/highcontrastdark.scss | 3 +++ 4 files changed, 12 insertions(+) diff --git a/apps/accessibility/css/dark.scss b/apps/accessibility/css/dark.scss index f0303e2692027..e5a53fad905c7 100644 --- a/apps/accessibility/css/dark.scss +++ b/apps/accessibility/css/dark.scss @@ -21,6 +21,9 @@ $color-box-shadow: transparentize(darken($color-main-background, 70%), 0.5); $color-border: lighten($color-main-background, 7%); $color-border-dark: lighten($color-main-background, 14%); +$image-logo: url('../../../core/img/logo/logo.svg?v=1') !default; +$image-logoheader: url('../../../core/img/logo/logo.svg?v=1') !default; + #app-navigation > ul > li > a:first-child, #app-navigation > ul > li > ul > li > a:first-child, #contactsmenu-menu a, diff --git a/apps/accessibility/css/fontdyslexic.scss b/apps/accessibility/css/fontdyslexic.scss index 9ee81c5f7ba56..275fb4af22ab0 100644 --- a/apps/accessibility/css/fontdyslexic.scss +++ b/apps/accessibility/css/fontdyslexic.scss @@ -13,3 +13,6 @@ } $font-face: OpenDyslexic, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen-Sans, Cantarell, Ubuntu, 'Helvetica Neue', Arial, 'Noto Color Emoji', sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; + +$image-logo: url('../../../core/img/logo/logo.svg?v=1') !default; +$image-logoheader: url('../../../core/img/logo/logo.svg?v=1') !default; diff --git a/apps/accessibility/css/highcontrast.scss b/apps/accessibility/css/highcontrast.scss index 707e34a2b4283..a6c44d30a0b79 100644 --- a/apps/accessibility/css/highcontrast.scss +++ b/apps/accessibility/css/highcontrast.scss @@ -20,6 +20,9 @@ $color-box-shadow: $color-main-text; $color-border: darken($color-main-background, 50%); $color-border-dark: darken($color-main-background, 50%); +$image-logo: url('../../../core/img/logo/logo.svg?v=1') !default; +$image-logoheader: url('../../../core/img/logo/logo.svg?v=1') !default; + [class^='icon-'], [class*=' icon-'], .action, #appmenu li a, diff --git a/apps/accessibility/css/highcontrastdark.scss b/apps/accessibility/css/highcontrastdark.scss index c5238dc2433c5..f302d112dab80 100644 --- a/apps/accessibility/css/highcontrastdark.scss +++ b/apps/accessibility/css/highcontrastdark.scss @@ -17,6 +17,9 @@ $color-box-shadow: $color-main-text; $color-border: lighten($color-main-background, 50%); $color-border-dark: lighten($color-main-background, 50%); +$image-logo: url('../../../core/img/logo/logo.svg?v=1') !default; +$image-logoheader: url('../../../core/img/logo/logo.svg?v=1') !default; + [class^='icon-'], [class*=' icon-'], .action, #appmenu li a, From 1d0a40a4a6a89971555a1e23a9832d69aad46d7e Mon Sep 17 00:00:00 2001 From: ZitronePlus Date: Wed, 30 Jun 2021 14:52:48 +0200 Subject: [PATCH 03/27] Fix for #26526 fix for sql query replaced double quotes with single quotes. Query should now also work for dbs with sql_mode including "ANSI" and "ANSI_QUOTES" --- apps/settings/lib/SetupChecks/SupportedDatabase.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/settings/lib/SetupChecks/SupportedDatabase.php b/apps/settings/lib/SetupChecks/SupportedDatabase.php index 1684567609b7e..089fb69bbc966 100644 --- a/apps/settings/lib/SetupChecks/SupportedDatabase.php +++ b/apps/settings/lib/SetupChecks/SupportedDatabase.php @@ -63,7 +63,7 @@ public function check() { case MySQL57Platform::class: # extends MySQLPlatform case MariaDb1027Platform::class: # extends MySQLPlatform case MySQLPlatform::class: - $result = $this->connection->prepare('SHOW VARIABLES LIKE "version";'); + $result = $this->connection->prepare("SHOW VARIABLES LIKE 'version';"); $result->execute(); $row = $result->fetch(); $version = strtolower($row['Value']); From 83b39d6111fe5c63fe3d488d15e565fd04e1ceaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 16 Apr 2021 18:21:11 +0200 Subject: [PATCH 04/27] Add missing scope to test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- tests/lib/Accounts/AccountPropertyTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/lib/Accounts/AccountPropertyTest.php b/tests/lib/Accounts/AccountPropertyTest.php index 50c3b8f84a439..c2ba96ef8a50a 100644 --- a/tests/lib/Accounts/AccountPropertyTest.php +++ b/tests/lib/Accounts/AccountPropertyTest.php @@ -78,6 +78,7 @@ public function scopesProvider() { // current values [IAccountManager::SCOPE_PRIVATE, IAccountManager::SCOPE_PRIVATE], [IAccountManager::SCOPE_LOCAL, IAccountManager::SCOPE_LOCAL], + [IAccountManager::SCOPE_FEDERATED, IAccountManager::SCOPE_FEDERATED], [IAccountManager::SCOPE_PUBLISHED, IAccountManager::SCOPE_PUBLISHED], // legacy values [IAccountManager::VISIBILITY_PRIVATE, IAccountManager::SCOPE_LOCAL], From 2c70e4689af774d010a1106bf17559071ee926e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 16 Apr 2021 18:24:40 +0200 Subject: [PATCH 05/27] Fix "Federated" scope not shown when the lookup server is disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the lookup server is disabled the address books can still be exchanged between trusted servers. Therefore the user should be able to set the "Federated" scope in that case. Signed-off-by: Daniel Calviño Sánchez --- apps/settings/js/federationsettingsview.js | 8 ++++---- apps/settings/js/settings/personalInfo.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/settings/js/federationsettingsview.js b/apps/settings/js/federationsettingsview.js index 602acab5c8deb..58e2583546c6e 100644 --- a/apps/settings/js/federationsettingsview.js +++ b/apps/settings/js/federationsettingsview.js @@ -15,7 +15,8 @@ * @constructs FederationScopeMenu * @memberof OC.Settings * @param {object} options - * @param {bool} [options.lookupServerUploadEnabled=false] whether uploading to the lookup server is enabled + * @param {bool} [options.showPublishedScope=false] whether show the + * "v2-published" scope or not */ var FederationSettingsView = OC.Backbone.View.extend({ _inputFields: undefined, @@ -31,7 +32,7 @@ } else { this._config = new OC.Settings.UserSettings(); } - this.showFederationScopes = !!options.showFederationScopes; + this.showPublishedScope = !!options.showPublishedScope; this._inputFields = [ 'displayname', @@ -85,8 +86,7 @@ excludedScopes.push('v2-private'); } - if (!self.showFederationScopes) { - excludedScopes.push('v2-federated'); + if (!self.showPublishedScope) { excludedScopes.push('v2-published'); } diff --git a/apps/settings/js/settings/personalInfo.js b/apps/settings/js/settings/personalInfo.js index 2e57537d424df..72ce39d5693b8 100644 --- a/apps/settings/js/settings/personalInfo.js +++ b/apps/settings/js/settings/personalInfo.js @@ -204,7 +204,7 @@ window.addEventListener('DOMContentLoaded', function () { var federationSettingsView = new OC.Settings.FederationSettingsView({ el: settingsEl, config: userSettings, - showFederationScopes: !!settingsEl.data('lookup-server-upload-enabled'), + showPublishedScope: !!settingsEl.data('lookup-server-upload-enabled'), }); userSettings.on("sync", function() { From 1e19b1cc994d38b19a29494cc1579fe3182a34bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 24 Jun 2021 01:46:44 +0200 Subject: [PATCH 06/27] Hide "federated" scope when Federation app is disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the Federation app is disabled it is not possible to synchronize the users from a different server. Signed-off-by: Daniel Calviño Sánchez --- apps/settings/js/federationsettingsview.js | 7 +++++++ apps/settings/js/settings/personalInfo.js | 1 + apps/settings/lib/Settings/Personal/PersonalInfo.php | 2 ++ .../settings/templates/settings/personal/personal.info.php | 3 ++- 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/apps/settings/js/federationsettingsview.js b/apps/settings/js/federationsettingsview.js index 58e2583546c6e..ca56992b1ed66 100644 --- a/apps/settings/js/federationsettingsview.js +++ b/apps/settings/js/federationsettingsview.js @@ -15,6 +15,8 @@ * @constructs FederationScopeMenu * @memberof OC.Settings * @param {object} options + * @param {bool} [options.showFederatedScope=false] whether show the + * "v2-federated" scope or not * @param {bool} [options.showPublishedScope=false] whether show the * "v2-published" scope or not */ @@ -32,6 +34,7 @@ } else { this._config = new OC.Settings.UserSettings(); } + this.showFederatedScope = !!options.showFederatedScope; this.showPublishedScope = !!options.showPublishedScope; this._inputFields = [ @@ -86,6 +89,10 @@ excludedScopes.push('v2-private'); } + if (!self.showFederatedScope) { + excludedScopes.push('v2-federated'); + } + if (!self.showPublishedScope) { excludedScopes.push('v2-published'); } diff --git a/apps/settings/js/settings/personalInfo.js b/apps/settings/js/settings/personalInfo.js index 72ce39d5693b8..dcac153c1f64f 100644 --- a/apps/settings/js/settings/personalInfo.js +++ b/apps/settings/js/settings/personalInfo.js @@ -204,6 +204,7 @@ window.addEventListener('DOMContentLoaded', function () { var federationSettingsView = new OC.Settings.FederationSettingsView({ el: settingsEl, config: userSettings, + showFederatedScope: !!settingsEl.data('federation-enabled'), showPublishedScope: !!settingsEl.data('lookup-server-upload-enabled'), }); diff --git a/apps/settings/lib/Settings/Personal/PersonalInfo.php b/apps/settings/lib/Settings/Personal/PersonalInfo.php index 387843c552258..fede5c4f5948b 100644 --- a/apps/settings/lib/Settings/Personal/PersonalInfo.php +++ b/apps/settings/lib/Settings/Personal/PersonalInfo.php @@ -92,6 +92,7 @@ public function __construct( } public function getForm(): TemplateResponse { + $federationEnabled = $this->appManager->isEnabledForUser('federation'); $federatedFileSharingEnabled = $this->appManager->isEnabledForUser('federatedfilesharing'); $lookupServerUploadEnabled = false; if ($federatedFileSharingEnabled) { @@ -124,6 +125,7 @@ public function getForm(): TemplateResponse { 'usage_relative' => round($storageInfo['relative']), 'quota' => $storageInfo['quota'], 'avatarChangeSupported' => $user->canChangeAvatar(), + 'federationEnabled' => $federationEnabled, 'lookupServerUploadEnabled' => $lookupServerUploadEnabled, 'avatarScope' => $account->getProperty(IAccountManager::PROPERTY_AVATAR)->getScope(), 'displayNameChangeSupported' => $user->canChangeDisplayName(), diff --git a/apps/settings/templates/settings/personal/personal.info.php b/apps/settings/templates/settings/personal/personal.info.php index 23ee1ca0367a7..c1a5735aa8fdf 100644 --- a/apps/settings/templates/settings/personal/personal.info.php +++ b/apps/settings/templates/settings/personal/personal.info.php @@ -35,7 +35,8 @@ ]); ?> -
+

t('Personal info')); ?>

From 193cf6cfde8a91e225947021e582997403074c78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 24 Jun 2021 01:47:07 +0200 Subject: [PATCH 07/27] Split capability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "federated" and "published" scopes are independent one from each other, so the capability that encompassed both needs to be split. Signed-off-by: Daniel Calviño Sánchez --- apps/provisioning_api/lib/Capabilities.php | 9 ++++--- .../tests/CapabilitiesTest.php | 24 ++++++++++++------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/apps/provisioning_api/lib/Capabilities.php b/apps/provisioning_api/lib/Capabilities.php index 59c462c5d90c1..835bbfe9b5c62 100644 --- a/apps/provisioning_api/lib/Capabilities.php +++ b/apps/provisioning_api/lib/Capabilities.php @@ -41,20 +41,23 @@ public function __construct(IAppManager $appManager) { * @return array Array containing the apps capabilities */ public function getCapabilities() { - $federationScopesEnabled = false; + $federatedScopeEnabled = $this->appManager->isEnabledForUser('federation'); + + $publishedScopeEnabled = false; $federatedFileSharingEnabled = $this->appManager->isEnabledForUser('federatedfilesharing'); if ($federatedFileSharingEnabled) { /** @var FederatedShareProvider $shareProvider */ $shareProvider = \OC::$server->query(FederatedShareProvider::class); - $federationScopesEnabled = $shareProvider->isLookupServerUploadEnabled(); + $publishedScopeEnabled = $shareProvider->isLookupServerUploadEnabled(); } return [ 'provisioning_api' => [ 'version' => $this->appManager->getAppVersion('provisioning_api'), 'AccountPropertyScopesVersion' => 2, - 'AccountPropertyScopesFederationEnabled' => $federationScopesEnabled, + 'AccountPropertyScopesFederatedEnabled' => $federatedScopeEnabled, + 'AccountPropertyScopesPublishedEnabled' => $publishedScopeEnabled, ] ]; } diff --git a/apps/provisioning_api/tests/CapabilitiesTest.php b/apps/provisioning_api/tests/CapabilitiesTest.php index 97f9ba562bcf3..8fc29b442eb4e 100644 --- a/apps/provisioning_api/tests/CapabilitiesTest.php +++ b/apps/provisioning_api/tests/CapabilitiesTest.php @@ -57,20 +57,25 @@ public function setUp(): void { public function getCapabilitiesProvider() { return [ - [false, false, false], - [true, false, false], - [true, true, true], + [true, false, false, true, false], + [true, true, false, true, false], + [true, true, true, true, true], + [false, false, false, false, false], + [false, true, false, false, false], + [false, true, true, false, true], ]; } /** * @dataProvider getCapabilitiesProvider */ - public function testGetCapabilities($federationAppEnabled, $lookupServerEnabled, $expectedFederationScopesEnabled) { - $this->appManager->expects($this->once()) - ->method('isEnabledForUser') - ->with('federatedfilesharing') - ->willReturn($federationAppEnabled); + public function testGetCapabilities($federationAppEnabled, $federatedFileSharingAppEnabled, $lookupServerEnabled, $expectedFederatedScopeEnabled, $expectedPublishedScopeEnabled) { + $this->appManager->expects($this->any()) + ->method('isEnabledForUser') + ->will($this->returnValueMap([ + ['federation', null, $federationAppEnabled], + ['federatedfilesharing', null, $federatedFileSharingAppEnabled], + ])); $federatedShareProvider = $this->createMock(FederatedShareProvider::class); $this->overwriteService(FederatedShareProvider::class, $federatedShareProvider); @@ -83,7 +88,8 @@ public function testGetCapabilities($federationAppEnabled, $lookupServerEnabled, 'provisioning_api' => [ 'version' => '1.12', 'AccountPropertyScopesVersion' => 2, - 'AccountPropertyScopesFederationEnabled' => $expectedFederationScopesEnabled, + 'AccountPropertyScopesFederatedEnabled' => $expectedFederatedScopeEnabled, + 'AccountPropertyScopesPublishedEnabled' => $expectedPublishedScopeEnabled, ], ]; $this->assertSame($expected, $this->capabilities->getCapabilities()); From 1b8ebf2cf1ecdcd977b62cae158b7d9804b2e43e Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 8 Jul 2021 16:52:59 +0200 Subject: [PATCH 08/27] Use cached user backend info for password login Signed-off-by: Joas Schilling --- lib/private/User/Manager.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 3e30861f2a4a6..dbbfc2b53a282 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -246,7 +246,13 @@ public function checkPasswordNoLogging($loginName, $password) { $loginName = str_replace("\0", '', $loginName); $password = str_replace("\0", '', $password); - foreach ($this->backends as $backend) { + $cachedBackend = $this->cache->get($loginName); + if ($cachedBackend !== null && isset($this->backends[$cachedBackend])) { + $backends = [$this->backends[$cachedBackend]]; + } else { + $backends = $this->backends; + } + foreach ($backends as $backend) { if ($backend->implementsActions(Backend::CHECK_PASSWORD)) { $uid = $backend->checkPassword($loginName, $password); if ($uid !== false) { @@ -257,10 +263,10 @@ public function checkPasswordNoLogging($loginName, $password) { // since http basic auth doesn't provide a standard way of handling non ascii password we allow password to be urlencoded // we only do this decoding after using the plain password fails to maintain compatibility with any password that happens - // to contains urlencoded patterns by "accident". + // to contain urlencoded patterns by "accident". $password = urldecode($password); - foreach ($this->backends as $backend) { + foreach ($backends as $backend) { if ($backend->implementsActions(Backend::CHECK_PASSWORD)) { $uid = $backend->checkPassword($loginName, $password); if ($uid !== false) { From d9e36fa5e61becd7da1b989c1a0226eaf7ec89f8 Mon Sep 17 00:00:00 2001 From: Daniel Josua Fuchs Date: Wed, 15 Sep 2021 11:43:02 +0200 Subject: [PATCH 09/27] removed the '!default' flags, as not necessary Signed-off-by: Daniel Josua Fuchs --- apps/accessibility/css/dark.scss | 4 ++-- apps/accessibility/css/fontdyslexic.scss | 4 ++-- apps/accessibility/css/highcontrast.scss | 4 ++-- apps/accessibility/css/highcontrastdark.scss | 3 --- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/apps/accessibility/css/dark.scss b/apps/accessibility/css/dark.scss index e5a53fad905c7..6cced2997bca6 100644 --- a/apps/accessibility/css/dark.scss +++ b/apps/accessibility/css/dark.scss @@ -21,8 +21,8 @@ $color-box-shadow: transparentize(darken($color-main-background, 70%), 0.5); $color-border: lighten($color-main-background, 7%); $color-border-dark: lighten($color-main-background, 14%); -$image-logo: url('../../../core/img/logo/logo.svg?v=1') !default; -$image-logoheader: url('../../../core/img/logo/logo.svg?v=1') !default; +$image-logo: url('../../../core/img/logo/logo.svg?v=1'); +$image-logoheader: url('../../../core/img/logo/logo.svg?v=1'); #app-navigation > ul > li > a:first-child, #app-navigation > ul > li > ul > li > a:first-child, diff --git a/apps/accessibility/css/fontdyslexic.scss b/apps/accessibility/css/fontdyslexic.scss index 275fb4af22ab0..647509bf2dc23 100644 --- a/apps/accessibility/css/fontdyslexic.scss +++ b/apps/accessibility/css/fontdyslexic.scss @@ -14,5 +14,5 @@ $font-face: OpenDyslexic, -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Oxygen-Sans, Cantarell, Ubuntu, 'Helvetica Neue', Arial, 'Noto Color Emoji', sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; -$image-logo: url('../../../core/img/logo/logo.svg?v=1') !default; -$image-logoheader: url('../../../core/img/logo/logo.svg?v=1') !default; +$image-logo: url('../../../core/img/logo/logo.svg?v=1'); +$image-logoheader: url('../../../core/img/logo/logo.svg?v=1'); diff --git a/apps/accessibility/css/highcontrast.scss b/apps/accessibility/css/highcontrast.scss index a6c44d30a0b79..a1b14abec33f1 100644 --- a/apps/accessibility/css/highcontrast.scss +++ b/apps/accessibility/css/highcontrast.scss @@ -20,8 +20,8 @@ $color-box-shadow: $color-main-text; $color-border: darken($color-main-background, 50%); $color-border-dark: darken($color-main-background, 50%); -$image-logo: url('../../../core/img/logo/logo.svg?v=1') !default; -$image-logoheader: url('../../../core/img/logo/logo.svg?v=1') !default; +$image-logo: url('../../../core/img/logo/logo.svg?v=1'); +$image-logoheader: url('../../../core/img/logo/logo.svg?v=1'); [class^='icon-'], [class*=' icon-'], .action, diff --git a/apps/accessibility/css/highcontrastdark.scss b/apps/accessibility/css/highcontrastdark.scss index f302d112dab80..c5238dc2433c5 100644 --- a/apps/accessibility/css/highcontrastdark.scss +++ b/apps/accessibility/css/highcontrastdark.scss @@ -17,9 +17,6 @@ $color-box-shadow: $color-main-text; $color-border: lighten($color-main-background, 50%); $color-border-dark: lighten($color-main-background, 50%); -$image-logo: url('../../../core/img/logo/logo.svg?v=1') !default; -$image-logoheader: url('../../../core/img/logo/logo.svg?v=1') !default; - [class^='icon-'], [class*=' icon-'], .action, #appmenu li a, From 35be21dbb7d3f32405c9813694ad61c2393114d8 Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Wed, 29 Sep 2021 17:32:13 +0200 Subject: [PATCH 10/27] Scheduling plugin not updating responding attendee status take two Signed-off-by: Anna Larch --- apps/dav/lib/Controller/InvitationResponseController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/dav/lib/Controller/InvitationResponseController.php b/apps/dav/lib/Controller/InvitationResponseController.php index f6923152dcea6..9dbe43199d896 100644 --- a/apps/dav/lib/Controller/InvitationResponseController.php +++ b/apps/dav/lib/Controller/InvitationResponseController.php @@ -198,7 +198,7 @@ private function buildITipResponse(array $row, string $partStat, int $guests = n $iTipMessage->method = 'REPLY'; $iTipMessage->sequence = $row['sequence']; $iTipMessage->sender = $row['attendee']; - $iTipMessage->recipient = $row['organizer']; + $iTipMessage->recipient = $row['attendee']; $message = << Date: Fri, 8 Oct 2021 16:34:43 +0200 Subject: [PATCH 11/27] Update attendence for external users For local users it's possible to select their calendar via the principal url and first update their own attendance status. External users have no calendar event hence the recipient is the organizer. Signed-off-by: Daniel Kesselberg --- .../InvitationResponseServer.php | 6 ++ .../InvitationResponseController.php | 7 +- .../InvitationResponseControllerTest.php | 97 ++++++++++++++----- 3 files changed, 85 insertions(+), 25 deletions(-) diff --git a/apps/dav/lib/CalDAV/InvitationResponse/InvitationResponseServer.php b/apps/dav/lib/CalDAV/InvitationResponse/InvitationResponseServer.php index d1ea6902af39a..7910eb614b71f 100644 --- a/apps/dav/lib/CalDAV/InvitationResponse/InvitationResponseServer.php +++ b/apps/dav/lib/CalDAV/InvitationResponse/InvitationResponseServer.php @@ -121,4 +121,10 @@ public function handleITipMessage(Message $iTipMessage) { $schedulingPlugin = $this->server->getPlugin('caldav-schedule'); $schedulingPlugin->scheduleLocalDelivery($iTipMessage); } + + public function isExternalAttendee(string $principalUri): bool { + /** @var \Sabre\DAVACL\Plugin $aclPlugin */ + $aclPlugin = $this->server->getPlugin('acl'); + return $aclPlugin->getPrincipalByUri($principalUri) === null; + } } diff --git a/apps/dav/lib/Controller/InvitationResponseController.php b/apps/dav/lib/Controller/InvitationResponseController.php index 9dbe43199d896..de22e3ba6a9e6 100644 --- a/apps/dav/lib/Controller/InvitationResponseController.php +++ b/apps/dav/lib/Controller/InvitationResponseController.php @@ -198,7 +198,12 @@ private function buildITipResponse(array $row, string $partStat, int $guests = n $iTipMessage->method = 'REPLY'; $iTipMessage->sequence = $row['sequence']; $iTipMessage->sender = $row['attendee']; - $iTipMessage->recipient = $row['attendee']; + + if ($this->responseServer->isExternalAttendee($row['attendee'])) { + $iTipMessage->recipient = $row['organizer']; + } else { + $iTipMessage->recipient = $row['attendee']; + } $message = <<. * */ + namespace OCA\DAV\Tests\Unit\DAV\Controller; use OCA\DAV\CalDAV\InvitationResponse\InvitationResponseServer; @@ -77,7 +78,17 @@ protected function setUp(): void { ); } - public function testAccept() { + public function attendeeProvider(): array { + return [ + 'local attendee' => [false], + 'external attendee' => [true] + ]; + } + + /** + * @dataProvider attendeeProvider + */ + public function testAccept(bool $isExternalAttendee): void { $this->buildQueryExpects('TOKEN123', [ 'id' => 0, 'uid' => 'this-is-the-events-uid', @@ -110,21 +121,26 @@ public function testAccept() { $called = false; $this->responseServer->expects($this->once()) ->method('handleITipMessage') - ->willReturnCallback(function (Message $iTipMessage) use (&$called, $expected) { + ->willReturnCallback(function (Message $iTipMessage) use (&$called, $isExternalAttendee, $expected) { $called = true; $this->assertEquals('this-is-the-events-uid', $iTipMessage->uid); $this->assertEquals('VEVENT', $iTipMessage->component); $this->assertEquals('REPLY', $iTipMessage->method); $this->assertEquals(null, $iTipMessage->sequence); $this->assertEquals('mailto:attendee@foo.bar', $iTipMessage->sender); - $this->assertEquals('mailto:organizer@foo.bar', $iTipMessage->recipient); + if ($isExternalAttendee) { + $this->assertEquals('mailto:organizer@foo.bar', $iTipMessage->recipient); + } else { + $this->assertEquals('mailto:attendee@foo.bar', $iTipMessage->recipient); + } $iTipMessage->scheduleStatus = '1.2;Message delivered locally'; $this->assertEquals($expected, $iTipMessage->message->serialize()); }); - - + $this->responseServer->expects($this->once()) + ->method('isExternalAttendee') + ->willReturn($isExternalAttendee); $response = $this->controller->accept('TOKEN123'); $this->assertInstanceOf(TemplateResponse::class, $response); @@ -133,7 +149,10 @@ public function testAccept() { $this->assertTrue($called); } - public function testAcceptSequence() { + /** + * @dataProvider attendeeProvider + */ + public function testAcceptSequence(bool $isExternalAttendee): void { $this->buildQueryExpects('TOKEN123', [ 'id' => 0, 'uid' => 'this-is-the-events-uid', @@ -166,21 +185,26 @@ public function testAcceptSequence() { $called = false; $this->responseServer->expects($this->once()) ->method('handleITipMessage') - ->willReturnCallback(function (Message $iTipMessage) use (&$called, $expected) { + ->willReturnCallback(function (Message $iTipMessage) use (&$called, $isExternalAttendee, $expected) { $called = true; $this->assertEquals('this-is-the-events-uid', $iTipMessage->uid); $this->assertEquals('VEVENT', $iTipMessage->component); $this->assertEquals('REPLY', $iTipMessage->method); $this->assertEquals(1337, $iTipMessage->sequence); $this->assertEquals('mailto:attendee@foo.bar', $iTipMessage->sender); - $this->assertEquals('mailto:organizer@foo.bar', $iTipMessage->recipient); + if ($isExternalAttendee) { + $this->assertEquals('mailto:organizer@foo.bar', $iTipMessage->recipient); + } else { + $this->assertEquals('mailto:attendee@foo.bar', $iTipMessage->recipient); + } $iTipMessage->scheduleStatus = '1.2;Message delivered locally'; $this->assertEquals($expected, $iTipMessage->message->serialize()); }); - - + $this->responseServer->expects($this->once()) + ->method('isExternalAttendee') + ->willReturn($isExternalAttendee); $response = $this->controller->accept('TOKEN123'); $this->assertInstanceOf(TemplateResponse::class, $response); @@ -189,7 +213,10 @@ public function testAcceptSequence() { $this->assertTrue($called); } - public function testAcceptRecurrenceId() { + /** + * @dataProvider attendeeProvider + */ + public function testAcceptRecurrenceId(bool $isExternalAttendee): void { $this->buildQueryExpects('TOKEN123', [ 'id' => 0, 'uid' => 'this-is-the-events-uid', @@ -223,21 +250,26 @@ public function testAcceptRecurrenceId() { $called = false; $this->responseServer->expects($this->once()) ->method('handleITipMessage') - ->willReturnCallback(function (Message $iTipMessage) use (&$called, $expected) { + ->willReturnCallback(function (Message $iTipMessage) use (&$called, $isExternalAttendee, $expected) { $called = true; $this->assertEquals('this-is-the-events-uid', $iTipMessage->uid); $this->assertEquals('VEVENT', $iTipMessage->component); $this->assertEquals('REPLY', $iTipMessage->method); $this->assertEquals(0, $iTipMessage->sequence); $this->assertEquals('mailto:attendee@foo.bar', $iTipMessage->sender); - $this->assertEquals('mailto:organizer@foo.bar', $iTipMessage->recipient); + if ($isExternalAttendee) { + $this->assertEquals('mailto:organizer@foo.bar', $iTipMessage->recipient); + } else { + $this->assertEquals('mailto:attendee@foo.bar', $iTipMessage->recipient); + } $iTipMessage->scheduleStatus = '1.2;Message delivered locally'; $this->assertEquals($expected, $iTipMessage->message->serialize()); }); - - + $this->responseServer->expects($this->once()) + ->method('isExternalAttendee') + ->willReturn($isExternalAttendee); $response = $this->controller->accept('TOKEN123'); $this->assertInstanceOf(TemplateResponse::class, $response); @@ -273,7 +305,10 @@ public function testAcceptExpiredToken() { $this->assertEquals([], $response->getParams()); } - public function testDecline() { + /** + * @dataProvider attendeeProvider + */ + public function testDecline(bool $isExternalAttendee): void { $this->buildQueryExpects('TOKEN123', [ 'id' => 0, 'uid' => 'this-is-the-events-uid', @@ -306,21 +341,26 @@ public function testDecline() { $called = false; $this->responseServer->expects($this->once()) ->method('handleITipMessage') - ->willReturnCallback(function (Message $iTipMessage) use (&$called, $expected) { + ->willReturnCallback(function (Message $iTipMessage) use (&$called, $isExternalAttendee, $expected) { $called = true; $this->assertEquals('this-is-the-events-uid', $iTipMessage->uid); $this->assertEquals('VEVENT', $iTipMessage->component); $this->assertEquals('REPLY', $iTipMessage->method); $this->assertEquals(null, $iTipMessage->sequence); $this->assertEquals('mailto:attendee@foo.bar', $iTipMessage->sender); - $this->assertEquals('mailto:organizer@foo.bar', $iTipMessage->recipient); + if ($isExternalAttendee) { + $this->assertEquals('mailto:organizer@foo.bar', $iTipMessage->recipient); + } else { + $this->assertEquals('mailto:attendee@foo.bar', $iTipMessage->recipient); + } $iTipMessage->scheduleStatus = '1.2;Message delivered locally'; $this->assertEquals($expected, $iTipMessage->message->serialize()); }); - - + $this->responseServer->expects($this->once()) + ->method('isExternalAttendee') + ->willReturn($isExternalAttendee); $response = $this->controller->decline('TOKEN123'); $this->assertInstanceOf(TemplateResponse::class, $response); @@ -336,7 +376,10 @@ public function testOptions() { $this->assertEquals(['token' => 'TOKEN123'], $response->getParams()); } - public function testProcessMoreOptionsResult() { + /** + * @dataProvider attendeeProvider + */ + public function testProcessMoreOptionsResult(bool $isExternalAttendee): void { $this->request->expects($this->at(0)) ->method('getParam') ->with('partStat') @@ -384,20 +427,26 @@ public function testProcessMoreOptionsResult() { $called = false; $this->responseServer->expects($this->once()) ->method('handleITipMessage') - ->willReturnCallback(function (Message $iTipMessage) use (&$called, $expected) { + ->willReturnCallback(function (Message $iTipMessage) use (&$called, $isExternalAttendee, $expected) { $called = true; $this->assertEquals('this-is-the-events-uid', $iTipMessage->uid); $this->assertEquals('VEVENT', $iTipMessage->component); $this->assertEquals('REPLY', $iTipMessage->method); $this->assertEquals(null, $iTipMessage->sequence); $this->assertEquals('mailto:attendee@foo.bar', $iTipMessage->sender); - $this->assertEquals('mailto:organizer@foo.bar', $iTipMessage->recipient); + if ($isExternalAttendee) { + $this->assertEquals('mailto:organizer@foo.bar', $iTipMessage->recipient); + } else { + $this->assertEquals('mailto:attendee@foo.bar', $iTipMessage->recipient); + } $iTipMessage->scheduleStatus = '1.2;Message delivered locally'; $this->assertEquals($expected, $iTipMessage->message->serialize()); }); - + $this->responseServer->expects($this->once()) + ->method('isExternalAttendee') + ->willReturn($isExternalAttendee); $response = $this->controller->processMoreOptionsResult('TOKEN123'); From 09ffac5e6dd5355c9aaf49c098942fa1e4fbed25 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 13 Oct 2021 19:42:31 +0200 Subject: [PATCH 12/27] s3 external storage listing rework Signed-off-by: Robin Appelman --- .github/workflows/s3-external.yml | 65 +++++ .../lib/Lib/Storage/AmazonS3.php | 243 +++++++++++------- .../Files/ObjectStore/S3ObjectTrait.php | 2 +- 3 files changed, 213 insertions(+), 97 deletions(-) create mode 100644 .github/workflows/s3-external.yml diff --git a/.github/workflows/s3-external.yml b/.github/workflows/s3-external.yml new file mode 100644 index 0000000000000..c51d070533d74 --- /dev/null +++ b/.github/workflows/s3-external.yml @@ -0,0 +1,65 @@ +name: S3 External storage +on: + push: + branches: + - master + - stable* + paths: + - 'apps/files_external/**' + pull_request: + paths: + - 'apps/files_external/**' + +env: + APP_NAME: files_external + +jobs: + s3-external-tests: + runs-on: ubuntu-latest + + strategy: + # do not stop on another job's failure + fail-fast: false + matrix: + php-versions: ['7.4', '8.0'] + + name: php${{ matrix.php-versions }}-${{ matrix.ftpd }} + + services: + minio: + image: minio/minio:RELEASE.2021-10-06T23-36-31Z + ports: + - "9000:9000" + + steps: + - name: Checkout server + uses: actions/checkout@v2 + with: + submodules: true + + - name: Set up php ${{ matrix.php-versions }} + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php-versions }} + tools: phpunit + extensions: mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, zip, gd + + - name: Set up Nextcloud + run: | + mkdir data + ./occ maintenance:install --verbose --database=sqlite --database-name=nextcloud --database-host=127.0.0.1 --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass password + ./occ app:enable --force ${{ env.APP_NAME }} + php -S localhost:8080 & + - name: PHPUnit + run: | + echo " true,'hostname' => 'localhost','key' => 'minioadmin','secret' => 'minioadmin', 'bucket' => 'bucket', 'port' => 9000, 'use_ssl' => false, 'autocreate' => true, 'use_path_style' => true];" > apps/${{ env.APP_NAME }}/tests/config.amazons3.php + phpunit --configuration tests/phpunit-autotest-external.xml apps/files_external/tests/Storage/Amazons3Test.php + s3-external-summary: + runs-on: ubuntu-latest + needs: s3-external-tests + + if: always() + + steps: + - name: Summary status + run: if ${{ needs.s3-external-tests.result != 'success' }}; then exit 1; fi diff --git a/apps/files_external/lib/Lib/Storage/AmazonS3.php b/apps/files_external/lib/Lib/Storage/AmazonS3.php index 1bdd11e39bd63..f85797d29f488 100644 --- a/apps/files_external/lib/Lib/Storage/AmazonS3.php +++ b/apps/files_external/lib/Lib/Storage/AmazonS3.php @@ -46,6 +46,7 @@ use Icewind\Streams\IteratorDirectory; use OC\Cache\CappedMemoryCache; use OC\Files\Cache\CacheEntry; +use OC\Files\Filesystem; use OC\Files\ObjectStore\S3ConnectionTrait; use OC\Files\ObjectStore\S3ObjectTrait; use OCP\Constants; @@ -71,6 +72,9 @@ public function needsPartFile() { /** @var IMimeTypeDetector */ private $mimeDetector; + /** @var bool|null */ + private $versioningEnabled = null; + public function __construct($parameters) { parent::__construct($parameters); $this->parseParams($parameters); @@ -120,12 +124,20 @@ private function invalidateCache($key) { unset($this->objectCache[$existingKey]); } } - unset($this->directoryCache[$key], $this->filesCache[$key]); + unset($this->filesCache[$key]); + $keys = array_keys($this->directoryCache->getData()); + $keyLength = strlen($key); + foreach ($keys as $existingKey) { + if (substr($existingKey, 0, $keyLength) === $key) { + unset($this->directoryCache[$existingKey]); + } + } + unset($this->directoryCache[$key]); } /** * @param $key - * @return Result|boolean + * @return array|false */ private function headObject($key) { if (!isset($this->objectCache[$key])) { @@ -133,7 +145,7 @@ private function headObject($key) { $this->objectCache[$key] = $this->getConnection()->headObject([ 'Bucket' => $this->bucket, 'Key' => $key - ]); + ])->toArray(); } catch (S3Exception $e) { if ($e->getStatusCode() >= 500) { throw $e; @@ -159,32 +171,44 @@ private function headObject($key) { * @throws \Exception */ private function doesDirectoryExist($path) { - if (!isset($this->directoryCache[$path])) { + if ($path === '.' || $path === '') { + return true; + } + + if (isset($this->directoryCache[$path])) { + return $this->directoryCache[$path]; + } + try { // Maybe this isn't an actual key, but a prefix. // Do a prefix listing of objects to determine. - try { - $result = $this->getConnection()->listObjects([ - 'Bucket' => $this->bucket, - 'Prefix' => rtrim($path, '/'), - 'MaxKeys' => 1, - 'Delimiter' => '/', - ]); + $result = $this->getConnection()->listObjectsV2([ + 'Bucket' => $this->bucket, + 'Prefix' => rtrim($path, '/'), + 'MaxKeys' => 1, + ]); - if ((isset($result['Contents'][0]['Key']) && $result['Contents'][0]['Key'] === rtrim($path, '/') . '/') - || isset($result['CommonPrefixes'])) { - $this->directoryCache[$path] = true; - } else { - $this->directoryCache[$path] = false; - } - } catch (S3Exception $e) { - if ($e->getStatusCode() === 403) { - $this->directoryCache[$path] = false; - } - throw $e; + if (isset($result['Contents'])) { + $this->directoryCache[$path] = true; + return true; } + + // empty directories have their own object + $object = $this->headObject($path); + + if ($object) { + $this->directoryCache[$path] = true; + return true; + } + } catch (S3Exception $e) { + if ($e->getStatusCode() === 403) { + $this->directoryCache[$path] = false; + } + throw $e; } - return $this->directoryCache[$path]; + + $this->directoryCache[$path] = false; + return false; } /** @@ -284,7 +308,9 @@ public function rmdir($path) { protected function clearBucket() { $this->clearCache(); try { - $this->getConnection()->clearBucket($this->bucket); + $this->getConnection()->clearBucket([ + "Bucket" => $this->bucket + ]); return true; // clearBucket() is not working with Ceph, so if it fails we try the slower approach } catch (\Exception $e) { @@ -318,7 +344,9 @@ private function batchDelete($path = null) { } // we reached the end when the list is no longer truncated } while ($objects['IsTruncated']); - $this->deleteObject($path); + if ($path !== '' && $path !== null) { + $this->deleteObject($path); + } } catch (S3Exception $e) { \OC::$server->getLogger()->logException($e, ['app' => 'files_external']); return false; @@ -327,54 +355,12 @@ private function batchDelete($path = null) { } public function opendir($path) { - $path = $this->normalizePath($path); - - if ($this->isRoot($path)) { - $path = ''; - } else { - $path .= '/'; - } - try { - $files = []; - $results = $this->getConnection()->getPaginator('ListObjects', [ - 'Bucket' => $this->bucket, - 'Delimiter' => '/', - 'Prefix' => $path, - ]); - - foreach ($results as $result) { - // sub folders - if (is_array($result['CommonPrefixes'])) { - foreach ($result['CommonPrefixes'] as $prefix) { - $directoryName = trim($prefix['Prefix'], '/'); - $files[] = substr($directoryName, strlen($path)); - $this->directoryCache[$directoryName] = true; - } - } - if (is_array($result['Contents'])) { - foreach ($result['Contents'] as $object) { - if (isset($object['Key']) && $object['Key'] === $path) { - // it's the directory itself, skip - continue; - } - $file = basename( - isset($object['Key']) ? $object['Key'] : $object['Prefix'] - ); - $files[] = $file; - - // store this information for later usage - $this->filesCache[$path . $file] = [ - 'ContentLength' => $object['Size'], - 'LastModified' => (string)$object['LastModified'], - ]; - } - } - } - - return IteratorDirectory::wrap($files); - } catch (S3Exception $e) { - \OC::$server->getLogger()->logException($e, ['app' => 'files_external']); + $content = iterator_to_array($this->getDirectoryContent($path)); + return IteratorDirectory::wrap(array_map(function (array $item) { + return $item['name']; + }, $content)); + } catch (S3Exception $e) { return false; } } @@ -382,33 +368,19 @@ public function opendir($path) { public function stat($path) { $path = $this->normalizePath($path); - try { - $stat = []; - if ($this->is_dir($path)) { - $cacheEntry = $this->getCache()->get($path); - if ($cacheEntry instanceof CacheEntry) { - $stat['size'] = $cacheEntry->getSize(); - $stat['mtime'] = $cacheEntry->getMTime(); - } else { - // Use dummy values - $stat['size'] = -1; // Pending - $stat['mtime'] = time(); - } - } else { - $stat['size'] = $this->getContentLength($path); - $stat['mtime'] = strtotime($this->getLastModified($path)); + if ($this->is_dir($path)) { + $stat = $this->getDirectoryMetaData($path); + } else { + $object = $this->headObject($path); + if ($object === false) { + return false; } - $stat['atime'] = time(); - - return $stat; - } catch (S3Exception $e) { - \OC::$server->getLogger()->logException($e, ['app' => 'files_external']); - return false; + $object["Key"] = $path; + $stat = $this->objectToMetaData($object); } - } + $stat['atime'] = time(); - public function hasUpdated($path, $time) { - return $this->getMountOption('filesystem_check_changes', 1) === 1 || parent::hasUpdated($path, $time); + return $stat; } /** @@ -711,4 +683,83 @@ public function writeBack($tmpFile, $path) { public static function checkDependencies() { return true; } + + public function getDirectoryContent($directory): \Traversable { + $path = $this->normalizePath($directory); + + if ($this->isRoot($path)) { + $path = ''; + } else { + $path .= '/'; + } + + $results = $this->getConnection()->getPaginator('ListObjectsV2', [ + 'Bucket' => $this->bucket, + 'Delimiter' => '/', + 'Prefix' => $path, + ]); + + foreach ($results as $result) { + // sub folders + if (is_array($result['CommonPrefixes'])) { + foreach ($result['CommonPrefixes'] as $prefix) { + $dir = $this->getDirectoryMetaData($prefix['Prefix']); + if ($dir) { + yield $dir; + } + } + } + if (is_array($result['Contents'])) { + foreach ($result['Contents'] as $object) { + $this->objectCache[$object['Key']] = $object; + if ($object['Key'] !== $path) { + yield $this->objectToMetaData($object); + } + } + } + } + } + + private function objectToMetaData(array $object): array { + return [ + 'name' => basename($object['Key']), + 'mimetype' => $this->mimeDetector->detectPath($object['Key']), + 'mtime' => strtotime($object['LastModified']), + 'storage_mtime' => strtotime($object['LastModified']), + 'etag' => $object['ETag'], + 'permissions' => Constants::PERMISSION_ALL - Constants::PERMISSION_CREATE, + 'size' => (int)($object['Size'] ?? $object['ContentLength']), + ]; + } + + private function getDirectoryMetaData(string $path): ?array { + $path = trim($path, '/'); + // when versioning is enabled, delete markers are returned as part of CommonPrefixes + // resulting in "ghost" folders, verify that each folder actually exists + if ($this->versioningEnabled() && !$this->doesDirectoryExist($path)) { + return null; + } + $cacheEntry = $this->getCache()->get($path); + if ($cacheEntry instanceof CacheEntry) { + return $cacheEntry->getData(); + } else { + return [ + 'name' => basename($path), + 'mimetype' => 'httpd/unix-directory', + 'mtime' => time(), + 'storage_mtime' => time(), + 'etag' => uniqid(), + 'permissions' => Constants::PERMISSION_ALL, + 'size' => -1, + ]; + } + } + + public function versioningEnabled(): bool { + if ($this->versioningEnabled === null) { + $result = $this->getConnection()->getBucketVersioning(['Bucket' => $this->getBucket()]); + $this->versioningEnabled = $result->get('Status') === 'Enabled'; + } + return $this->versioningEnabled; + } } diff --git a/lib/private/Files/ObjectStore/S3ObjectTrait.php b/lib/private/Files/ObjectStore/S3ObjectTrait.php index c88246094ed81..01da7a88dc8f1 100644 --- a/lib/private/Files/ObjectStore/S3ObjectTrait.php +++ b/lib/private/Files/ObjectStore/S3ObjectTrait.php @@ -65,7 +65,7 @@ public function readObject($urn) { } $opts = [ 'http' => [ - 'protocol_version' => 1.1, + 'protocol_version' => $request->getProtocolVersion(), 'header' => $headers, ], ]; From 5e3c8b3af2f2b25fcadc9aaf3ed10afa6e1d63c4 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 14 Oct 2021 18:36:55 +0200 Subject: [PATCH 13/27] doesDirectoryExist fixes Signed-off-by: Robin Appelman --- apps/files_external/lib/Lib/Storage/AmazonS3.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/files_external/lib/Lib/Storage/AmazonS3.php b/apps/files_external/lib/Lib/Storage/AmazonS3.php index f85797d29f488..1ad1cd699d3b6 100644 --- a/apps/files_external/lib/Lib/Storage/AmazonS3.php +++ b/apps/files_external/lib/Lib/Storage/AmazonS3.php @@ -46,7 +46,6 @@ use Icewind\Streams\IteratorDirectory; use OC\Cache\CappedMemoryCache; use OC\Files\Cache\CacheEntry; -use OC\Files\Filesystem; use OC\Files\ObjectStore\S3ConnectionTrait; use OC\Files\ObjectStore\S3ObjectTrait; use OCP\Constants; @@ -174,6 +173,7 @@ private function doesDirectoryExist($path) { if ($path === '.' || $path === '') { return true; } + $path = rtrim($path, '/') . '/'; if (isset($this->directoryCache[$path])) { return $this->directoryCache[$path]; @@ -183,7 +183,7 @@ private function doesDirectoryExist($path) { // Do a prefix listing of objects to determine. $result = $this->getConnection()->listObjectsV2([ 'Bucket' => $this->bucket, - 'Prefix' => rtrim($path, '/'), + 'Prefix' => $path, 'MaxKeys' => 1, ]); @@ -360,7 +360,7 @@ public function opendir($path) { return IteratorDirectory::wrap(array_map(function (array $item) { return $item['name']; }, $content)); - } catch (S3Exception $e) { + } catch (S3Exception $e) { return false; } } @@ -435,7 +435,7 @@ public function is_dir($path) { } try { - return $this->isRoot($path) || $this->doesDirectoryExist($path); + return $this->doesDirectoryExist($path); } catch (S3Exception $e) { \OC::$server->getLogger()->logException($e, ['app' => 'files_external']); return false; From 294af4275c3eaf8a40564edc4706ed7fab3f18c2 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 14 Oct 2021 19:16:58 +0200 Subject: [PATCH 14/27] more reliable directory copy Signed-off-by: Robin Appelman --- .../lib/Lib/Storage/AmazonS3.php | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/apps/files_external/lib/Lib/Storage/AmazonS3.php b/apps/files_external/lib/Lib/Storage/AmazonS3.php index 1ad1cd699d3b6..6fbe913ec7ce7 100644 --- a/apps/files_external/lib/Lib/Storage/AmazonS3.php +++ b/apps/files_external/lib/Lib/Storage/AmazonS3.php @@ -49,6 +49,7 @@ use OC\Files\ObjectStore\S3ConnectionTrait; use OC\Files\ObjectStore\S3ObjectTrait; use OCP\Constants; +use OCP\Files\FileInfo; use OCP\Files\IMimeTypeDetector; class AmazonS3 extends \OC\Files\Storage\Common { @@ -272,7 +273,7 @@ public function mkdir($path) { 'Bucket' => $this->bucket, 'Key' => $path . '/', 'Body' => '', - 'ContentType' => 'httpd/unix-directory' + 'ContentType' => FileInfo::MIMETYPE_FOLDER ]); $this->testTimeout(); } catch (S3Exception $e) { @@ -575,11 +576,11 @@ public function touch($path, $mtime = null) { return true; } - public function copy($path1, $path2) { + public function copy($path1, $path2, $isFile = null) { $path1 = $this->normalizePath($path1); $path2 = $this->normalizePath($path2); - if ($this->is_file($path1)) { + if ($isFile === true || $this->is_file($path1)) { try { $this->getConnection()->copyObject([ 'Bucket' => $this->bucket, @@ -595,28 +596,17 @@ public function copy($path1, $path2) { $this->remove($path2); try { - $this->getConnection()->copyObject([ - 'Bucket' => $this->bucket, - 'Key' => $path2 . '/', - 'CopySource' => S3Client::encodeKey($this->bucket . '/' . $path1 . '/') - ]); + $this->mkdir($path2); $this->testTimeout(); } catch (S3Exception $e) { \OC::$server->getLogger()->logException($e, ['app' => 'files_external']); return false; } - $dh = $this->opendir($path1); - if (is_resource($dh)) { - while (($file = readdir($dh)) !== false) { - if (\OC\Files\Filesystem::isIgnoredDir($file)) { - continue; - } - - $source = $path1 . '/' . $file; - $target = $path2 . '/' . $file; - $this->copy($source, $target); - } + foreach ($this->getDirectoryContent($path1) as $item) { + $source = $path1 . '/' . $item['name']; + $target = $path2 . '/' . $item['name']; + $this->copy($source, $target, $item['mimetype'] !== FileInfo::MIMETYPE_FOLDER); } } @@ -745,7 +735,7 @@ private function getDirectoryMetaData(string $path): ?array { } else { return [ 'name' => basename($path), - 'mimetype' => 'httpd/unix-directory', + 'mimetype' => FileInfo::MIMETYPE_FOLDER, 'mtime' => time(), 'storage_mtime' => time(), 'etag' => uniqid(), From 294b218895f446fe53bf9882c964d7cb49baf41d Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 14 Oct 2021 17:28:32 +0200 Subject: [PATCH 15/27] ci Signed-off-by: Robin Appelman --- .github/workflows/s3-external.yml | 71 +++++++++++++++++-- .../tests/Storage/Amazons3Test.php | 6 ++ .../tests/Storage/VersionedAmazonS3Test.php | 43 +++++++++++ tests/lib/Files/Storage/Storage.php | 3 + 4 files changed, 117 insertions(+), 6 deletions(-) create mode 100644 apps/files_external/tests/Storage/VersionedAmazonS3Test.php diff --git a/.github/workflows/s3-external.yml b/.github/workflows/s3-external.yml index c51d070533d74..497de81ae2f79 100644 --- a/.github/workflows/s3-external.yml +++ b/.github/workflows/s3-external.yml @@ -14,7 +14,7 @@ env: APP_NAME: files_external jobs: - s3-external-tests: + s3-external-tests-minio: runs-on: ubuntu-latest strategy: @@ -23,11 +23,14 @@ jobs: matrix: php-versions: ['7.4', '8.0'] - name: php${{ matrix.php-versions }}-${{ matrix.ftpd }} + name: php${{ matrix.php-versions }}-minio services: minio: - image: minio/minio:RELEASE.2021-10-06T23-36-31Z + env: + MINIO_ACCESS_KEY: minio + MINIO_SECRET_KEY: minio123 + image: bitnami/minio:2021.10.6 ports: - "9000:9000" @@ -52,14 +55,70 @@ jobs: php -S localhost:8080 & - name: PHPUnit run: | - echo " true,'hostname' => 'localhost','key' => 'minioadmin','secret' => 'minioadmin', 'bucket' => 'bucket', 'port' => 9000, 'use_ssl' => false, 'autocreate' => true, 'use_path_style' => true];" > apps/${{ env.APP_NAME }}/tests/config.amazons3.php + echo " true,'hostname' => 'localhost','key' => 'minio','secret' => 'minio123', 'bucket' => 'bucket', 'port' => 9000, 'use_ssl' => false, 'autocreate' => true, 'use_path_style' => true];" > apps/${{ env.APP_NAME }}/tests/config.amazons3.php phpunit --configuration tests/phpunit-autotest-external.xml apps/files_external/tests/Storage/Amazons3Test.php + phpunit --configuration tests/phpunit-autotest-external.xml apps/files_external/tests/Storage/VersionedAmazonS3Test.php + - name: S3 logs + if: always() + run: | + docker ps -a + docker logs $(docker ps -aq) + s3-external-tests-localstack: + runs-on: ubuntu-latest + + strategy: + # do not stop on another job's failure + fail-fast: false + matrix: + php-versions: ['7.4', '8.0'] + + name: php${{ matrix.php-versions }}-localstack + + services: + minio: + env: + SERVICES: s3 + DEBUG: 1 + image: localstack/localstack:0.12.7 + ports: + - "4566:4566" + + steps: + - name: Checkout server + uses: actions/checkout@v2 + with: + submodules: true + + - name: Set up php ${{ matrix.php-versions }} + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php-versions }} + tools: phpunit + extensions: mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, zip, gd + + - name: Set up Nextcloud + run: | + mkdir data + ./occ maintenance:install --verbose --database=sqlite --database-name=nextcloud --database-host=127.0.0.1 --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass password + ./occ app:enable --force ${{ env.APP_NAME }} + php -S localhost:8080 & + - name: PHPUnit + run: | + echo " true,'hostname' => 'localhost','key' => 'ignored','secret' => 'ignored', 'bucket' => 'bucket', 'port' => 4566, 'use_ssl' => false, 'autocreate' => true, 'use_path_style' => true];" > apps/${{ env.APP_NAME }}/tests/config.amazons3.php + phpunit --configuration tests/phpunit-autotest-external.xml apps/files_external/tests/Storage/Amazons3Test.php + phpunit --configuration tests/phpunit-autotest-external.xml apps/files_external/tests/Storage/VersionedAmazonS3Test.php + - name: S3 logs + if: always() + run: | + docker ps -a + docker logs $(docker ps -aq) + s3-external-summary: runs-on: ubuntu-latest - needs: s3-external-tests + needs: [s3-external-tests-minio, s3-external-tests-localstack] if: always() steps: - name: Summary status - run: if ${{ needs.s3-external-tests.result != 'success' }}; then exit 1; fi + run: if ${{ needs.s3-external-tests-minio.result != 'success' }} || ${{ needs.s3-external-tests-localstack.result != 'success' }}; then exit 1; fi diff --git a/apps/files_external/tests/Storage/Amazons3Test.php b/apps/files_external/tests/Storage/Amazons3Test.php index c013d304cceb0..d231539fb54a6 100644 --- a/apps/files_external/tests/Storage/Amazons3Test.php +++ b/apps/files_external/tests/Storage/Amazons3Test.php @@ -38,6 +38,8 @@ */ class Amazons3Test extends \Test\Files\Storage\Storage { private $config; + /** @var AmazonS3 */ + protected $instance; protected function setUp(): void { parent::setUp(); @@ -60,4 +62,8 @@ protected function tearDown(): void { public function testStat() { $this->markTestSkipped('S3 doesn\'t update the parents folder mtime'); } + + public function testHashInFileName() { + $this->markTestSkipped('Localstack has a bug with hashes in filename'); + } } diff --git a/apps/files_external/tests/Storage/VersionedAmazonS3Test.php b/apps/files_external/tests/Storage/VersionedAmazonS3Test.php new file mode 100644 index 0000000000000..a16a9944d578b --- /dev/null +++ b/apps/files_external/tests/Storage/VersionedAmazonS3Test.php @@ -0,0 +1,43 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Files_External\Tests\Storage; + +/** + * @group DB + */ +class VersionedAmazonS3Test extends Amazons3Test { + protected function setUp(): void { + parent::setUp(); + try { + $this->instance->getConnection()->putBucketVersioning([ + 'Bucket' => $this->instance->getBucket(), + 'VersioningConfiguration' => [ + 'Status' => 'Enabled', + ], + ]); + } catch (\Exception $e) { + $this->markTestSkipped("s3 backend doesn't seem to support versioning"); + } + } +} diff --git a/tests/lib/Files/Storage/Storage.php b/tests/lib/Files/Storage/Storage.php index 9fae1a8484aaa..c4248b7e0da8c 100644 --- a/tests/lib/Files/Storage/Storage.php +++ b/tests/lib/Files/Storage/Storage.php @@ -498,6 +498,9 @@ public function testRenameDirectory() { $this->assertTrue($this->instance->file_exists('target/subfolder')); $this->assertTrue($this->instance->file_exists('target/subfolder/test.txt')); + $contents = iterator_to_array($this->instance->getDirectoryContent('')); + $this->assertCount(1, $contents); + $this->assertEquals('foo', $this->instance->file_get_contents('target/test1.txt')); $this->assertEquals('qwerty', $this->instance->file_get_contents('target/test2.txt')); $this->assertEquals('bar', $this->instance->file_get_contents('target/subfolder/test.txt')); From d3bd0b5a1bfd8125866a9a86cbb05d3d2c6e14bb Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 15 Oct 2021 14:54:09 +0200 Subject: [PATCH 16/27] optimize filetype for s3 directories a bit Signed-off-by: Robin Appelman --- apps/files_external/lib/Lib/Storage/AmazonS3.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/files_external/lib/Lib/Storage/AmazonS3.php b/apps/files_external/lib/Lib/Storage/AmazonS3.php index 6fbe913ec7ce7..c4c82c6a8795e 100644 --- a/apps/files_external/lib/Lib/Storage/AmazonS3.php +++ b/apps/files_external/lib/Lib/Storage/AmazonS3.php @@ -451,6 +451,9 @@ public function filetype($path) { } try { + if (isset($this->directoryCache[$path])) { + return 'dir'; + } if (isset($this->filesCache[$path]) || $this->headObject($path)) { return 'file'; } From 34637697e1310728d138aa4386f890dfc4ffd288 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 15 Oct 2021 15:15:56 +0200 Subject: [PATCH 17/27] remove old migration method Signed-off-by: Robin Appelman --- .../lib/Lib/Storage/AmazonS3.php | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/apps/files_external/lib/Lib/Storage/AmazonS3.php b/apps/files_external/lib/Lib/Storage/AmazonS3.php index c4c82c6a8795e..9c7d6a554b3b5 100644 --- a/apps/files_external/lib/Lib/Storage/AmazonS3.php +++ b/apps/files_external/lib/Lib/Storage/AmazonS3.php @@ -212,37 +212,6 @@ private function doesDirectoryExist($path) { return false; } - /** - * Updates old storage ids (v0.2.1 and older) that are based on key and secret to new ones based on the bucket name. - * TODO Do this in a repair step. requires iterating over all users and loading the mount.json from their home - * - * @param array $params - */ - public function updateLegacyId(array $params) { - $oldId = 'amazon::' . $params['key'] . md5($params['secret']); - - // find by old id or bucket - $stmt = \OC::$server->getDatabaseConnection()->prepare( - 'SELECT `numeric_id`, `id` FROM `*PREFIX*storages` WHERE `id` IN (?, ?)' - ); - $stmt->execute([$oldId, $this->id]); - while ($row = $stmt->fetch()) { - $storages[$row['id']] = $row['numeric_id']; - } - - if (isset($storages[$this->id]) && isset($storages[$oldId])) { - // if both ids exist, delete the old storage and corresponding filecache entries - \OC\Files\Cache\Storage::remove($oldId); - } elseif (isset($storages[$oldId])) { - // if only the old id exists do an update - $stmt = \OC::$server->getDatabaseConnection()->prepare( - 'UPDATE `*PREFIX*storages` SET `id` = ? WHERE `id` = ?' - ); - $stmt->execute([$this->id, $oldId]); - } - // only the bucket based id may exist, do nothing - } - /** * Remove a file or folder * From 4bd08af2adf6742751aba089d28f6cb486e69e13 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 15 Oct 2021 16:02:57 +0200 Subject: [PATCH 18/27] more reliable hasUpdated for s3 Signed-off-by: Robin Appelman --- apps/files_external/lib/Lib/Storage/AmazonS3.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/apps/files_external/lib/Lib/Storage/AmazonS3.php b/apps/files_external/lib/Lib/Storage/AmazonS3.php index 9c7d6a554b3b5..527a87aeb9a2b 100644 --- a/apps/files_external/lib/Lib/Storage/AmazonS3.php +++ b/apps/files_external/lib/Lib/Storage/AmazonS3.php @@ -724,4 +724,17 @@ public function versioningEnabled(): bool { } return $this->versioningEnabled; } + + public function hasUpdated($path, $time) { + // for files we can get the proper mtime + if ($path !== '' && $object = $this->headObject($path)) { + $stat = $this->objectToMetaData($object); + return $stat['mtime'] > $time; + } else { + // for directories, the only real option we have is to do a prefix listing and iterate over all objects + // however, since this is just as expensive as just re-scanning the directory, we can simply return true + // and have the scanner figure out if anything has actually changed + return true; + } + } } From 55346b5d6cb24d11157b7a2e73a91fda85bb9af5 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 15 Oct 2021 16:03:18 +0200 Subject: [PATCH 19/27] more reliable return value for Watcher::checkUpdate Signed-off-by: Robin Appelman --- lib/private/Files/Cache/Watcher.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/private/Files/Cache/Watcher.php b/lib/private/Files/Cache/Watcher.php index 1c1ce6d777bb2..acc76f263dc57 100644 --- a/lib/private/Files/Cache/Watcher.php +++ b/lib/private/Files/Cache/Watcher.php @@ -88,7 +88,14 @@ public function checkUpdate($path, $cachedEntry = null) { } if ($cachedEntry === false || $this->needsUpdate($path, $cachedEntry)) { $this->update($path, $cachedEntry); - return true; + + if ($cachedEntry === false) { + return true; + } else { + // storage backends can sometimes return false positives, only return true if the scanner actually found a change + $newEntry = $this->cache->get($path); + return $newEntry->getStorageMTime() > $cachedEntry->getStorageMTime(); + } } else { return false; } From 247e12da966aeceb833e8c60b833e089bee89f8a Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 15 Oct 2021 16:23:39 +0200 Subject: [PATCH 20/27] always set Key field in `headObject` Signed-off-by: Robin Appelman --- apps/files_external/lib/Lib/Storage/AmazonS3.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/files_external/lib/Lib/Storage/AmazonS3.php b/apps/files_external/lib/Lib/Storage/AmazonS3.php index 527a87aeb9a2b..aa0afac15439f 100644 --- a/apps/files_external/lib/Lib/Storage/AmazonS3.php +++ b/apps/files_external/lib/Lib/Storage/AmazonS3.php @@ -154,6 +154,9 @@ private function headObject($key) { } } + if (is_array($this->objectCache[$key]) && !isset($this->objectCache[$key]["Key"])) { + $this->objectCache[$key]["Key"] = $key; + } return $this->objectCache[$key]; } @@ -345,7 +348,6 @@ public function stat($path) { if ($object === false) { return false; } - $object["Key"] = $path; $stat = $this->objectToMetaData($object); } $stat['atime'] = time(); From 9068fd4a21f866ab23699f4840aafd5024e76198 Mon Sep 17 00:00:00 2001 From: JanBartels Date: Mon, 18 Oct 2021 21:41:01 +0200 Subject: [PATCH 21/27] Patch for master-branch --- lib/private/Share20/Manager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index da8de81208eba..ccc2d454a94de 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1667,7 +1667,7 @@ public function getAccessList(\OCP\Files\Node $path, $recursive = true, $current if ($path->getId() !== $userFolder->getId() && !$userFolder->isSubNode($path)) { $nodes = $userFolder->getById($path->getId()); $path = array_shift($nodes); - if ($path->getOwner() === null) { + if ($path === null || $path->getOwner() === null) { return []; } $owner = $path->getOwner()->getUID(); From eb6e6e3a85a5d65215736998c916c5fe30d8271b Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 19 Oct 2021 14:58:40 +0200 Subject: [PATCH 22/27] minor directory detect improvements Signed-off-by: Robin Appelman --- apps/files_external/lib/Lib/Storage/AmazonS3.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/files_external/lib/Lib/Storage/AmazonS3.php b/apps/files_external/lib/Lib/Storage/AmazonS3.php index aa0afac15439f..cb5fe134e6f2f 100644 --- a/apps/files_external/lib/Lib/Storage/AmazonS3.php +++ b/apps/files_external/lib/Lib/Storage/AmazonS3.php @@ -204,7 +204,7 @@ private function doesDirectoryExist($path) { return true; } } catch (S3Exception $e) { - if ($e->getStatusCode() === 403) { + if ($e->getStatusCode() >= 400 && $e->getStatusCode() < 500) { $this->directoryCache[$path] = false; } throw $e; @@ -422,7 +422,7 @@ public function filetype($path) { } try { - if (isset($this->directoryCache[$path])) { + if (isset($this->directoryCache[$path]) && $this->directoryCache[$path]) { return 'dir'; } if (isset($this->filesCache[$path]) || $this->headObject($path)) { From a1ca901e58e89a8cd7848910fd9700b4b6f5b402 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 19 Oct 2021 15:03:22 +0200 Subject: [PATCH 23/27] cache versioning enabled status Signed-off-by: Robin Appelman --- .../lib/Lib/Storage/AmazonS3.php | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/apps/files_external/lib/Lib/Storage/AmazonS3.php b/apps/files_external/lib/Lib/Storage/AmazonS3.php index cb5fe134e6f2f..827fd63d1d655 100644 --- a/apps/files_external/lib/Lib/Storage/AmazonS3.php +++ b/apps/files_external/lib/Lib/Storage/AmazonS3.php @@ -51,6 +51,8 @@ use OCP\Constants; use OCP\Files\FileInfo; use OCP\Files\IMimeTypeDetector; +use OCP\ICacheFactory; +use OCP\IMemcache; class AmazonS3 extends \OC\Files\Storage\Common { use S3ConnectionTrait; @@ -75,6 +77,9 @@ public function needsPartFile() { /** @var bool|null */ private $versioningEnabled = null; + /** @var IMemcache */ + private $memCache; + public function __construct($parameters) { parent::__construct($parameters); $this->parseParams($parameters); @@ -82,6 +87,9 @@ public function __construct($parameters) { $this->directoryCache = new CappedMemoryCache(); $this->filesCache = new CappedMemoryCache(); $this->mimeDetector = \OC::$server->get(IMimeTypeDetector::class); + /** @var ICacheFactory $cacheFactory */ + $cacheFactory = \OC::$server->get(ICacheFactory::class); + $this->memCache = $cacheFactory->createLocal('s3-external'); } /** @@ -721,8 +729,14 @@ private function getDirectoryMetaData(string $path): ?array { public function versioningEnabled(): bool { if ($this->versioningEnabled === null) { - $result = $this->getConnection()->getBucketVersioning(['Bucket' => $this->getBucket()]); - $this->versioningEnabled = $result->get('Status') === 'Enabled'; + $cached = $this->memCache->get('versioning-enabled::' . $this->getBucket()); + if ($cached === null) { + $result = $this->getConnection()->getBucketVersioning(['Bucket' => $this->getBucket()]); + $this->versioningEnabled = $result->get('Status') === 'Enabled'; + $this->memCache->set('versioning-enabled::' . $this->getBucket(), $this->versioningEnabled, 60); + } else { + $this->versioningEnabled = $cached; + } } return $this->versioningEnabled; } From 4a3234500c44a65c0b2779ca267cbd134e3d5059 Mon Sep 17 00:00:00 2001 From: nextcloud-command Date: Fri, 22 Oct 2021 04:11:24 +0000 Subject: [PATCH 24/27] Update psalm baseline Signed-off-by: GitHub --- build/psalm-baseline.xml | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 679e85f1f1421..a55b6a2dac189 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1366,12 +1366,6 @@ clearBucket - - - FTP_BINARY - FTP_BINARY - - put @@ -2495,9 +2489,6 @@ $levelNum - - $identifier === false - @@ -2864,11 +2855,8 @@ - + getName - isBuiltin - isBuiltin - isBuiltin From 130ab63ca1c163b23107389fee5bbe37456490ac Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Wed, 20 Oct 2021 18:25:30 +0200 Subject: [PATCH 25/27] Bump doctrine/dbal from 3.0.0 to 3.1.3 Signed-off-by: Christoph Wurst --- 3rdparty | 2 +- lib/private/DB/QueryBuilder/QueryBuilder.php | 4 ++-- lib/private/Repair/SqliteAutoincrement.php | 2 +- lib/public/DB/QueryBuilder/IQueryBuilder.php | 4 ++-- tests/lib/DB/QueryBuilder/QueryBuilderTest.php | 10 +++------- 5 files changed, 9 insertions(+), 13 deletions(-) diff --git a/3rdparty b/3rdparty index b72af468984ee..217764f87ec6b 160000 --- a/3rdparty +++ b/3rdparty @@ -1 +1 @@ -Subproject commit b72af468984ee6b374ff614f2d87419fc12d0e90 +Subproject commit 217764f87ec6bbd359c76e57f0f48897d7c47646 diff --git a/lib/private/DB/QueryBuilder/QueryBuilder.php b/lib/private/DB/QueryBuilder/QueryBuilder.php index 3b1c48306a678..e1f74f5327cac 100644 --- a/lib/private/DB/QueryBuilder/QueryBuilder.php +++ b/lib/private/DB/QueryBuilder/QueryBuilder.php @@ -462,9 +462,9 @@ public function setFirstResult($firstResult) { /** * Gets the position of the first result the query object was set to retrieve (the "offset"). - * Returns NULL if {@link setFirstResult} was not applied to this QueryBuilder. + * Returns 0 if {@link setFirstResult} was not applied to this QueryBuilder. * - * @return integer The position of the first result. + * @return int The position of the first result. */ public function getFirstResult() { return $this->queryBuilder->getFirstResult(); diff --git a/lib/private/Repair/SqliteAutoincrement.php b/lib/private/Repair/SqliteAutoincrement.php index 9bd8fa36deba6..4a8b2a45d3f34 100644 --- a/lib/private/Repair/SqliteAutoincrement.php +++ b/lib/private/Repair/SqliteAutoincrement.php @@ -83,7 +83,7 @@ public function run(IOutput $out) { foreach ($columnNames as $columnName) { $columnSchema = $tableSchema->getColumn($columnName); $columnDiff = new ColumnDiff($columnSchema->getName(), $columnSchema); - $tableDiff->changedColumns[] = $columnDiff; + $tableDiff->changedColumns[$columnSchema->getName()] = $columnDiff; $schemaDiff->changedTables[] = $tableDiff; } } catch (SchemaException $e) { diff --git a/lib/public/DB/QueryBuilder/IQueryBuilder.php b/lib/public/DB/QueryBuilder/IQueryBuilder.php index e3daf4b3b465d..5d1116075d8e9 100644 --- a/lib/public/DB/QueryBuilder/IQueryBuilder.php +++ b/lib/public/DB/QueryBuilder/IQueryBuilder.php @@ -289,9 +289,9 @@ public function setFirstResult($firstResult); /** * Gets the position of the first result the query object was set to retrieve (the "offset"). - * Returns NULL if {@link setFirstResult} was not applied to this QueryBuilder. + * Returns 0 if {@link setFirstResult} was not applied to this QueryBuilder. * - * @return integer The position of the first result. + * @return int The position of the first result. * @since 8.2.0 */ public function getFirstResult(); diff --git a/tests/lib/DB/QueryBuilder/QueryBuilderTest.php b/tests/lib/DB/QueryBuilder/QueryBuilderTest.php index aef1acc40c1d9..1927850470743 100644 --- a/tests/lib/DB/QueryBuilder/QueryBuilderTest.php +++ b/tests/lib/DB/QueryBuilder/QueryBuilderTest.php @@ -102,7 +102,7 @@ protected function deleteTestingRows($appId = 'testFirstResult') { public function dataFirstResult() { return [ - [null, [99, 98, 97, 96, 95, 94, 93, 92, 91]], + [0, [99, 98, 97, 96, 95, 94, 93, 92, 91]], [0, [99, 98, 97, 96, 95, 94, 93, 92, 91]], [1, [98, 97, 96, 95, 94, 93, 92, 91]], [5, [94, 93, 92, 91]], @@ -112,7 +112,7 @@ public function dataFirstResult() { /** * @dataProvider dataFirstResult * - * @param int $firstResult + * @param int|null $firstResult * @param array $expectedSet */ public function testFirstResult($firstResult, $expectedSet) { @@ -121,14 +121,10 @@ public function testFirstResult($firstResult, $expectedSet) { if ($firstResult !== null) { $this->queryBuilder->setFirstResult($firstResult); - - // FIXME Remove this once Doctrine/DBAL is >2.5.1: - // FIXME See https://github.com/doctrine/dbal/pull/782 - $this->queryBuilder->setMaxResults(100); } $this->assertSame( - $firstResult, + $firstResult ?? 0, $this->queryBuilder->getFirstResult() ); From 7dd7256cfe8bdb2e11def60167e1b418458e68ea Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Thu, 21 Oct 2021 14:12:36 +0200 Subject: [PATCH 26/27] Prevent duplicate auth token activity updates The auth token activity logic works as follows * Read auth token * Compare last activity time stamp to current time * Update auth token activity if it's older than x seconds This works fine in isolation but with concurrency that means that occasionally the same token is read simultaneously by two processes and both of these processes will trigger an update of the same row. Affectively the second update doesn't add much value. It might set the time stamp to the exact same time stamp or one a few seconds later. But the last activity is no precise science, we don't need this accuracy. This patch changes the UPDATE query to include the expected value in a comparison with the current data. This results in an affected row when the data in the DB still has an old time stamp, but won't affect a row if the time stamp is (nearly) up to date. This is a micro optimization and will possibly not show any significant performance improvement. Yet in setups with a DB cluster it means that the write node has to send fewer changes to the read nodes due to the lower number of actual changes. Signed-off-by: Christoph Wurst --- .../Token/PublicKeyTokenMapper.php | 39 +++++++++++++++++++ .../Token/PublicKeyTokenProvider.php | 3 +- .../Token/PublicKeyTokenProviderTest.php | 13 +++---- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/lib/private/Authentication/Token/PublicKeyTokenMapper.php b/lib/private/Authentication/Token/PublicKeyTokenMapper.php index 8aab7daf62308..0c532312acee8 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenMapper.php +++ b/lib/private/Authentication/Token/PublicKeyTokenMapper.php @@ -190,4 +190,43 @@ public function hasExpiredTokens(string $uid): bool { return count($data) === 1; } + + /** + * Update the last activity timestamp + * + * In highly concurrent setups it can happen that two parallel processes + * trigger the update at (nearly) the same time. In that special case it's + * not necessary to hit the database with two actual updates. Therefore the + * target last activity is included in the WHERE clause with a few seconds + * of tolerance. + * + * Example: + * - process 1 (P1) reads the token at timestamp 1500 + * - process 1 (P2) reads the token at timestamp 1501 + * - activity update interval is 100 + * + * This means + * + * - P1 will see a last_activity smaller than the current time and update + * the token row + * - If P2 reads after P1 had written, it will see 1600 as last activity + * and the comparison on last_activity won't be truthy. This means no rows + * need to be updated a second time + * - If P2 reads before P1 had written, it will see 1501 as last activity, + * but the comparison on last_activity will still not be truthy and the + * token row is not updated a second time + * + * @param IToken $token + * @param int $now + */ + public function updateActivity(IToken $token, int $now): void { + $qb = $this->db->getQueryBuilder(); + $update = $qb->update($this->getTableName()) + ->set('last_activity', $qb->createNamedParameter($now, IQueryBuilder::PARAM_INT)) + ->where( + $qb->expr()->eq('id', $qb->createNamedParameter($token->getId(), IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT), + $qb->expr()->lt('last_activity', $qb->createNamedParameter($now - 15, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT) + ); + $update->executeStatement(); + } } diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php index 22c62d82fac05..b9cfce6c86930 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -221,9 +221,8 @@ public function updateTokenActivity(IToken $token) { /** @var PublicKeyToken $token */ $now = $this->time->getTime(); if ($token->getLastActivity() < ($now - $activityInterval)) { - // Update token only once per minute $token->setLastActivity($now); - $this->mapper->update($token); + $this->mapper->updateActivity($token, $now); } } diff --git a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php index f27100b5d781c..486660f17c65f 100644 --- a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php +++ b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php @@ -100,10 +100,10 @@ public function testGenerateToken() { public function testUpdateToken() { $tk = new PublicKeyToken(); - $tk->setLastActivity($this->time - 200); $this->mapper->expects($this->once()) - ->method('update') - ->with($tk); + ->method('updateActivity') + ->with($tk, $this->time); + $tk->setLastActivity($this->time - 200); $this->tokenProvider->updateTokenActivity($tk); @@ -112,16 +112,15 @@ public function testUpdateToken() { public function testUpdateTokenDebounce() { $tk = new PublicKeyToken(); - $this->config->method('getSystemValueInt') ->willReturnCallback(function ($value, $default) { return $default; }); - $tk->setLastActivity($this->time - 30); + $this->mapper->expects($this->never()) - ->method('update') - ->with($tk); + ->method('updateActivity') + ->with($tk, $this->time); $this->tokenProvider->updateTokenActivity($tk); } From b30f499597ad8f6fc0ce363ef2591c537cea428f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Fri, 22 Oct 2021 09:24:30 +0200 Subject: [PATCH 27/27] Make calendar schedule options translatable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/dav/templates/schedule-response-options.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/dav/templates/schedule-response-options.php b/apps/dav/templates/schedule-response-options.php index 27c062d88be9e..f5ebb46cb2614 100644 --- a/apps/dav/templates/schedule-response-options.php +++ b/apps/dav/templates/schedule-response-options.php @@ -24,8 +24,8 @@
- - + +