From 55096c2a58701e0d982f0c79d930c0b08099b690 Mon Sep 17 00:00:00 2001
From: Igor Biki <ibiki@uwaterloo.ca>
Date: Thu, 20 Feb 2025 15:49:36 -0500
Subject: [PATCH 1/8] ISTWCMS-7266: Refactor link handling logic in UwCblLinks
block.
Replaced redundant base path handling with streamlined logic for link cleaning and title overrides. Introduced new parameters to `cleanUpLink()` for better flexibility and removed unused `prepareLink()` method. This improves code readability while maintaining functionality.
---
src/Plugin/Block/UwCblLinks.php | 97 ++++++++++++++-------------------
1 file changed, 41 insertions(+), 56 deletions(-)
diff --git a/src/Plugin/Block/UwCblLinks.php b/src/Plugin/Block/UwCblLinks.php
index 9d5631e8..7b5f9a64 100644
--- a/src/Plugin/Block/UwCblLinks.php
+++ b/src/Plugin/Block/UwCblLinks.php
@@ -266,13 +266,8 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
// If this is not an external link, get the friendly path
// to the page. If it is external, just use the value.
elseif (!UrlHelper::isExternal($value)) {
- // Get the base path of the site, this is required for Pantheon.
- $base_path = $this->requestStack
- ->getCurrentRequest()
- ?->getBasePath();
-
// Add the url to the links array.
- $links['items'][$count][$index] = $this->prepareLink($value, $base_path);
+ $links['items'][$count][$index] = $this->cleanUpLink($value, TRUE, FALSE);
}
else {
@@ -284,10 +279,10 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
// See if link title needs to be modified.
if (empty($item['title'])) {
- $links['items'][$count]['title'] = $this->getTitleOverride($item['uri'], $base_path);
+ $links['items'][$count][$index] = $this->getTitleOverride($item['uri']);
}
else {
- $links['items'][$count]['title'] = $value;
+ $links['items'][$count][$index] = $value;
}
}
else {
@@ -312,11 +307,6 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
// Array to store the links.
$links = [];
- // Base path if exists.
- $base_path = $this->requestStack
- ->getCurrentRequest()
- ?->getBasePath();
-
// Attach the js library.
$form['#attached']['library'][] = 'uw_custom_blocks/uw_cbl_links';
@@ -472,7 +462,7 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
// If there is a link already set, setup the details to
// have the link in the title so when it is closed,
// we can see what the actual is.
- $value_uri = $this->cleanUpLink($links['items'][$i]['uri'] ?? '', $base_path);
+ $value_uri = $this->cleanUpLink($links['items'][$i]['uri'] ?? '');
if (isset($links['items'][$i]['uri'])) {
$link = $this->t('Link (@uri)', ['@uri' => $value_uri]);
@@ -928,17 +918,24 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
*
* @param string $raw_link
* The original link that needs to be cleaned up.
- * @param string|null $base_path
- * The base path to remove from the link if it exists, default NULL.
+ * @param bool $return_alias
+ * Should alias be returned instead of an internal url.
+ * @param bool $prefix_base_path
+ * Should base_path be concateneted before url.
*
* @return string
* The cleaned link with specified prefixes removed.
*/
- private function cleanUpLink(string $raw_link, ?string $base_path = NULL): string {
+ private function cleanUpLink(string $raw_link, bool $return_alias = FALSE, bool $prefix_base_path = TRUE): string {
if (empty($raw_link)) {
return $raw_link;
}
+ // Get the base path of the site, this is required for Pantheon.
+ $base_path = $this->requestStack
+ ->getCurrentRequest()
+ ?->getBasePath();
+
// Function to modify its own variable, leaving function argument unchanged.
$link = $raw_link;
@@ -949,17 +946,12 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
$link = substr_replace($link, '', 0, strlen('internal:'));
}
- // If a link starts with base_path, remove it, but only when base_path
- // is not NULL. And make sure the resulting link starts with slash.
+ // Remove base path, this will be used to validate url.
if ($base_path && str_starts_with($link, $base_path)) {
$link = substr_replace($link, '', 0, strlen($base_path));
-
- if (!str_starts_with($link, '/')) {
- $link = '/' . $link;
- }
}
- // If an external link is prefix with leading slash.
+ // If an external link is detected, and it has a leading slash, remove it.
$updated = preg_replace(self::LEADING_SLASH_REPLACE_REGEX, '$1', $link);
// In case match is not found, unchanged value is returned.
@@ -968,38 +960,31 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
$link = $updated;
}
- return $link;
- }
-
- /**
- * Prepares and sanitizes a link by removing specific base path prefixes.
- *
- * This method removes the base path and 'internal:/' prefix from the
- * provided link to ensure it is in the desired format.
- *
- * @param string $link
- * The link to be sanitized.
- * @param string|null $base_path
- * The base path to be removed if the link starts with it.
- *
- * @return string
- * The sanitized link without the specified prefixes.
- */
- private function prepareLink(string $link, ?string $base_path = NULL): string {
+ $path = $this->pathValidator->getUrlIfValidWithoutAccessCheck($link);
- if (empty($link)) {
- return $link;
- }
+ // If node alias is used, modify it so a canonical link is used.
+ if ($path && !$path->isExternal()) {
+ if ($return_alias) {
+ $canonical = $path->toString();
+ }
+ else {
+ $canonical = $path->getInternalPath();
+ }
- $url = $this->cleanUpLink($link, $base_path);
+ if (!str_starts_with($canonical, '/')) {
+ $canonical = '/' . $canonical;
+ }
- $path = $this->pathValidator->getUrlIfValidWithoutAccessCheck($url);
+ if ($base_path && $prefix_base_path) {
+ $canonical = $base_path . $canonical;
+ }
- if ($path && !$path->isExternal()) {
- return $path->toString();
+ if ($link !== $canonical) {
+ $link = $canonical;
+ }
}
- return $url;
+ return $link;
}
/**
@@ -1011,17 +996,15 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
*
* @param string $url
* The URL to validate and extract the title from.
- * @param string|null $base_path
- * Base path.
*
* @return string
* The overridden title is if available, or an empty string
* if no title can be determined.
*/
- private function getTitleOverride(string $url, ?string $base_path = NULL): string {
+ private function getTitleOverride(string $url): string {
$title = "";
- $link = $this->cleanUpLink($url, $base_path);
+ $link = $this->cleanUpLink($url, prefix_base_path: FALSE);
/** @var \Drupal\Core\Url|bool $route_check */
$path = $this->pathValidator->getUrlIfValidWithoutAccessCheck($link);
@@ -1036,8 +1019,10 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
->load($params['view_id']);
if ($view) {
- $view->getDisplay($params['display_id']);
- $title = $view->label();
+ $display = $view->getDisplay($params['display_id']);
+
+ // Get display id title, if that fails, use view title.
+ $title = $display['display_options']['title'] ?? $view->label();
}
}
// This part should take care of nodes, taxonomy terms and webforms.
--
GitLab
From 6a61d02597c9393324ada972c94900ac8784f106 Mon Sep 17 00:00:00 2001
From: Igor Biki <ibiki@uwaterloo.ca>
Date: Mon, 24 Feb 2025 14:14:25 -0500
Subject: [PATCH 2/8] ISTWCMS-7266: Refactor URL validation logic for internal
links.
Extracted base path validation into a reusable private method `getInternalUrlIfValid`, improving code clarity and maintainability. Simplified the handling of leading slashes for external links by updating comments and removing redundant logic.
---
src/Plugin/Block/UwCblLinks.php | 41 +++++++++++++++++++++++++++------
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/src/Plugin/Block/UwCblLinks.php b/src/Plugin/Block/UwCblLinks.php
index 7b5f9a64..a0203796 100644
--- a/src/Plugin/Block/UwCblLinks.php
+++ b/src/Plugin/Block/UwCblLinks.php
@@ -8,6 +8,7 @@ use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Path\PathValidatorInterface;
use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
+use Drupal\Core\Url;
use Drupal\media\Entity\Media;
use Drupal\path_alias\AliasManager;
use Drupal\uw_cfg_common\Service\UWService;
@@ -946,12 +947,8 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
$link = substr_replace($link, '', 0, strlen('internal:'));
}
- // Remove base path, this will be used to validate url.
- if ($base_path && str_starts_with($link, $base_path)) {
- $link = substr_replace($link, '', 0, strlen($base_path));
- }
-
- // If an external link is detected, and it has a leading slash, remove it.
+ // In some cases, the external link is prefixed with a leading slash. This
+ // removes that leading slash.
$updated = preg_replace(self::LEADING_SLASH_REPLACE_REGEX, '$1', $link);
// In case match is not found, unchanged value is returned.
@@ -960,7 +957,7 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
$link = $updated;
}
- $path = $this->pathValidator->getUrlIfValidWithoutAccessCheck($link);
+ $path = $this->getInternalUrlIfValid($link, $base_path);
// If node alias is used, modify it so a canonical link is used.
if ($path && !$path->isExternal()) {
@@ -1045,4 +1042,34 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
return $title ?? "";
}
+ /**
+ * Validates and retrieves an internal URL if applicable.
+ *
+ * Checks if the provided URL starts with the specified base path
+ * and attempts to validate it as an internal path without access checks.
+ *
+ * @param string $url
+ * The URL to be validated.
+ * @param string $base_path
+ * The base path to check against.
+ *
+ * @return \Drupal\Core\Url|null
+ * The valid internal URL object if the URL is valid and matches the
+ * base path, or NULL otherwise.
+ */
+ private function getInternalUrlIfValid(string $url, string $base_path): ?Url {
+ $link = $url;
+
+ // Remove base path, this will be used to validate url.
+ if ($base_path && str_starts_with($link, $base_path)) {
+ $link = substr_replace($link, '', 0, strlen($base_path));
+ }
+
+ if ($path = $this->pathValidator->getUrlIfValidWithoutAccessCheck($link)) {
+ return $path;
+ }
+
+ return NULL;
+ }
+
}
--
GitLab
From 7075c9f2c09adae9e78c4af187e4f2cac309474f Mon Sep 17 00:00:00 2001
From: Igor Biki <ibiki@uwaterloo.ca>
Date: Thu, 27 Feb 2025 09:08:36 -0500
Subject: [PATCH 3/8] ISTWCMS-7266: Fix regex to validate URLs with case
insensitivity.
Updated the regex pattern to ensure it validates URLs with "http" or "https" in a case-insensitive manner. This change improves user input validation and prevents errors caused by mismatched cases.
---
src/Plugin/Block/UwCblLinks.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/Plugin/Block/UwCblLinks.php b/src/Plugin/Block/UwCblLinks.php
index a0203796..d9e66e8b 100644
--- a/src/Plugin/Block/UwCblLinks.php
+++ b/src/Plugin/Block/UwCblLinks.php
@@ -687,7 +687,7 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
// set the form error.
if (
$value !== '' &&
- !preg_match('/^(http(s):\/\/.)/', $value) &&
+ !preg_match('/^(https?:\/\/.)/i', $value) &&
!preg_match('/^[\/#]/', $value)
) {
--
GitLab
From 5e5b2f88739ad15303fe9f7f77f9b8b9d7dfa000 Mon Sep 17 00:00:00 2001
From: Igor Biki <ibiki@uwaterloo.ca>
Date: Thu, 27 Feb 2025 11:40:57 -0500
Subject: [PATCH 4/8] ISTWCMS-7266: Normalize URLs to lowercase before base
path validation
This change ensures that URLs are converted to lowercase during the base path validation process, improving consistency and reducing potential mismatches due to casing. It enhances the robustness of URL handling within the custom block functionality.
---
src/Plugin/Block/UwCblLinks.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/Plugin/Block/UwCblLinks.php b/src/Plugin/Block/UwCblLinks.php
index d9e66e8b..fc27d512 100644
--- a/src/Plugin/Block/UwCblLinks.php
+++ b/src/Plugin/Block/UwCblLinks.php
@@ -1061,7 +1061,7 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
$link = $url;
// Remove base path, this will be used to validate url.
- if ($base_path && str_starts_with($link, $base_path)) {
+ if ($base_path && str_starts_with(strtolower($link), $base_path)) {
$link = substr_replace($link, '', 0, strlen($base_path));
}
--
GitLab
From 6d4b7a2e2fff1571d8144c823def7819a39d62da Mon Sep 17 00:00:00 2001
From: Igor Biki <ibiki@uwaterloo.ca>
Date: Fri, 28 Feb 2025 13:55:44 -0500
Subject: [PATCH 5/8] ISTWCMS-7266: Add URI cleanup logic to UwCblLinks block
links processing
Previously, the 'uri' key in links was not being sanitized. This update ensures that the 'uri' is processed using the cleanUpLink method with base path prefixing enabled, improving link consistency and security.
---
src/Plugin/Block/UwCblLinks.php | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/Plugin/Block/UwCblLinks.php b/src/Plugin/Block/UwCblLinks.php
index fc27d512..23f6b84b 100644
--- a/src/Plugin/Block/UwCblLinks.php
+++ b/src/Plugin/Block/UwCblLinks.php
@@ -775,6 +775,9 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
) {
$links['items'][$count][$key] = NULL;
}
+ elseif ($key === 'uri') {
+ $links['items'][$count][$key] = $this->cleanUpLink($item['link_info'][$key], prefix_base_path: TRUE);
+ }
else {
$links['items'][$count][$key] = $item['link_info'][$key];
}
--
GitLab
From 713546d1946a416d7365ea8ece4191a1b9aef7a8 Mon Sep 17 00:00:00 2001
From: Igor Biki <ibiki@uwaterloo.ca>
Date: Fri, 28 Feb 2025 14:19:42 -0500
Subject: [PATCH 6/8] ISTWCMS-7266: Handle fragments and query strings in
canonical path generation
Previously, fragment or query string components were not appended to canonical paths. This update ensures these components are retained by detecting and appending them to the generated path. It improves the accuracy and completeness of internal path handling.
---
src/Plugin/Block/UwCblLinks.php | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/Plugin/Block/UwCblLinks.php b/src/Plugin/Block/UwCblLinks.php
index 23f6b84b..510b6d90 100644
--- a/src/Plugin/Block/UwCblLinks.php
+++ b/src/Plugin/Block/UwCblLinks.php
@@ -969,6 +969,13 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
}
else {
$canonical = $path->getInternalPath();
+
+ // Detects if there is a fragment or query string.
+ $fragment_query = preg_replace('/^[^#?]+/', '', $path->toString());
+
+ if (!empty($fragment_query)) {
+ $canonical .= $fragment_query;
+ }
}
if (!str_starts_with($canonical, '/')) {
--
GitLab
From 5df6c910e04d2cde3f7d804c34ef852e8cfbc1bf Mon Sep 17 00:00:00 2001
From: Igor Biki <ibiki@uwaterloo.ca>
Date: Fri, 28 Feb 2025 15:27:11 -0500
Subject: [PATCH 7/8] ISTWCMS-7266: Refine fragment/query handling in
UwCblLinks block
Ensure fragment or query strings are appended only if they start with `#` or `?`. This prevents unintended behavior where links remain unchanged when conditions are not met.
---
src/Plugin/Block/UwCblLinks.php | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/Plugin/Block/UwCblLinks.php b/src/Plugin/Block/UwCblLinks.php
index 510b6d90..ddb00529 100644
--- a/src/Plugin/Block/UwCblLinks.php
+++ b/src/Plugin/Block/UwCblLinks.php
@@ -973,7 +973,9 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
// Detects if there is a fragment or query string.
$fragment_query = preg_replace('/^[^#?]+/', '', $path->toString());
- if (!empty($fragment_query)) {
+ // Need to check if fragment/query starts with # or ?, to avoid
+ // a scenario where the link is returned unchanged.
+ if (!empty($fragment_query) && (str_starts_with($fragment_query, '#') || str_starts_with($fragment_query, '?'))) {
$canonical .= $fragment_query;
}
}
--
GitLab
From bbf9be3a2e9698c7cb7cc9616c012d22dbde0958 Mon Sep 17 00:00:00 2001
From: Igor Biki <ibiki@uwaterloo.ca>
Date: Wed, 5 Mar 2025 10:53:51 -0500
Subject: [PATCH 8/8] ISTWCMS-7266: Fix link processing to handle base path
more accurately
Removed unnecessary prefix_base_path parameter and adjusted base path comparison to include a trailing slash. These updates ensure the URL handling logic is more consistent and robust.
---
src/Plugin/Block/UwCblLinks.php | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/Plugin/Block/UwCblLinks.php b/src/Plugin/Block/UwCblLinks.php
index ddb00529..cd16c5a5 100644
--- a/src/Plugin/Block/UwCblLinks.php
+++ b/src/Plugin/Block/UwCblLinks.php
@@ -776,7 +776,7 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
$links['items'][$count][$key] = NULL;
}
elseif ($key === 'uri') {
- $links['items'][$count][$key] = $this->cleanUpLink($item['link_info'][$key], prefix_base_path: TRUE);
+ $links['items'][$count][$key] = $this->cleanUpLink($item['link_info'][$key]);
}
else {
$links['items'][$count][$key] = $item['link_info'][$key];
@@ -1073,7 +1073,7 @@ class UwCblLinks extends BlockBase implements ContainerFactoryPluginInterface {
$link = $url;
// Remove base path, this will be used to validate url.
- if ($base_path && str_starts_with(strtolower($link), $base_path)) {
+ if ($base_path && str_starts_with(strtolower($link), $base_path . '/')) {
$link = substr_replace($link, '', 0, strlen($base_path));
}
--
GitLab