From a8d0d1ef587023f8dd8ea7fca4c7cc2ca760ab5d Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Tue, 6 Aug 2024 12:41:30 +1200 Subject: [PATCH 1/5] TLN Update translations (#11323) --- lang/de.yml | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/lang/de.yml b/lang/de.yml index e13317605..e85811b47 100644 --- a/lang/de.yml +++ b/lang/de.yml @@ -27,9 +27,22 @@ de: SilverStripe\Control\RequestProcessor: INVALID_REQUEST: 'Ungültige Anfrage' REQUEST_ABORTED: 'Anfrage abgebrochen' + SilverStripe\Dev\DevBuildController: + CAN_DEV_BUILD_DESCRIPTION: 'Darf /dev/build ausführen' + CAN_DEV_BUILD_HELP: 'Darf den Build-Befehl ausführen (/dev/build).' + SilverStripe\Dev\DevConfigController: + CAN_DEV_CONFIG_DESCRIPTION: 'Darf /dev/config anzeigen' + CAN_DEV_CONFIG_HELP: 'Darf die gesamte Anwendungskonfiguration einsehen (/dev/config).' SilverStripe\Dev\DevConfirmationController: INFO_DESCRIPTION: 'Bestätige potenziell gefährliche Aktion' INFO_TITLE: Sicherheitsbestätigung + SilverStripe\Dev\DevelopmentAdmin: + ALL_DEV_ADMIN_DESCRIPTION: 'Darf alle /dev-Endpunkte anzeigen und ausführen' + ALL_DEV_ADMIN_HELP: 'Darf alle /dev-Endpunkte anzeigen und ausführen' + PERMISSIONS_CATEGORY: 'Dev Berechtigungen' + SilverStripe\Dev\TaskRunner: + BUILDTASK_CAN_RUN_DESCRIPTION: 'Darf alle /dev/tasks anzeigen und ausführen' + BUILDTASK_CAN_RUN_HELP: 'Darf alle /dev/tasks anzeigen und ausführen (/dev/tasks). Dies kann noch durch individuelle Berechtigungen für die Tasks überschrieben werden' SilverStripe\Forms\CheckboxField: NOANSWER: Nein YESANSWER: Ja @@ -42,6 +55,7 @@ de: CURRENT_PASSWORD_MISSING: 'Bitte geben Sie Ihr derzeitiges Passwort ein.' LOGGED_IN_ERROR: 'Sie müssen eingeloggt sein, um Ihr Passwort ändern zu können!' MAXIMUM: 'Passwörter dürfen maximal {max} Zeichen lang sein.' + RANDOM_IF_EMPTY: 'Wenn dieses Feld leer gelassen wird, wird automatisch ein Zufallspasswort generiert.' SHOWONCLICKTITLE: 'Passwort ändern' SilverStripe\Forms\DateField: NOTSET: 'Nicht gesetzt' @@ -103,7 +117,7 @@ de: Create: Erstellen Delete: Löschen DeletePermissionsFailure: 'Keine Berechtigungen zum löschen' - Deleted: 'Gelöscht {type} {name}' + Deleted: 'Gelöscht {type} "{name}"' Save: Speichern Saved: '{name} {link} gespeichert' SilverStripe\Forms\GridField\GridFieldDetailForm_ItemRequest: @@ -111,6 +125,8 @@ de: NEW: 'Neuen Eintrag hinzufügen' NEXT: 'Gehe zu nächstem Eintrag' PREVIOUS: 'Gehe zu vorherigem Eintrag' + SAVEDUP: 'Erfolgreich gespeichert' + SAVETOASTMESSAGE: '{type} "{title}" erfolgreich gespeichert.' ViewPermissionsFailure: 'Sie haben nicht die nötigen Berechtigungen um {ObjectTitle} aufzurufen.' SilverStripe\Forms\GridField\GridFieldEditButton: EDIT: Bearbeiten @@ -138,6 +154,10 @@ de: IsNullLabel: 'ist NULL' SilverStripe\Forms\NumericField: VALIDATION: "'{value}' ist kein numerischer Wert, nur nummerische Werte sind in diesem Feld erlaubt" + SilverStripe\Forms\SearchableDropdownTrait: + SELECT: Auswählen... + SELECT_OR_TYPE_TO_SEARCH: 'Auswählen oder tippen um zu suchen...' + TYPE_TO_SEARCH: 'Tippen um zu suchen...' SilverStripe\Forms\TextField: VALIDATEMAXLENGTH: 'Der für {name} eingegebene Wert darf nicht mehr als {maxLength} Zeichen lang sein' SilverStripe\Forms\TimeField: @@ -145,11 +165,14 @@ de: SilverStripe\Forms\UrlField: INVALID: 'Bitte geben Sie eine gültige URL ein' SilverStripe\ORM\DataObject: + GENERALSEARCH: 'Generelle Suche' PLURALNAME: DatenObjekte PLURALS: one: 'Ein DatenObjekt' other: '{count} DatenObjekte' SINGULARNAME: DatenObjekt + many_many_FileTracking: Datei-Verfolgung + many_many_LinkTracking: Link-Verfolgung SilverStripe\ORM\FieldType\DBBoolean: ANY: alle NOANSWER: Nein @@ -233,7 +256,11 @@ de: RolesAddEditLink: 'Rollen hinzufügen/editieren' SINGULARNAME: Gruppe Sort: Sortierreihenfolge + ValidationIdentifierAlreadyExists: 'Es existiert bereits eine Gruppe ({group}) mit dem selben {identifier}' + db_AccessAllSubsites: 'Zugang zu allen Unterseiten' db_Description: Beschreibung + db_LastSynced: 'Zuletzt aktualisiert' + db_Locked: Gesperrt db_Sort: Sortierung db_Title: Titel has_many_Groups: Gruppe @@ -241,6 +268,13 @@ de: has_one_Parent: Übergeordnet many_many_Members: Mitglieder many_many_Roles: Rollen + SilverStripe\Security\InheritedPermissionsExtension: + db_CanEditType: 'Kann Typ bearbeiten' + db_CanViewType: 'Kann Typ ansehen' + many_many_EditorGroups: Bearbeitungsgruppen + many_many_EditorMembers: Bearbeitungsnutzer + many_many_ViewerGroups: Betrachtergruppen + many_many_ViewerMembers: Betrachtungsnutzer SilverStripe\Security\LoginAttempt: Email: E-Mail-Adresse EmailHashed: 'E-Mail-Adresse (gehashed)' @@ -263,6 +297,7 @@ de: CURRENT_PASSWORD: 'Derzeitiges Passwort' EDIT_PASSWORD: 'Neues Passwort' EMAIL: E-Mail + EMAIL_FAILED: 'Beim Versuch, Ihnen einen Link zum Zurücksetzen des Passworts per E-Mail zu schicken, ist ein Fehler aufgetreten.' EMPTYNEWPASSWORD: 'Das neue Passwort darf nicht leer sein. Bitte versuchen Sie es erneut.' ENTEREMAIL: 'Bitte geben Sie eine E-Mail-Adresse ein, um einen Link zum Zurücksetzen des Passworts zu erhalten.' ERRORLOCKEDOUT2: 'Ihr Zugang wurde auf Grund von einer unzulässig hohen Anzahl von falschen Zugangsversuchen gesperrt. Bitte versuchen Sie es in {count} Minuten noch einmal.' @@ -281,6 +316,7 @@ de: PLURALS: one: 'Ein Mitglied' other: '{count} Mitglieder' + RequiresPasswordChangeOnNextLogin: 'Erfordert Passwortänderung bei nächster Anmeldung' SINGULARNAME: Benutzer SUBJECTPASSWORDCHANGED: 'Ihr Passwort wurde geändert' SUBJECTPASSWORDRESET: 'Ihr Link zur Passwortrücksetzung' @@ -290,15 +326,34 @@ de: ValidationIdentifierFailed: 'Das vorhandene Mitglied #{id} mit identischer Bezeichnung kann nicht überschrieben werden ({name} = {value}))' WELCOMEBACK: 'Hallo {firstname}. Schön, dass du wieder da bist' YOUROLDPASSWORD: 'Ihr altes Passwort' + belongs_many_many_BlogPosts: Blogbeiträge belongs_many_many_Groups: Gruppe + db_AccountResetExpired: 'Zurücksetzen des Kontos ist abgelaufen' + db_AccountResetHash: 'Hash zum zurücksetzen des Accounts' + db_AutoLoginExpired: 'Automatische Anmeldung abgelaufen' + db_AutoLoginHash: 'Hash für die automatische Anmeldung' + db_BlogProfileSummary: 'Zusammenfassung für das Blogprofil' db_Email: E-Mail + db_FailedLoginCount: 'Anzahl der fehlgeschlagenen Anmeldungen' db_FirstName: Vorname + db_HasSkippedMFARegistration: 'Hat die MFA-Registrierung übersprungen' + db_IsExpired: 'Ist abgelaufen' + db_LastSynced: 'Zuletzt aktualisiert' db_Locale: 'Interface Sprachumgebung' db_LockedOutUntil: 'Gesperrt bis' db_Password: Passwort + db_PasswordEncryption: Passwortverschlüsselung db_PasswordExpiry: 'Ablaufdatum des Passworts' + db_Salt: Salz db_Surname: Nachname db_URLSegment: URL-Segment + db_Username: Nutzername + has_many_LoggedPasswords: 'Protokollierte Passwörter' + has_many_LoginSessions: Login-Sitzungen + has_many_RegisteredMFAMethods: 'Registrierte MFA-Methoden' + has_one_AFile: 'Eine Datei' + has_one_AImage: 'Ein Bild' + has_one_BlogProfileImage: 'Blogprofil Bild' SilverStripe\Security\MemberAuthenticator\CMSMemberLoginForm: AUTHENTICATORNAME: 'CMS Benutzer Login Formular' BUTTONFORGOTPASSWORD: 'Passwort vergessen' @@ -316,6 +371,8 @@ de: other: '{count} Benutzerpasswörter' SINGULARNAME: Benutzerpasswort db_Password: Passwort + db_PasswordEncryption: Passwortverschlüsselung + db_Salt: Salz has_one_Member: Benutzer SilverStripe\Security\PasswordValidator: LOWCHARSTRENGTH: 'Bitte erhöhen Sie die Sicherheit des Passworts, indem Sie auch einige der folgenden Zeichen verwenden: {chars}' @@ -363,6 +420,8 @@ de: PLURALS: one: 'Ein Login Hash' other: '{count} Login Hashes' + db_DeviceID: 'Geräte ID' + db_ExpiryDate: Ablaufdatum has_one_Member: Benutzer SilverStripe\Security\Security: ALREADYLOGGEDIN: 'Sie haben keinen Zugriff auf diese Seite. Wenn Sie ein anderes Konto besitzen, mit dem Sie auf diese Seite zugreifen können, melden Sie sich bitte unten an.' From c3f0104a89aafea617690697dfcefa39980f04b1 Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Tue, 6 Aug 2024 15:20:43 +1200 Subject: [PATCH 2/5] FIX Clear table logic for MySQL 8 --- src/ORM/Connect/MySQLDatabase.php | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/ORM/Connect/MySQLDatabase.php b/src/ORM/Connect/MySQLDatabase.php index 0aeb6067f..8e9e05851 100644 --- a/src/ORM/Connect/MySQLDatabase.php +++ b/src/ORM/Connect/MySQLDatabase.php @@ -566,6 +566,26 @@ class MySQLDatabase extends Database implements TransactionManager */ public function clearTable($table) { - $this->query("TRUNCATE TABLE \"$table\""); + // Not simply using "TRUNCATE TABLE \"$table\"" because DELETE is a lot quicker + // than TRUNCATE which is very relevant during unit testing. Using TRUNCATE will lead to an + // approximately 50% increase it the total time of running unit tests. + // + // Using max(ID) to determine if the table should reset its auto-increment, rather than using + // SELECT "AUTO_INCREMENT" FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_SCHEMA = ? AND TABLE_NAME = ? + // after deleting from the table, because in MySQL 8, under certain conditions, notably + // when running behat, sometimes the auto-increment was being reset to 2 for unknown reasons + $self = $this; + $fn = function () use ($self, $table) { + $maxID = $self->query("SELECT MAX(ID) FROM \"$table\"")->value(); + $self->query("DELETE FROM \"$table\""); + if ($maxID > 0) { + $self->query("ALTER TABLE \"$table\" AUTO_INCREMENT = 1"); + } + }; + if ($this->supportsTransactions()) { + $this->withTransaction($fn); + } else { + $fn(); + } } } From 9fae48bbd74f8f710bb5f72c2fa7f68e1f959c9f Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Tue, 6 Aug 2024 15:35:36 +1200 Subject: [PATCH 3/5] FIX Allow clearing lazyloaded SearchableDropdownField. (#11324) --- src/Forms/SearchableDropdownTrait.php | 2 +- src/Forms/SearchableMultiDropdownField.php | 1 - tests/php/Forms/SearchableDropdownTraitTest.php | 6 +++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Forms/SearchableDropdownTrait.php b/src/Forms/SearchableDropdownTrait.php index 97923f711..d8d164531 100644 --- a/src/Forms/SearchableDropdownTrait.php +++ b/src/Forms/SearchableDropdownTrait.php @@ -29,7 +29,7 @@ trait SearchableDropdownTrait 'search', ]; - private bool $isClearable = false; + private bool $isClearable = true; private bool $isLazyLoaded = false; diff --git a/src/Forms/SearchableMultiDropdownField.php b/src/Forms/SearchableMultiDropdownField.php index 3fe8f8c0a..90608a40d 100644 --- a/src/Forms/SearchableMultiDropdownField.php +++ b/src/Forms/SearchableMultiDropdownField.php @@ -24,6 +24,5 @@ class SearchableMultiDropdownField extends MultiSelectField $this->setLabelField($labelField); $this->addExtraClass('ss-searchable-dropdown-field'); $this->setIsMultiple(true); - $this->setIsClearable(true); } } diff --git a/tests/php/Forms/SearchableDropdownTraitTest.php b/tests/php/Forms/SearchableDropdownTraitTest.php index 7f4858c81..d3796cfa2 100644 --- a/tests/php/Forms/SearchableDropdownTraitTest.php +++ b/tests/php/Forms/SearchableDropdownTraitTest.php @@ -205,16 +205,16 @@ class SearchableDropdownTraitTest extends SapphireTest $field->setForm($form); $schema = $field->getSchemaDataDefaults(); $this->assertFalse($schema['lazyLoad']); - $this->assertFalse($schema['clearable']); + $this->assertTrue($schema['clearable']); $this->assertSame('Select or type to search...', $schema['placeholder']); $this->assertTrue($schema['searchable']); $field->setIsLazyLoaded(true); - $field->setIsClearable(true); + $field->setIsClearable(false); $field->setPlaceholder('My placeholder'); $field->setIsSearchable(false); $schema = $field->getSchemaDataDefaults(); $this->assertTrue($schema['lazyLoad']); - $this->assertTrue($schema['clearable']); + $this->assertFalse($schema['clearable']); $this->assertSame('My placeholder', $schema['placeholder']); $this->assertFalse($schema['searchable']); } From 485bbc2774b25184d1c3c3cdfa18cdcde290a99c Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Tue, 6 Aug 2024 16:09:11 +1200 Subject: [PATCH 4/5] MNT Ignore phpstan errors we can't fix. (#11326) --- src/Forms/HTMLEditor/TinyMCEConfig.php | 1 + src/ORM/DataObject.php | 1 + src/Security/PasswordValidator.php | 1 + 3 files changed, 3 insertions(+) diff --git a/src/Forms/HTMLEditor/TinyMCEConfig.php b/src/Forms/HTMLEditor/TinyMCEConfig.php index 26f408ff8..23eab0c44 100644 --- a/src/Forms/HTMLEditor/TinyMCEConfig.php +++ b/src/Forms/HTMLEditor/TinyMCEConfig.php @@ -731,6 +731,7 @@ class TinyMCEConfig extends HTMLEditorConfig implements i18nEntityProvider } if (isset($preset['i18n'])) { + /** @phpstan-ignore translation.key (we need the key to be dynamic here) */ $preset['text'] = _t( $preset['i18n'], isset($preset['text']) ? $preset['text'] : '' diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index e57fae5aa..b9cb0b57b 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -3891,6 +3891,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } foreach ($types as $type => $attrs) { foreach ($attrs as $name => $spec) { + /** @phpstan-ignore translation.key (we need the key to be dynamic here) */ $autoLabels[$name] = _t( "{$ancestorClass}.{$type}_{$name}", FormField::name_to_label($name) diff --git a/src/Security/PasswordValidator.php b/src/Security/PasswordValidator.php index 179711877..62288201d 100644 --- a/src/Security/PasswordValidator.php +++ b/src/Security/PasswordValidator.php @@ -203,6 +203,7 @@ class PasswordValidator if (preg_match($tests[$name] ?? '', $password ?? '')) { continue; } + /** @phpstan-ignore translation.key (we need the key to be dynamic here) */ $missedTests[] = _t( __CLASS__ . '.STRENGTHTEST' . strtoupper($name ?? ''), $name, From 8c80a4fd1e92b1d0d019f4653065bd749730b358 Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Wed, 7 Aug 2024 10:38:25 +1200 Subject: [PATCH 5/5] FIX Do not suffix trailing slash to external links --- src/Control/Controller.php | 19 +- src/Control/Director.php | 11 +- tests/php/Control/ControllerTest.php | 332 +++++++++++++++--- tests/php/Control/DirectorTest.php | 6 +- .../Middleware/CanonicalURLMiddlewareTest.php | 2 + 5 files changed, 302 insertions(+), 68 deletions(-) diff --git a/src/Control/Controller.php b/src/Control/Controller.php index 5b94f0ad6..299ce72a7 100644 --- a/src/Control/Controller.php +++ b/src/Control/Controller.php @@ -680,6 +680,18 @@ class Controller extends RequestHandler implements TemplateGlobalProvider */ public static function normaliseTrailingSlash(string $url): string { + // Do not normalise external urls + // Note that urls without a scheme such as "www.example.com" will be counted as a relative file + if (!Director::is_site_url($url)) { + return $url; + } + + // Do not modify files + $extension = pathinfo(Director::makeRelative($url), PATHINFO_EXTENSION); + if ($extension) { + return $url; + } + $querystring = null; $fragmentIdentifier = null; @@ -694,14 +706,9 @@ class Controller extends RequestHandler implements TemplateGlobalProvider // Normlise trailing slash $shouldHaveTrailingSlash = Controller::config()->uninherited('add_trailing_slash'); - if ($shouldHaveTrailingSlash - && !str_ends_with($url, '/') - && !preg_match('/^(.*)\.([^\/]*)$/', Director::makeRelative($url)) - ) { - // Add trailing slash if enabled and url does not end with a file extension + if ($shouldHaveTrailingSlash && !str_ends_with($url, '/')) { $url .= '/'; } elseif (!$shouldHaveTrailingSlash) { - // Remove trailing slash if it shouldn't be there $url = rtrim($url, '/'); } diff --git a/src/Control/Director.php b/src/Control/Director.php index 8318e5dd6..119d4e746 100644 --- a/src/Control/Director.php +++ b/src/Control/Director.php @@ -821,7 +821,16 @@ class Director implements TemplateGlobalProvider // Allow extensions to weigh in $isSiteUrl = false; - static::singleton()->extend('updateIsSiteUrl', $isSiteUrl, $url); + // Not using static::singleton() here because it can break + // functional tests such as those in HTTPCacheControlIntegrationTest + // This happens because a singleton of Director is instantiating prior to tests being run, + // because Controller::normaliseTrailingSlash() is called during SapphireTest::setUp(), + // which in turn calls Director::is_site_url() + // For this specific use case we don't need to use dependency injection because the + // chance of the extend() method being customised in projects is low. + // Any extension hooks implementing updateIsSiteUrl() will still be called as expected + $director = new static(); + $director->extend('updateIsSiteUrl', $isSiteUrl, $url); if ($isSiteUrl) { return true; } diff --git a/tests/php/Control/ControllerTest.php b/tests/php/Control/ControllerTest.php index ee1222dda..29d520eb6 100644 --- a/tests/php/Control/ControllerTest.php +++ b/tests/php/Control/ControllerTest.php @@ -276,14 +276,17 @@ class ControllerTest extends FunctionalTest { /* Controller::join_links() will reliably join two URL-segments together so that they will be * appropriately parsed by the URL parser */ + Director::config()->set('alternate_base_url', 'https://www.internal.com'); Controller::config()->set('add_trailing_slash', false); $this->assertEquals("admin/crm/MyForm", Controller::join_links("admin/crm", "MyForm")); $this->assertEquals("admin/crm/MyForm", Controller::join_links("admin/crm/", "MyForm")); - $this->assertEquals("https://www.test.com/admin/crm/MyForm", Controller::join_links("https://www.test.com", "admin/crm/", "MyForm")); + $this->assertEquals("https://www.internal.com/admin/crm/MyForm", Controller::join_links("https://www.internal.com", "admin/crm/", "MyForm")); + $this->assertEquals("https://www.external.com/admin/crm/MyForm", Controller::join_links("https://www.external.com", "admin/crm/", "MyForm")); Controller::config()->set('add_trailing_slash', true); $this->assertEquals("admin/crm/MyForm/", Controller::join_links("admin/crm", "MyForm")); $this->assertEquals("admin/crm/MyForm/", Controller::join_links("admin/crm/", "MyForm")); - $this->assertEquals("https://www.test.com/admin/crm/MyForm/", Controller::join_links("https://www.test.com", "admin/crm/", "MyForm")); + $this->assertEquals("https://www.internal.com/admin/crm/MyForm/", Controller::join_links("https://www.internal.com", "admin/crm/", "MyForm")); + $this->assertEquals("https://www.external.com/admin/crm/MyForm", Controller::join_links("https://www.external.com", "admin/crm/", "MyForm")); /* It will also handle appropriate combination of querystring variables */ Controller::config()->set('add_trailing_slash', false); @@ -293,7 +296,8 @@ class ControllerTest extends FunctionalTest "admin/crm/MyForm?field=1&other=1", Controller::join_links("admin/crm/?field=1", "MyForm?other=1") ); - $this->assertEquals("https://www.test.com/admin/crm/MyForm?flush=1", Controller::join_links("https://www.test.com", "admin/crm/", "MyForm?flush=1")); + $this->assertEquals("https://www.internal.com/admin/crm/MyForm?flush=1", Controller::join_links("https://www.internal.com", "admin/crm/", "MyForm?flush=1")); + $this->assertEquals("https://www.external.com/admin/crm/MyForm?flush=1", Controller::join_links("https://www.external.com", "admin/crm/", "MyForm?flush=1")); Controller::config()->set('add_trailing_slash', true); $this->assertEquals("admin/crm/MyForm/?flush=1", Controller::join_links("admin/crm/?flush=1", "MyForm")); $this->assertEquals("admin/crm/MyForm/?flush=1", Controller::join_links("admin/crm/", "MyForm?flush=1")); @@ -301,7 +305,8 @@ class ControllerTest extends FunctionalTest "admin/crm/MyForm/?field=1&other=1", Controller::join_links("admin/crm/?field=1", "MyForm?other=1") ); - $this->assertEquals("https://www.test.com/admin/crm/MyForm/?flush=1", Controller::join_links("https://www.test.com", "admin/crm/", "MyForm?flush=1")); + $this->assertEquals("https://www.internal.com/admin/crm/MyForm/?flush=1", Controller::join_links("https://www.internal.com", "admin/crm/", "MyForm?flush=1")); + $this->assertEquals("https://www.external.com/admin/crm/MyForm?flush=1", Controller::join_links("https://www.external.com", "admin/crm/", "MyForm?flush=1")); /* It can handle arbitrary numbers of components, and will ignore empty ones */ Controller::config()->set('add_trailing_slash', false); @@ -340,8 +345,12 @@ class ControllerTest extends FunctionalTest Controller::join_links("admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") ); $this->assertEquals( - "https://www.test.com/admin/crm?foo=2&bar=3&baz=1", - Controller::join_links("https://www.test.com", "admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") + "https://www.internal.com/admin/crm?foo=2&bar=3&baz=1", + Controller::join_links("https://www.internal.com", "admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") + ); + $this->assertEquals( + "https://www.external.com/admin/crm?foo=2&bar=3&baz=1", + Controller::join_links("https://www.external.com", "admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") ); Controller::config()->set('add_trailing_slash', true); $this->assertEquals( @@ -349,8 +358,12 @@ class ControllerTest extends FunctionalTest Controller::join_links("admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") ); $this->assertEquals( - "https://www.test.com/admin/crm/?foo=2&bar=3&baz=1", - Controller::join_links("https://www.test.com", "admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") + "https://www.internal.com/admin/crm/?foo=2&bar=3&baz=1", + Controller::join_links("https://www.internal.com", "admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") + ); + $this->assertEquals( + "https://www.external.com/admin/crm?foo=2&bar=3&baz=1", + Controller::join_links("https://www.external.com", "admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3") ); Controller::config()->set('add_trailing_slash', false); @@ -361,8 +374,13 @@ class ControllerTest extends FunctionalTest ); $this->assertEquals('/admin/action', Controller::join_links('/admin', 'action')); $this->assertEquals( - 'https://www.test.com/admin/action', - Controller::join_links('https://www.test.com', '/', '/admin/', '/', '/action'), + 'https://www.internal.com/admin/action', + Controller::join_links('https://www.internal.com', '/', '/admin/', '/', '/action'), + 'Test that multiple slashes are trimmed.' + ); + $this->assertEquals( + 'https://www.external.com/admin/action', + Controller::join_links('https://www.external.com', '/', '/admin/', '/', '/action'), 'Test that multiple slashes are trimmed.' ); Controller::config()->set('add_trailing_slash', true); @@ -373,8 +391,13 @@ class ControllerTest extends FunctionalTest ); $this->assertEquals('/admin/action/', Controller::join_links('/admin', 'action')); $this->assertEquals( - 'https://www.test.com/admin/action/', - Controller::join_links('https://www.test.com', '/', '/admin/', '/', '/action'), + 'https://www.internal.com/admin/action/', + Controller::join_links('https://www.internal.com', '/', '/admin/', '/', '/action'), + 'Test that multiple slashes are trimmed.' + ); + $this->assertEquals( + 'https://www.external.com/admin/action', + Controller::join_links('https://www.external.com', '/', '/admin/', '/', '/action'), 'Test that multiple slashes are trimmed.' ); @@ -391,8 +414,12 @@ class ControllerTest extends FunctionalTest Controller::join_links("my-page#subsection", "?arg=var", "#second-section") ); $this->assertEquals( - "https://www.test.com/my-page?arg=var#second-section", - Controller::join_links("https://www.test.com", "my-page#subsection", "?arg=var", "#second-section") + "https://www.internal.com/my-page?arg=var#second-section", + Controller::join_links("https://www.internal.com", "my-page#subsection", "?arg=var", "#second-section") + ); + $this->assertEquals( + "https://www.external.com/my-page?arg=var#second-section", + Controller::join_links("https://www.external.com", "my-page#subsection", "?arg=var", "#second-section") ); Controller::config()->set('add_trailing_slash', true); $this->assertEquals( @@ -400,8 +427,12 @@ class ControllerTest extends FunctionalTest Controller::join_links("my-page#subsection", "?arg=var", "#second-section") ); $this->assertEquals( - "https://www.test.com/my-page/?arg=var#second-section", - Controller::join_links("https://www.test.com", "my-page#subsection", "?arg=var", "#second-section") + "https://www.internal.com/my-page/?arg=var#second-section", + Controller::join_links("https://www.internal.com", "my-page#subsection", "?arg=var", "#second-section") + ); + $this->assertEquals( + "https://www.external.com/my-page?arg=var#second-section", + Controller::join_links("https://www.external.com", "my-page#subsection", "?arg=var", "#second-section") ); /* Does type-safe checks for zero value */ @@ -413,60 +444,245 @@ class ControllerTest extends FunctionalTest // Test array args Controller::config()->set('add_trailing_slash', false); $this->assertEquals( - "https://www.test.com/admin/crm/MyForm?a=1&b=2&c=3", - Controller::join_links(["https://www.test.com", "?a=1", "admin/crm", "?b=2", "MyForm?c=3"]) + "https://www.internal.com/admin/crm/MyForm?a=1&b=2&c=3", + Controller::join_links(["https://www.internal.com", "?a=1", "admin/crm", "?b=2", "MyForm?c=3"]) + ); + $this->assertEquals( + "https://www.external.com/admin/crm/MyForm?a=1&b=2&c=3", + Controller::join_links(["https://www.external.com", "?a=1", "admin/crm", "?b=2", "MyForm?c=3"]) ); Controller::config()->set('add_trailing_slash', true); $this->assertEquals( - "https://www.test.com/admin/crm/MyForm/?a=1&b=2&c=3", - Controller::join_links(["https://www.test.com", "?a=1", "admin/crm", "?b=2", "MyForm?c=3"]) + "https://www.internal.com/admin/crm/MyForm/?a=1&b=2&c=3", + Controller::join_links(["https://www.internal.com", "?a=1", "admin/crm", "?b=2", "MyForm?c=3"]) + ); + $this->assertEquals( + "https://www.external.com/admin/crm/MyForm?a=1&b=2&c=3", + Controller::join_links(["https://www.external.com", "?a=1", "admin/crm", "?b=2", "MyForm?c=3"]) ); } - public function testNormaliseTrailingSlash() + public function provideNormaliseTrailingSlash(): array { - foreach ([true, false] as $withTrailingSlash) { - Controller::config()->set('add_trailing_slash', $withTrailingSlash); - $slash = $withTrailingSlash ? '/' : ''; - + // note 93.184.215.14 is the IP address for example.com + return [ // Correctly gives slash to a relative root path - $this->assertEquals('/', Controller::normaliseTrailingSlash('')); - $this->assertEquals('/', Controller::normaliseTrailingSlash('/')); - + [ + 'path' => '', + 'withSlash' => '/', + 'withoutSlash' => '/', + ], + [ + 'path' => '/', + 'withSlash' => '/', + 'withoutSlash' => '/', + ], // Correctly adds or removes trailing slash - $this->assertEquals("some/path{$slash}", Controller::normaliseTrailingSlash('some/path/')); - $this->assertEquals("some/path{$slash}", Controller::normaliseTrailingSlash('some/path')); - + [ + 'path' => 'some/path/', + 'withSlash' => 'some/path/', + 'withoutSlash' => 'some/path', + ], // Retains leading slash, if there is one - $this->assertEquals("/some/path{$slash}", Controller::normaliseTrailingSlash('/some/path/')); - $this->assertEquals("/some/path{$slash}", Controller::normaliseTrailingSlash('/some/path')); - - // Effectively treats absolute URL as relative - $this->assertEquals("https://www.google.com/some/path{$slash}", Controller::normaliseTrailingSlash('https://www.google.com/some/path/')); - $this->assertEquals("//www.google.com/some/path{$slash}", Controller::normaliseTrailingSlash('//www.google.com/some/path')); - $this->assertEquals("www.google.com/some/path{$slash}", Controller::normaliseTrailingSlash('www.google.com/some/path')); - $this->assertEquals("https://www.google.com{$slash}", Controller::normaliseTrailingSlash('https://www.google.com')); - $this->assertEquals("//www.google.com{$slash}", Controller::normaliseTrailingSlash('//www.google.com/')); - + [ + 'path' => '/some/path/', + 'withSlash' => '/some/path/', + 'withoutSlash' => '/some/path', + ], + // Treat absolute URLs pointing to the current site as relative + [ + 'path' => '/some/path/', + 'withSlash' => '/some/path/', + 'withoutSlash' => '/some/path', + ], + [ + 'path' => '/', + 'withSlash' => '/', + 'withoutSlash' => '', + ], + [ + 'path' => '', + 'withSlash' => '/', + 'withoutSlash' => '', + ], + // External links never get normalised + [ + 'path' => 'https://www.example.com/some/path', + 'withSlash' => 'https://www.example.com/some/path', + 'withoutSlash' => 'https://www.example.com/some/path', + ], + [ + 'path' => 'https://www.example.com/some/path/', + 'withSlash' => 'https://www.example.com/some/path/', + 'withoutSlash' => 'https://www.example.com/some/path/', + ], + [ + 'path' => 'https://www.example.com', + 'withSlash' => 'https://www.example.com', + 'withoutSlash' => 'https://www.example.com', + ], + [ + 'path' => 'https://www.example.com/', + 'withSlash' => 'https://www.example.com/', + 'withoutSlash' => 'https://www.example.com/', + ], + [ + 'path' => '//www.example.com/some/path', + 'withSlash' => '//www.example.com/some/path', + 'withoutSlash' => '//www.example.com/some/path', + ], + [ + 'path' => '//www.example.com/some/path/', + 'withSlash' => '//www.example.com/some/path/', + 'withoutSlash' => '//www.example.com/some/path/', + ], + [ + 'path' => '//www.example.com', + 'withSlash' => '//www.example.com', + 'withoutSlash' => '//www.example.com', + ], + [ + 'path' => '//www.example.com/', + 'withSlash' => '//www.example.com/', + 'withoutSlash' => '//www.example.com/', + ], + [ + 'path' => 'https://93.184.215.14/some/path', + 'withSlash' => 'https://93.184.215.14/some/path', + 'withoutSlash' => 'https://93.184.215.14/some/path', + ], + [ + 'path' => 'https://93.184.215.14/some/path/', + 'withSlash' => 'https://93.184.215.14/some/path/', + 'withoutSlash' => 'https://93.184.215.14/some/path/', + ], + // Links without a scheme with a path are treated as relative + // Note: content authors should be specifying a scheme in these cases themselves + [ + 'path' => 'www.example.com/some/path', + 'withSlash' => 'www.example.com/some/path/', + 'withoutSlash' => 'www.example.com/some/path', + ], + [ + 'path' => 'www.example.com/some/path/', + 'withSlash' => 'www.example.com/some/path/', + 'withoutSlash' => 'www.example.com/some/path', + ], + [ + 'path' => '93.184.215.14/some/path', + 'withSlash' => '93.184.215.14/some/path/', + 'withoutSlash' => '93.184.215.14/some/path', + ], + [ + 'path' => '93.184.215.14/some/path/', + 'withSlash' => '93.184.215.14/some/path/', + 'withoutSlash' => '93.184.215.14/some/path', + ], + // Links without a scheme or path are treated like files i.e. not altered + // Note: content authors should be specifying a scheme in these cases themselves + [ + 'path' => 'www.example.com', + 'withSlash' => 'www.example.com', + 'withoutSlash' => 'www.example.com', + ], + [ + 'path' => 'www.example.com/', + 'withSlash' => 'www.example.com/', + 'withoutSlash' => 'www.example.com/', + ], + [ + 'path' => '93.184.215.14', + 'withSlash' => '93.184.215.14', + 'withoutSlash' => '93.184.215.14', + ], + [ + 'path' => '93.184.215.14/', + 'withSlash' => '93.184.215.14/', + 'withoutSlash' => '93.184.215.14/', + ], // Retains query string and anchor if present - $this->assertEquals("some/path{$slash}?key=value&key2=value2", Controller::normaliseTrailingSlash('some/path/?key=value&key2=value2')); - $this->assertEquals("some/path{$slash}#some-id", Controller::normaliseTrailingSlash('some/path/#some-id')); - $this->assertEquals("some/path{$slash}?key=value&key2=value2#some-id", Controller::normaliseTrailingSlash('some/path/?key=value&key2=value2#some-id')); - $this->assertEquals("some/path{$slash}?key=value&key2=value2", Controller::normaliseTrailingSlash('some/path?key=value&key2=value2')); - $this->assertEquals("some/path{$slash}#some-id", Controller::normaliseTrailingSlash('some/path#some-id')); - $this->assertEquals("some/path{$slash}?key=value&key2=value2#some-id", Controller::normaliseTrailingSlash('some/path?key=value&key2=value2#some-id')); - + [ + 'path' => 'some/path/?key=value&key2=value2', + 'withSlash' => 'some/path/?key=value&key2=value2', + 'withoutSlash' => 'some/path?key=value&key2=value2', + ], + [ + 'path' => 'some/path/#some-id', + 'withSlash' => 'some/path/#some-id', + 'withoutSlash' => 'some/path#some-id', + ], + [ + 'path' => 'some/path?key=value&key2=value2#some-id', + 'withSlash' => 'some/path/?key=value&key2=value2#some-id', + 'withoutSlash' => 'some/path?key=value&key2=value2#some-id', + ], + [ + 'path' => 'some/path?key=value&key2=value2', + 'withSlash' => 'some/path/?key=value&key2=value2', + 'withoutSlash' => 'some/path?key=value&key2=value2', + ], + [ + 'path' => 'some/path#some-id', + 'withSlash' => 'some/path/#some-id', + 'withoutSlash' => 'some/path#some-id', + ], + [ + 'path' => 'some/path?key=value&key2=value2#some-id', + 'withSlash' => 'some/path/?key=value&key2=value2#some-id', + 'withoutSlash' => 'some/path?key=value&key2=value2#some-id', + ], // Don't ever add a trailing slash to the end of a URL that looks like a file - $this->assertEquals("https://www.google.com/some/file.txt", Controller::normaliseTrailingSlash('https://www.google.com/some/file.txt')); - $this->assertEquals("//www.google.com/some/file.txt", Controller::normaliseTrailingSlash('//www.google.com/some/file.txt')); - $this->assertEquals("www.google.com/some/file.txt", Controller::normaliseTrailingSlash('www.google.com/some/file.txt')); - $this->assertEquals("/some/file.txt", Controller::normaliseTrailingSlash('/some/file.txt')); - $this->assertEquals("some/file.txt", Controller::normaliseTrailingSlash('some/file.txt')); - $this->assertEquals("file.txt", Controller::normaliseTrailingSlash('file.txt')); - $this->assertEquals("some/file.txt?key=value&key2=value2#some-id", Controller::normaliseTrailingSlash('some/file.txt?key=value&key2=value2#some-id')); - // NOTE: `www.google.com` is already treated as "relative" by Director::makeRelative(), which means we can't tell that it's a host (and not a file). - $this->assertEquals("www.google.com", Controller::normaliseTrailingSlash('www.google.com')); - } + [ + 'path' => 'https://www.example.com/some/file.txt', + 'withSlash' => 'https://www.example.com/some/file.txt', + 'withoutSlash' => 'https://www.example.com/some/file.txt', + ], + [ + 'path' => '//www.example.com/some/file.txt', + 'withSlash' => '//www.example.com/some/file.txt', + 'withoutSlash' => '//www.example.com/some/file.txt', + ], + [ + 'path' => 'www.example.com/some/file.txt', + 'withSlash' => 'www.example.com/some/file.txt', + 'withoutSlash' => 'www.example.com/some/file.txt', + ], + [ + 'path' => '/some/file.txt', + 'withSlash' => '/some/file.txt', + 'withoutSlash' => '/some/file.txt', + ], + [ + 'path' => 'some/file.txt', + 'withSlash' => 'some/file.txt', + 'withoutSlash' => 'some/file.txt', + ], + [ + 'path' => 'file.txt', + 'withSlash' => 'file.txt', + 'withoutSlash' => 'file.txt', + ], + [ + 'path' => 'some/file.txt?key=value&key2=value2#some-id', + 'withSlash' => 'some/file.txt?key=value&key2=value2#some-id', + 'withoutSlash' => 'some/file.txt?key=value&key2=value2#some-id', + ], + ]; + } + + /** + * @dataProvider provideNormaliseTrailingSlash + */ + public function testNormaliseTrailingSlash(string $path, string $withSlash, string $withoutSlash): void + { + $absBaseUrlNoSlash = rtrim(Director::absoluteBaseURL(), '/'); + $path = str_replace('', $absBaseUrlNoSlash, $path); + $withSlash = str_replace('', $absBaseUrlNoSlash, $withSlash); + $withoutSlash = str_replace('', $absBaseUrlNoSlash, $withoutSlash); + Controller::config()->set('add_trailing_slash', true); + $this->assertEquals($withSlash, Controller::normaliseTrailingSlash($path), 'With trailing slash test'); + Controller::config()->set('add_trailing_slash', false); + $this->assertEquals($withoutSlash, Controller::normaliseTrailingSlash($path), 'Without trailing slash test'); } public function testLink() diff --git a/tests/php/Control/DirectorTest.php b/tests/php/Control/DirectorTest.php index f0332322e..1c65b5bb3 100644 --- a/tests/php/Control/DirectorTest.php +++ b/tests/php/Control/DirectorTest.php @@ -109,21 +109,21 @@ class DirectorTest extends SapphireTest // Test Director::BASE $this->assertEquals("http://www.mysite.com:9090{$slash}", Director::absoluteURL('http://www.mysite.com:9090/', Director::BASE)); - $this->assertEquals("http://www.mytest.com{$slash}", Director::absoluteURL('http://www.mytest.com', Director::BASE)); + $this->assertEquals("http://www.mytest.com", Director::absoluteURL('http://www.mytest.com', Director::BASE)); $this->assertEquals("http://www.mysite.com:9090/test{$slash}", Director::absoluteURL("http://www.mysite.com:9090/test", Director::BASE)); $this->assertEquals("http://www.mysite.com:9090/root{$slash}", Director::absoluteURL("/root", Director::BASE)); $this->assertEquals("http://www.mysite.com:9090/root/url{$slash}", Director::absoluteURL("/root/url", Director::BASE)); // Test Director::ROOT $this->assertEquals("http://www.mysite.com:9090{$slash}", Director::absoluteURL('http://www.mysite.com:9090/', Director::ROOT)); - $this->assertEquals("http://www.mytest.com{$slash}", Director::absoluteURL('http://www.mytest.com', Director::ROOT)); + $this->assertEquals("http://www.mytest.com", Director::absoluteURL('http://www.mytest.com', Director::ROOT)); $this->assertEquals("http://www.mysite.com:9090/test{$slash}", Director::absoluteURL("http://www.mysite.com:9090/test", Director::ROOT)); $this->assertEquals("http://www.mysite.com:9090/root{$slash}", Director::absoluteURL("/root", Director::ROOT)); $this->assertEquals("http://www.mysite.com:9090/root/url{$slash}", Director::absoluteURL("/root/url", Director::ROOT)); // Test Director::REQUEST $this->assertEquals("http://www.mysite.com:9090{$slash}", Director::absoluteURL('http://www.mysite.com:9090/', Director::REQUEST)); - $this->assertEquals("http://www.mytest.com{$slash}", Director::absoluteURL('http://www.mytest.com', Director::REQUEST)); + $this->assertEquals("http://www.mytest.com", Director::absoluteURL('http://www.mytest.com', Director::REQUEST)); $this->assertEquals("http://www.mysite.com:9090/test{$slash}", Director::absoluteURL("http://www.mysite.com:9090/test", Director::REQUEST)); $this->assertEquals("http://www.mysite.com:9090/root{$slash}", Director::absoluteURL("/root", Director::REQUEST)); $this->assertEquals("http://www.mysite.com:9090/root/url{$slash}", Director::absoluteURL("/root/url", Director::REQUEST)); diff --git a/tests/php/Control/Middleware/CanonicalURLMiddlewareTest.php b/tests/php/Control/Middleware/CanonicalURLMiddlewareTest.php index 5689fb767..2770874ab 100644 --- a/tests/php/Control/Middleware/CanonicalURLMiddlewareTest.php +++ b/tests/php/Control/Middleware/CanonicalURLMiddlewareTest.php @@ -8,6 +8,7 @@ use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\Middleware\CanonicalURLMiddleware; use SilverStripe\Core\Environment; use SilverStripe\Dev\SapphireTest; +use SilverStripe\Control\Director; class CanonicalURLMiddlewareTest extends SapphireTest { @@ -121,6 +122,7 @@ class CanonicalURLMiddlewareTest extends SapphireTest private function performRedirectTest(string $requestURL, CanonicalURLMiddleware $middleware, bool $shouldRedirect, bool $addTrailingSlash) { + Director::config()->set('alternate_base_url', 'https://www.example.com'); Environment::setEnv('REQUEST_URI', $requestURL); $request = new HTTPRequest('GET', $requestURL); $request->setScheme('https');