From a4edb6adc09abb1ca52e92d80111173bfa206132 Mon Sep 17 00:00:00 2001 From: Derek Wright Date: Wed, 6 Dec 2023 16:42:51 -1000 Subject: [PATCH 1/8] Bug #2418369: Initial Functional tests to show bugs in URL aliases + language handling --- .../PathologicLanguageAliasTest.php | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 tests/src/Functional/PathologicLanguageAliasTest.php diff --git a/tests/src/Functional/PathologicLanguageAliasTest.php b/tests/src/Functional/PathologicLanguageAliasTest.php new file mode 100644 index 0000000..a4f7c84 --- /dev/null +++ b/tests/src/Functional/PathologicLanguageAliasTest.php @@ -0,0 +1,158 @@ +setUpCurrentUser(); + + $this->createContentType([ + 'type' => 'page', + 'name' => 'Basic page', + ]); + + // Add more languages. + ConfigurableLanguage::createFromLangcode('fr')->save(); + ConfigurableLanguage::createFromLangcode('pt-br')->save(); + + // Enable URL language detection and selection. + \Drupal::configFactory()->getEditable('language.negotiation') + ->set('url.prefixes.fr', 'fr') + ->set('url.prefixes.pt-br', 'pt-br') + ->save(); + + // Configure Pathologic on a text format. + $this->buildFormat([ + 'settings_source' => 'local', + 'local_settings' => [ + 'protocol_style' => 'path', + ], + ]); + + // To reflect the changes for a multilingual site, rebuild the container. + $this->container->get('kernel')->rebuildContainer(); + } + + /** + * Tests how links to nodes and files are handled with translations. + */ + public function testContentTranslation(): void { + + // Create a node that will be referenced in a link inside another node. + $node_to_reference = $this->createNode([ + 'type' => 'page', + 'title' => 'Reference page', + ]); + + // Add translations and path aliases for the reference node, and try a whole + // series of possible input texts to see how they are handled. + $fr_reference = $node_to_reference->addTranslation('fr', [ + 'title' => 'Page de référence en français', + ])->save(); + $pt_br_reference = $node_to_reference->addTranslation('pt-br', [ + 'title' => 'Página de referência em Português', + ])->save(); + + $this->createPathAlias('/node/' . $node_to_reference->id(), '/reference-en', 'en'); + $this->createPathAlias('/node/' . $node_to_reference->id(), '/reference-fr', 'fr'); + $this->createPathAlias('/node/' . $node_to_reference->id(), '/referencia-pt', 'pt-br'); + + global $base_path; + $nid = $node_to_reference->id(); + + // The link replacement shouldn't change for any of these based on the language the filter runs with. + foreach (['en', 'fr', 'pt-br'] as $langcode) { + $this->assertSame( + 'Test file link', + $this->runFilter('Test file link', $langcode), + "$langcode: file links do not get a language prefix", + ); + $this->assertSame( + 'Test node link', + $this->runFilter('Test node link', $langcode), + "$langcode: node/N link uses EN alias", + ); + $this->assertSame( + 'Test node link', + $this->runFilter('Test node link', $langcode), + "$langcode: fr/node/N link uses the FR alias", + ); + $this->assertSame( + 'Test node link', + $this->runFilter('Test node link', $langcode), + "$langcode: pt-br/node/N link uses the PT-BR alias", + ); + $this->assertSame( + 'Test node link', + $this->runFilter('Test node link', $langcode), + "$langcode: /reference-en link uses EN alias", + ); + /* + $this->assertSame( + 'Test node link', + $this->runFilter('Test node link', $langcode), + "$langcode: fr/reference-fr link uses the FR alias", + ); + $this->assertSame( + 'Test node link', + $this->runFilter('Test node link', $langcode), + "$langcode: pt-br/referencia-pt uses the PT-BR alias", + ); + */ + } + + } + +} -- GitLab From 675f8a824970a5ebc13297e7e21af7125d93a01c Mon Sep 17 00:00:00 2001 From: Randal Vanheede <56416-RandalV@users.noreply.drupalcode.org> Date: Wed, 6 Dec 2023 16:45:46 -1000 Subject: [PATCH 2/8] Bug #2418369: Internal URL handling (language prefixes, base://, ...) (patch #52 rerolled for 2.0.x) --- pathologic.module | 62 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/pathologic.module b/pathologic.module index 8dcf7d8..7a53d7c 100644 --- a/pathologic.module +++ b/pathologic.module @@ -26,6 +26,7 @@ declare(strict_types=1); use Drupal\Component\Utility\Html; +use Drupal\Core\StreamWrapper\PublicStream; use Drupal\Core\Url; /** @@ -128,13 +129,14 @@ function _pathologic_replace($matches) { // guess… Note that we don't do the & thing here so that we can modify // $cached_settings later and not have the changes be "permanent." $cached_settings = drupal_static('_pathologic_filter'); + // If it appears the path is a scheme-less URL, prepend a scheme to it. // parse_url() cannot properly parse scheme-less URLs. Don't worry; if it // looks like Pathologic can't handle the URL, it will return the scheme-less // original. // @see https://drupal.org/node/1617944 // @see https://drupal.org/node/2030789 - if (strpos($matches[2], '//') === 0) { + if (str_starts_with($matches[2], '//')) { if (\Drupal::request()->isSecure()) { $matches[2] = 'https:' . $matches[2]; } @@ -180,6 +182,12 @@ function _pathologic_replace($matches) { $parts['path'] = ''; } + // Variable to define whether we need to rewrite/transform the URL through a + // URL object. + $dont_rewrite = FALSE; + // Variable to define whether the given path is a file path. + $is_file = FALSE; + // Check to see if we're dealing with a file. // @todo Should we still try to do path correction on these files too? if (isset($parts['scheme']) && $parts['scheme'] === 'files') { @@ -194,10 +202,14 @@ function _pathologic_replace($matches) { $new_parts['path'] = rawurldecode($new_parts['path']); $parts = $new_parts; // Don't do language handling for file paths. - $cached_settings['is_file'] = TRUE; + $is_file = TRUE; } - else { - $cached_settings['is_file'] = FALSE; + // Check to see if instead of a 'files:' scheme we have a normal internal + // file url starting with the public base path. + elseif (str_contains($parts['path'], PublicStream::basePath())) { + // This url should not be turned into a URL object, because we don't want + // language handling for this path. + $dont_rewrite = TRUE; } // Let's also bail out of this doesn't look like a local path. @@ -291,14 +303,14 @@ function _pathologic_replace($matches) { // If we didn't previously identify this as a file, check to see if the file // exists now that we have the correct path relative to DRUPAL_ROOT - if (!$cached_settings['is_file']) { - $cached_settings['is_file'] = !empty($parts['path']) && is_file(DRUPAL_ROOT . '/' . $parts['path']); + if (!$is_file) { + $is_file = !empty($parts['path']) && is_file(DRUPAL_ROOT . '/' . $parts['path']); } - // Okay, deal with language stuff. - // Let's see if we can split off a language prefix from the path. - $keep_language_prefix = $cached_settings['current_settings']['keep_language_prefix'] ?? FALSE; - if ($keep_language_prefix === FALSE && \Drupal::moduleHandler()->moduleExists('language')) { + // Let's see if path has a language prefix, so we can distinguish the target + // language for this path. + $specific_language = NULL; + if (\Drupal::moduleHandler()->moduleExists('language')) { // This logic is based on // \Drupal\language\Plugin\LanguageNegotiation\LanguageNegotiationUrl::getLangcode(). $languages = \Drupal::languageManager()->getLanguages(); @@ -311,8 +323,10 @@ function _pathologic_replace($matches) { // Search for prefix within added languages. foreach ($languages as $language) { if (isset($config['prefixes'][$language->getId()]) && $config['prefixes'][$language->getId()] == $prefix) { - $parts['path'] = implode('/', $path_args); - $parts['language_obj'] = $language; + $specific_language = $language; + if ($cached_settings['current_settings']['keep_language_prefix'] ?? FALSE === FALSE) { + $parts['path'] = implode('/', $path_args); + } break; } } @@ -336,7 +350,7 @@ function _pathologic_replace($matches) { 'absolute' => $cached_settings['current_settings']['protocol_style'] !== 'path', // If we seem to have found a language for the path, pass it along to // url(). Otherwise, ignore the 'language' parameter. - 'language' => isset($parts['language_obj']) ? $parts['language_obj'] : NULL, + 'language' => $specific_language ?? NULL, // A special parameter not actually used by url(), but we use it to see if // an alter hook implementation wants us to just pass through the original // URL. @@ -361,8 +375,28 @@ function _pathologic_replace($matches) { if ($parts['path'] == '') { $url = Url::fromRoute('', [], $url_params['options'])->toString(); } + elseif ($dont_rewrite) { + // The URL is just the internal path that doesn't need rewriting. + $url = $parts['path']; + if (!str_starts_with($url, '/')) { + $url = '/' . $url; + } + } else { - $path = (empty($url_params['options']['external']) ? 'base://' : '') . $url_params['path']; + // If we've been told this is already an external URL, leave it alone. + if (!empty($url_params['options']['external'])) { + $scheme = ''; + } + // If it's a file, use 'base:' so that the path is not re-written. + elseif ($is_file) { + $scheme = 'base:/'; + } + // For everything we did not recognize as external or files, we use the + // internal scheme so aliases and language prefixes are set correctly. + else { + $scheme = 'internal:/'; + } + $path = $scheme . $url_params['path']; try { $url = Url::fromUri($path, $url_params['options'])->toString(); } -- GitLab From 965a2119360aba4ff3dbdda043ebb91cf1e59c58 Mon Sep 17 00:00:00 2001 From: Derek Wright Date: Wed, 6 Dec 2023 16:46:32 -1000 Subject: [PATCH 3/8] Bug #2418369: Run all assertions, these now fail due to stripping langcode unnecessarily --- tests/src/Functional/PathologicLanguageAliasTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/src/Functional/PathologicLanguageAliasTest.php b/tests/src/Functional/PathologicLanguageAliasTest.php index a4f7c84..ec01361 100644 --- a/tests/src/Functional/PathologicLanguageAliasTest.php +++ b/tests/src/Functional/PathologicLanguageAliasTest.php @@ -139,7 +139,6 @@ class PathologicLanguageAliasTest extends BrowserTestBase { $this->runFilter('Test node link', $langcode), "$langcode: /reference-en link uses EN alias", ); - /* $this->assertSame( 'Test node link', $this->runFilter('Test node link', $langcode), @@ -150,7 +149,6 @@ class PathologicLanguageAliasTest extends BrowserTestBase { $this->runFilter('Test node link', $langcode), "$langcode: pt-br/referencia-pt uses the PT-BR alias", ); - */ } } -- GitLab From 2104987a4983a027394eae180581cd916cfa8cdc Mon Sep 17 00:00:00 2001 From: Derek Wright Date: Thu, 7 Dec 2023 09:27:15 -1000 Subject: [PATCH 4/8] [#2418369]: Remove unused use statements from PathologicLanguageAliasTest --- tests/src/Functional/PathologicLanguageAliasTest.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/src/Functional/PathologicLanguageAliasTest.php b/tests/src/Functional/PathologicLanguageAliasTest.php index ec01361..6872909 100644 --- a/tests/src/Functional/PathologicLanguageAliasTest.php +++ b/tests/src/Functional/PathologicLanguageAliasTest.php @@ -4,11 +4,7 @@ declare(strict_types=1); namespace Drupal\Tests\pathologic\Functional; -use Drupal\Core\Language\Language; -use Drupal\Core\StreamWrapper\PublicStream; -use Drupal\Core\Url; use Drupal\language\Entity\ConfigurableLanguage; -use Drupal\node\Entity\Node; use Drupal\Tests\BrowserTestBase; use Drupal\Tests\node\Traits\ContentTypeCreationTrait; use Drupal\Tests\node\Traits\NodeCreationTrait; -- GitLab From 6fa75b34140c7e2719f954b3df2b59446ac976d9 Mon Sep 17 00:00:00 2001 From: Derek Wright Date: Thu, 7 Dec 2023 14:27:13 -1000 Subject: [PATCH 5/8] [#2418369] Merge test coverage into PathologicLanguageTest from #3388563 --- .../PathologicLanguageAliasTest.php | 152 ------------------ .../src/Functional/PathologicLanguageTest.php | 38 +++-- 2 files changed, 23 insertions(+), 167 deletions(-) delete mode 100644 tests/src/Functional/PathologicLanguageAliasTest.php diff --git a/tests/src/Functional/PathologicLanguageAliasTest.php b/tests/src/Functional/PathologicLanguageAliasTest.php deleted file mode 100644 index 6872909..0000000 --- a/tests/src/Functional/PathologicLanguageAliasTest.php +++ /dev/null @@ -1,152 +0,0 @@ -setUpCurrentUser(); - - $this->createContentType([ - 'type' => 'page', - 'name' => 'Basic page', - ]); - - // Add more languages. - ConfigurableLanguage::createFromLangcode('fr')->save(); - ConfigurableLanguage::createFromLangcode('pt-br')->save(); - - // Enable URL language detection and selection. - \Drupal::configFactory()->getEditable('language.negotiation') - ->set('url.prefixes.fr', 'fr') - ->set('url.prefixes.pt-br', 'pt-br') - ->save(); - - // Configure Pathologic on a text format. - $this->buildFormat([ - 'settings_source' => 'local', - 'local_settings' => [ - 'protocol_style' => 'path', - ], - ]); - - // To reflect the changes for a multilingual site, rebuild the container. - $this->container->get('kernel')->rebuildContainer(); - } - - /** - * Tests how links to nodes and files are handled with translations. - */ - public function testContentTranslation(): void { - - // Create a node that will be referenced in a link inside another node. - $node_to_reference = $this->createNode([ - 'type' => 'page', - 'title' => 'Reference page', - ]); - - // Add translations and path aliases for the reference node, and try a whole - // series of possible input texts to see how they are handled. - $fr_reference = $node_to_reference->addTranslation('fr', [ - 'title' => 'Page de référence en français', - ])->save(); - $pt_br_reference = $node_to_reference->addTranslation('pt-br', [ - 'title' => 'Página de referência em Português', - ])->save(); - - $this->createPathAlias('/node/' . $node_to_reference->id(), '/reference-en', 'en'); - $this->createPathAlias('/node/' . $node_to_reference->id(), '/reference-fr', 'fr'); - $this->createPathAlias('/node/' . $node_to_reference->id(), '/referencia-pt', 'pt-br'); - - global $base_path; - $nid = $node_to_reference->id(); - - // The link replacement shouldn't change for any of these based on the language the filter runs with. - foreach (['en', 'fr', 'pt-br'] as $langcode) { - $this->assertSame( - 'Test file link', - $this->runFilter('Test file link', $langcode), - "$langcode: file links do not get a language prefix", - ); - $this->assertSame( - 'Test node link', - $this->runFilter('Test node link', $langcode), - "$langcode: node/N link uses EN alias", - ); - $this->assertSame( - 'Test node link', - $this->runFilter('Test node link', $langcode), - "$langcode: fr/node/N link uses the FR alias", - ); - $this->assertSame( - 'Test node link', - $this->runFilter('Test node link', $langcode), - "$langcode: pt-br/node/N link uses the PT-BR alias", - ); - $this->assertSame( - 'Test node link', - $this->runFilter('Test node link', $langcode), - "$langcode: /reference-en link uses EN alias", - ); - $this->assertSame( - 'Test node link', - $this->runFilter('Test node link', $langcode), - "$langcode: fr/reference-fr link uses the FR alias", - ); - $this->assertSame( - 'Test node link', - $this->runFilter('Test node link', $langcode), - "$langcode: pt-br/referencia-pt uses the PT-BR alias", - ); - } - - } - -} diff --git a/tests/src/Functional/PathologicLanguageTest.php b/tests/src/Functional/PathologicLanguageTest.php index b8d825d..1d28c58 100644 --- a/tests/src/Functional/PathologicLanguageTest.php +++ b/tests/src/Functional/PathologicLanguageTest.php @@ -9,6 +9,7 @@ use Drupal\Tests\BrowserTestBase; use Drupal\Tests\node\Traits\ContentTypeCreationTrait; use Drupal\Tests\node\Traits\NodeCreationTrait; use Drupal\Tests\pathologic\Traits\PathologicFormatTrait; +use Drupal\Tests\Traits\Core\PathAliasTestTrait; use Drupal\Tests\user\Traits\UserCreationTrait; /** @@ -20,6 +21,7 @@ class PathologicLanguageTest extends BrowserTestBase { use ContentTypeCreationTrait; use NodeCreationTrait; + use PathAliasTestTrait; use PathologicFormatTrait; use UserCreationTrait; @@ -32,6 +34,8 @@ class PathologicLanguageTest extends BrowserTestBase { 'language', 'locale', 'node', + 'path', + 'path_alias', 'pathologic', 'text', 'user', @@ -94,7 +98,7 @@ class PathologicLanguageTest extends BrowserTestBase { 'title' => 'Reference page', ]); - // Add translations for the reference node, and try a whole + // Add translations and path aliases for the reference node, and try a whole // series of possible input texts to see how they are handled. $node_to_reference->addTranslation('fr', [ 'title' => 'Page de référence en français', @@ -103,6 +107,10 @@ class PathologicLanguageTest extends BrowserTestBase { 'title' => 'Página de referência em Português', ])->save(); + $this->createPathAlias('/node/' . $node_to_reference->id(), '/reference-en', 'en'); + $this->createPathAlias('/node/' . $node_to_reference->id(), '/reference-fr', 'fr'); + $this->createPathAlias('/node/' . $node_to_reference->id(), '/referencia-pt', 'pt-br'); + global $base_path; $nid = $node_to_reference->id(); @@ -115,19 +123,19 @@ class PathologicLanguageTest extends BrowserTestBase { "$langcode: file links do not get a language prefix", ); $this->assertSame( - 'Test node link', + 'Test node link', $this->runFilter('Test node link', $langcode), - "$langcode: node/N link should be unchanged" + "$langcode: node/N link uses EN alias", ); $this->assertSame( - 'Test node link', + 'Test node link', $this->runFilter('Test node link', $langcode), - "$langcode: fr/node/N link should be unchanged", + "$langcode: fr/node/N link uses the FR alias", ); $this->assertSame( - 'Test node link', + 'Test node link', $this->runFilter('Test node link', $langcode), - "$langcode: pt-br/node/N link should be unchanged", + "$langcode: pt-br/node/N link uses the PT-BR alias", ); $this->assertSame( 'Test node link', @@ -161,19 +169,19 @@ class PathologicLanguageTest extends BrowserTestBase { "$langcode: file links do not get a language prefix", ); $this->assertSame( - 'Test node link', + 'Test node link', $this->runFilter('Test node link', $langcode), - "$langcode: node/N link is unchanged" + "$langcode: node/N link uses EN alias", ); $this->assertSame( - 'Test node link', + 'Test node link', $this->runFilter('Test node link', $langcode), - "$langcode: fr/node/N link should have no langcode prefix" + "$langcode: fr/node/N link should use the FR alias with langcode, even if prefix is stripped", ); $this->assertSame( - 'Test node link', + 'Test node link', $this->runFilter('Test node link', $langcode), - "$langcode: pt-br/node/N link should have no langcode prefix" + "$langcode: pt-br/node/N link should use the PT-BR alias with langcode, even if the prefix is stripped", ); $this->assertSame( 'Test node link', @@ -183,12 +191,12 @@ class PathologicLanguageTest extends BrowserTestBase { $this->assertSame( 'Test node link', $this->runFilter('Test node link', $langcode), - "$langcode: fr/reference-fr link should have no langcode prefix", + "$langcode: fr/reference-fr link (no URL processing) uses the FR alias with langcode removed", ); $this->assertSame( 'Test node link', $this->runFilter('Test node link', $langcode), - "$langcode: pt-br/referencia-pt link should have no langcode prefix", + "$langcode: pt-br/referencia-pt link (no URL processing) uses the PT-BR alias with langcode removed", ); } } -- GitLab From c0e6d0a8a36a8e6980e03bc0dee3b47ab8458725 Mon Sep 17 00:00:00 2001 From: Derek Wright Date: Thu, 14 Dec 2023 17:30:59 -1000 Subject: [PATCH 6/8] [#2418369] Fix logic bug introduced by merge conflict. --- pathologic.module | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pathologic.module b/pathologic.module index 7a53d7c..2daced6 100644 --- a/pathologic.module +++ b/pathologic.module @@ -324,7 +324,7 @@ function _pathologic_replace($matches) { foreach ($languages as $language) { if (isset($config['prefixes'][$language->getId()]) && $config['prefixes'][$language->getId()] == $prefix) { $specific_language = $language; - if ($cached_settings['current_settings']['keep_language_prefix'] ?? FALSE === FALSE) { + if (($cached_settings['current_settings']['keep_language_prefix'] ?? FALSE) === FALSE) { $parts['path'] = implode('/', $path_args); } break; -- GitLab From d0780e4b4bee60b342671f1686f12022d825affe Mon Sep 17 00:00:00 2001 From: Trent Crawford Date: Tue, 17 Jun 2025 11:01:59 +0200 Subject: [PATCH 7/8] [#2418369] Prevent system file URLs being broken when processed with the internal scheme. --- pathologic.module | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pathologic.module b/pathologic.module index 2daced6..bc40d4f 100644 --- a/pathologic.module +++ b/pathologic.module @@ -397,6 +397,15 @@ function _pathologic_replace($matches) { $scheme = 'internal:/'; } $path = $scheme . $url_params['path']; + + // We pass $url_param['options'] to fromUri(). + // If this contains query params these will override the file query param + // provided by PathProcessorFiles, which is invoked by path validator in + // Url::fromInternalUri(). This would result in the path being broken + // (resolving to just /system/files). + if (str_starts_with($path, 'internal:/system/files')) { + unset($url_params['options']['query']); + } try { $url = Url::fromUri($path, $url_params['options'])->toString(); } -- GitLab From 14b4f15cfb652a693b522a658ec33115ae0a7548 Mon Sep 17 00:00:00 2001 From: Trent Crawford Date: Tue, 17 Jun 2025 12:09:00 +0200 Subject: [PATCH 8/8] [#2418369] Add test coverage for paths to a system file. --- tests/src/Kernel/PathologicTest.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/src/Kernel/PathologicTest.php b/tests/src/Kernel/PathologicTest.php index cde31e6..6ec9b55 100644 --- a/tests/src/Kernel/PathologicTest.php +++ b/tests/src/Kernel/PathologicTest.php @@ -75,6 +75,12 @@ class PathologicTest extends PathologicKernelTestBase { 'path' => '', 'opts' => [], ], + 'system/files/myfile.pdf' => [ + 'path' => 'system/files', + 'opts' => [ + 'query' => ['file' => 'myfile.pdf'], + ], + ], ]; foreach (['full', 'proto-rel', 'path'] as $protocol_style) { @@ -146,6 +152,11 @@ class PathologicTest extends PathologicKernelTestBase { $this->runFilter(''), "Path with just slash. Clean URLs: $clean; protocol style: $protocol_style.", ); + $this->assertSame( + '', + $this->runFilter(''), + "Path to system file. Clean URLs: $clean; protocol style: $protocol_style.", + ); } global $base_path; -- GitLab