Fix migration 7: URL placeholders incorrectly replaced

mstilkerich · Apr 3, 2024 · 8e9e9a7 · 8e9e9a7
1 parent a7762a2
commit 8e9e9a7
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Changelog for RCMCardDAV
 
+## Version 5.x.x (to 5.1.0)
+
+- Fix migration 7: The migration replaced placeholders in the addressbook URLs with the data of the user during whose
+  login the migration was executed. This would result in broken URLs in such cases.
+
 ## Version 5.1.0 (to 5.0.1)
 
 - Allow to hide contacts without email also for user-defined addressbooks (Fixes #429)

diff --git a/dbmigrations/0007-replaceurlplaceholders/migrate.php b/dbmigrations/0007-replaceurlplaceholders/migrate.php
@@ -59,7 +59,7 @@ public function migrate(rcube_db $dbh, LoggerInterface $logger): bool
         }
 
         while ($row = $dbh->fetch_assoc($sql_result)) {
-            $url = self::replacePlaceholdersUrl((string) $row["url"]);
+            $url = self::replacePlaceholdersUrl((string) $row["url"], (string) $row["username"]);
             if ($url != $row["url"]) {
                 if ($dbh->query("UPDATE $abook_table SET url=? WHERE id=?", $url, $row["id"]) === false) {
                     $logger->error(
@@ -80,14 +80,15 @@ public function migrate(rcube_db $dbh, LoggerInterface $logger): bool
         return true;
     }
 
-    private static function replacePlaceholdersUrl(string $url): string
+    private static function replacePlaceholdersUrl(string $url, string $username): string
     {
-        $rcmail = rcmail::get_instance();
+        $comp = explode("@", $username, 2);
+
         return strtr($url, [
-            '%u' => (string) $_SESSION['username'],
-            '%l' => $rcmail->user->get_username('local'),
-            '%d' => $rcmail->user->get_username('domain'),
-            '%V' => strtr((string) $_SESSION['username'], "@.", "__")
+            '%u' => $username,
+            '%l' => $comp[0],
+            '%d' => $comp[1] ?? '',
+            '%V' => strtr($username, "@.", "__")
         ]);
     }
 }

diff --git a/tests/DBInteroperability/DatabaseMigrationTest.php b/tests/DBInteroperability/DatabaseMigrationTest.php
@@ -115,6 +115,28 @@ private static function resetRcubeDb(): void
             ],
         ],
 
+        '0007-replaceurlplaceholders' => [
+            'insertRows' => [
+                [
+                    'table' => 'carddav_addressbooks',
+                    'cols' => [
+                        'name', 'username', 'password', 'url', 'active',
+                        'user_id', 'presetname', 'use_categories',
+                    ],
+                    'rows' => [
+                        [
+                            'StructUrl0', '%u', '%p', 'https://%l.x.de/%u/%V/%d/personal', '1',
+                            [ 'users', 0, '0000-dbinit' ], null, '1'
+                        ],
+                        [
+                            'StructUrl0', '%u', '%p', 'https://%l.x.de/%u/%V/%d/personal', '1',
+                            [ 'users', 1, '0000-dbinit' ], null, '1'
+                        ],
+                    ],
+                ],
+            ],
+        ],
+
         '0017-accountentities' => [
             // these rows are inserted before the migration is executed
             // datasets need to match the DB schema before the migration
@@ -189,6 +211,16 @@ private static function resetRcubeDb(): void
                             'Radicale', 'ruser', 'rpass', 'https://radicale.example.com/dav/',
                             [ 'users', 1, '0000-dbinit' ], '0', '86400', null
                         ],
+                        [
+                            'StructUrl0', '%u', '%p',
+                            'https://testuser.x.de/testuser@example.com/testuser_example_com/example.com/',
+                            [ 'users', 0, '0000-dbinit' ], '0', '86400', null
+                        ],
+                        [
+                            'StructUrl0', '%u', '%p',
+                            'https://another.x.de/another@user.com/another_user_com/user.com/',
+                            [ 'users', 1, '0000-dbinit' ], '0', '86400', null
+                        ],
                     ],
                 ],
                 [
@@ -198,6 +230,18 @@ private static function resetRcubeDb(): void
                         'account_id', 'last_updated', 'refresh_time', 'sync_token', 'use_categories',
                     ],
                     'rows' => [
+                        [
+                            'StructUrl0',
+                            'https://testuser.x.de/testuser@example.com/testuser_example_com/example.com/personal',
+                            '1',
+                            [ 'carddav_accounts', 4, '<checkRows>' ], '0', '3600', '', '1'
+                        ],
+                        [
+                            'StructUrl0',
+                            'https://another.x.de/another@user.com/another_user_com/user.com/personal',
+                            '1',
+                            [ 'carddav_accounts', 5, '<checkRows>' ], '0', '3600', '', '1'
+                        ],
                         [
                             'Personal', 'https://nc.cloud.com/c/personal', '1',
                             [ 'carddav_accounts', 0, '<checkRows>' ], '1626690000', '1234', 'sync@123', '1'
@@ -243,6 +287,16 @@ private static function resetRcubeDb(): void
                         [ 'Nextcloud',  'https://nc.cloud.com/c/', 'admPreset' ],
                         [ 'Nextcloud', 'https://nc.cloud.com/c/', null ],
                         [ 'Radicale', 'https://radicale.example.com/dav/', null ],
+                        [
+                            'StructUrl0',
+                            'https://testuser.x.de/testuser@example.com/testuser_example_com/example.com/',
+                            null
+                        ],
+                        [
+                            'StructUrl0',
+                            'https://another.x.de/another@user.com/another_user_com/user.com/',
+                            null
+                        ],
                     ],
                 ],
             ],