From 56e7439d20caa67879072124c5231d4b51ca3044 Mon Sep 17 00:00:00 2001
From: Lee Rowlands <lee.rowlands@previousnext.com.au>
Date: Wed, 31 Jan 2024 08:46:12 +1000
Subject: [PATCH] Issue #2885351 by Nikolay Shapovalov, paulocs,
clemens.tolboom, larowlan, dcam, mrshowerman, smustgrave, alexpott,
borisson_: Query string duplications
---
.../Field/FieldFormatter/LinkFormatter.php | 7 +-
.../tests/src/Functional/LinkFieldTest.php | 184 +++++++++++++++++-
.../link/tests/src/Unit/LinkFormatterTest.php | 1 -
3 files changed, 186 insertions(+), 6 deletions(-)
diff --git a/core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php b/core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php
index c8709c568999..86ecad00407b 100644
--- a/core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php
+++ b/core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php
@@ -205,16 +205,15 @@ public function viewElements(FieldItemListInterface $items, $langcode) {
}
}
else {
+ // Skip the #options to prevent duplications of query parameters.
$element[$delta] = [
'#type' => 'link',
'#title' => $link_title,
- '#options' => $url->getOptions(),
+ '#url' => $url,
];
- $element[$delta]['#url'] = $url;
if (!empty($item->_attributes)) {
- $element[$delta]['#options'] += ['attributes' => []];
- $element[$delta]['#options']['attributes'] += $item->_attributes;
+ $element[$delta]['#attributes'] = $item->_attributes;
// Unset field item attributes since they have been included in the
// formatter output and should not be rendered in the field template.
unset($item->_attributes);
diff --git a/core/modules/link/tests/src/Functional/LinkFieldTest.php b/core/modules/link/tests/src/Functional/LinkFieldTest.php
index a0b118f65c16..185ea6f833d0 100644
--- a/core/modules/link/tests/src/Functional/LinkFieldTest.php
+++ b/core/modules/link/tests/src/Functional/LinkFieldTest.php
@@ -179,6 +179,9 @@ protected function doTestURLValidation() {
'entity:user/999999' => 'entity:user/999999',
];
+ // Add to array url with complex query parameters.
+ $valid_internal_entries += $this->getUrlWithComplexQueryInputList();
+
// Define some invalid URLs.
$validation_error_1 = "The path '@link_path' is invalid.";
$validation_error_2 = 'Manually entered paths should start with one of the following characters: / ? #';
@@ -462,7 +465,7 @@ protected function doTestLinkFormatter() {
// Not using generatePermutations(), since that leads to 32 cases, which
// would not test actual link field formatter functionality but rather
// the link generator and options/attributes. Only 'url_plain' has a
- // dependency on 'url_only', so we have a total of ~10 cases.
+ // dependency on 'url_only'.
$options = [
'trim_length' => [NULL, 6],
'rel' => [NULL, 'nofollow'],
@@ -545,6 +548,185 @@ protected function doTestLinkFormatter() {
}
}
+ /**
+ * Tests the default 'link' formatter with complex query parameters.
+ */
+ public function testLinkFormatterQueryParametersDuplication(): void {
+ $test_urls = $this->getUrlWithComplexQuery();
+ $field_name = $this->randomMachineName();
+ // Create a field with settings to validate.
+ $this->fieldStorage = FieldStorageConfig::create([
+ 'field_name' => $field_name,
+ 'entity_type' => 'entity_test',
+ 'type' => 'link',
+ 'cardinality' => count($test_urls),
+ ]);
+ $this->fieldStorage->save();
+ FieldConfig::create([
+ 'field_storage' => $this->fieldStorage,
+ 'label' => 'Read more about this entity',
+ 'bundle' => 'entity_test',
+ 'settings' => [
+ 'title' => DRUPAL_OPTIONAL,
+ 'link_type' => LinkItemInterface::LINK_GENERIC,
+ ],
+ ])->save();
+ /** @var \Drupal\Core\Entity\EntityDisplayRepositoryInterface $display_repository */
+ $display_repository = \Drupal::service('entity_display.repository');
+ $display_repository->getFormDisplay('entity_test', 'entity_test', 'default')
+ ->setComponent($field_name, [
+ 'type' => 'link_default',
+ ])
+ ->save();
+ $display_options = [
+ 'type' => 'link',
+ 'label' => 'hidden',
+ ];
+ $display_repository->getViewDisplay('entity_test', 'entity_test', 'full')
+ ->setComponent($field_name, $display_options)
+ ->save();
+
+ // Create an entity with link field values provided
+ // by $this->getUrlWithComplexQuery().
+ $entity = EntityTest::create();
+ $links = [];
+ // Prepare values for field.
+ foreach ($test_urls as $key => $test_url) {
+ $links[$key] = [
+ 'uri' => 'internal:' . $test_url['inputByUser'],
+ 'title' => $test_url['inputByUser'],
+ ];
+ }
+ $entity->{$field_name}->setValue($links);
+ $entity->save();
+
+ // Verify that the link is output according to the formatter settings.
+ // Not using generatePermutations(), since that leads to 32 cases, which
+ // would not test actual link field formatter functionality but rather
+ // the link generator and options/attributes. Only 'url_plain' has a
+ // dependency on 'url_only'.
+ $options = [
+ 'trim_length' => [NULL, 6],
+ 'rel' => [NULL, 'nofollow'],
+ 'target' => [NULL, '_blank'],
+ 'url_only' => [
+ ['url_only' => FALSE],
+ ['url_only' => FALSE, 'url_plain' => TRUE],
+ ['url_only' => TRUE],
+ ['url_only' => TRUE, 'url_plain' => TRUE],
+ ],
+ ];
+ foreach ($options as $setting => $values) {
+ foreach ($values as $new_value) {
+ // Update the field formatter settings.
+ if (!is_array($new_value)) {
+ $display_options['settings'] = [$setting => $new_value];
+ }
+ else {
+ $display_options['settings'] = $new_value;
+ }
+ $display_repository->getViewDisplay('entity_test', 'entity_test', 'full')
+ ->setComponent($field_name, $display_options)
+ ->save();
+
+ $output = $this->renderTestEntity($entity->id());
+ foreach ($test_urls as $test_url) {
+ $url = $test_url['renderedHref'];
+ $title = $test_url['inputByUser'];
+ switch ($setting) {
+ case 'trim_length':
+ $title = isset($new_value) ? Unicode::truncate($title, $new_value, FALSE, TRUE) : $title;
+ $this->assertStringContainsString('<a href="' . $url . '">' . Html::escape($title) . '</a>', $output);
+ break;
+
+ case 'rel':
+ $rel = isset($new_value) ? ' rel="' . $new_value . '"' : '';
+ $this->assertStringContainsString('<a href="' . $url . '"' . $rel . '>' . Html::escape($title) . '</a>', $output);
+ break;
+
+ case 'target':
+ $target = isset($new_value) ? ' target="' . $new_value . '"' : '';
+ $this->assertStringContainsString('<a href="' . $url . '"' . $target . '>' . Html::escape($title) . '</a>', $output);
+ break;
+
+ case 'url_only':
+ // In this case, $new_value is an array.
+ if (!$new_value['url_only']) {
+ $this->assertStringContainsString('<a href="' . $url . '">' . Html::escape($title) . '</a>', $output);
+ break;
+ }
+ if (empty($new_value['url_plain'])) {
+ $this->assertStringContainsString('<a href="' . $url . '">' . $url . '</a>', $output);
+ break;
+ }
+ $this->assertStringNotContainsString('<a href="' . $url . '">' . $url . '</a>', $output);
+ $this->assertStringContainsString($url, $output);
+ break;
+ }
+ }
+ }
+ }
+ }
+
+ /**
+ * Get array of url with complex query parameters for render check.
+ *
+ * @return array
+ * The URLs to test.
+ */
+ protected function getUrlWithComplexQuery(): array {
+ $test_urls = [
+ [
+ 'inputByUser' => '?a[]=1&a[]=2',
+ 'renderedHref' => '?a%5B0%5D=1&a%5B1%5D=2',
+ ],
+ [
+ 'inputByUser' => '?b[0]=1&b[1]=2',
+ 'renderedHref' => '?b%5B0%5D=1&b%5B1%5D=2',
+ ],
+ // UrlHelper::buildQuery will change order of params.
+ [
+ 'inputByUser' => '?c[]=1&d=3&c[]=2',
+ 'renderedHref' => '?c%5B0%5D=1&c%5B1%5D=2&d=3',
+ ],
+ [
+ 'inputByUser' => '?e[f][g]=h',
+ 'renderedHref' => '?e%5Bf%5D%5Bg%5D=h',
+ ],
+ [
+ 'inputByUser' => '?i[j[k]]=l',
+ 'renderedHref' => '?i%5Bj%5Bk%5D=l',
+ ],
+
+ // Query string replace value.
+ [
+ 'inputByUser' => '?x=1&x=2',
+ 'renderedHref' => '?x=2',
+ ],
+ [
+ 'inputByUser' => '?z[0]=1&z[0]=2',
+ 'renderedHref' => '?z%5B0%5D=2',
+ ],
+ ];
+ return $test_urls;
+ }
+
+ /**
+ * Get list of url with complex query parameters for input check.
+ *
+ * @return array
+ * The URLs with complex query parameters.
+ */
+ protected function getUrlWithComplexQueryInputList(): array {
+ $test_urls = $this->getUrlWithComplexQuery();
+ $list_urls = [];
+ foreach ($test_urls as $test_url) {
+ $list_urls[$test_url['inputByUser']] = Html::escape($test_url['inputByUser']);
+ }
+
+ return $list_urls;
+ }
+
/**
* Tests the 'link_separate' formatter.
*
diff --git a/core/modules/link/tests/src/Unit/LinkFormatterTest.php b/core/modules/link/tests/src/Unit/LinkFormatterTest.php
index 600a1aa603ee..c86e1d9bf7a8 100644
--- a/core/modules/link/tests/src/Unit/LinkFormatterTest.php
+++ b/core/modules/link/tests/src/Unit/LinkFormatterTest.php
@@ -147,7 +147,6 @@ public function testFormatterLinkItem() {
[
'#type' => 'link',
'#title' => 'http://example.com',
- '#options' => [],
'#url' => $expectedUrl,
],
], $elements);
--
GitLab