Issue #2885351 by Nikolay Shapovalov, paulocs, clemens.tolboom, larowlan, dcam, mrshowerman, smustgrave, alexpott, borisson_: Query string duplications

core/modules/link/src/Plugin/Field/FieldFormatter/LinkFormatter.php

@@ -205,16 +205,15 @@ class LinkFormatter extends FormatterBase {
         }
       }
       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]['#attributes'] = $item->_attributes;
-          $element[$delta]['#options']['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);

core/modules/link/tests/src/Functional/LinkFieldTest.php

@@ -179,6 +179,9 @@ class LinkFieldTest extends BrowserTestBase {
       '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 @@ class LinkFieldTest extends BrowserTestBase {
     // 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 @@ class LinkFieldTest extends BrowserTestBase {
     }
   }
 
+  /**
+   * 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&amp;a%5B1%5D=2',
+      ],
+      [
+        'inputByUser' => '?b[0]=1&b[1]=2',
+        'renderedHref' => '?b%5B0%5D=1&amp;b%5B1%5D=2',
+      ],
+      // UrlHelper::buildQuery will change order of params.
+      [
+        'inputByUser' => '?c[]=1&d=3&c[]=2',
+        'renderedHref' => '?c%5B0%5D=1&amp;c%5B1%5D=2&amp;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.
    *

core/modules/link/tests/src/Unit/LinkFormatterTest.php

@@ -147,7 +147,6 @@ class LinkFormatterTest extends UnitTestCase {
       [
         '#type' => 'link',
         '#title' => 'http://example.com',
-        '#options' => [],
         '#url' => $expectedUrl,
       ],
     ], $elements);