From 1e15ed9d4fe5ff278867f7425dbd3328a3b52301 Mon Sep 17 00:00:00 2001
From: Lee Rowlands <lee.rowlands@previousnext.com.au>
Date: Tue, 18 Aug 2020 08:10:25 +1000
Subject: [PATCH] Issue #3164686 by mondrake, longwave, ridhimaabrol24:
 WebAssert::addressEquals() and AssertLegacyTrait::assertUrl() fail to check
 the querystring

---
 .../tests/src/Functional/BlockUiTest.php      | 11 +++----
 .../Functional/ManageFieldsFunctionalTest.php |  2 +-
 .../tests/src/Functional/ForumIndexTest.php   |  2 +-
 .../src/Functional/LanguageSwitchingTest.php  |  2 +-
 .../src/Functional/Form/ConfirmFormTest.php   |  3 +-
 .../src/Functional/Form/RedirectTest.php      |  8 +++--
 .../Ajax/AjaxFormCacheTest.php                |  4 ---
 .../Drupal/FunctionalTests/WebAssertTest.php  | 29 +++++++++++++++++++
 core/tests/Drupal/Tests/WebAssert.php         |  8 ++++-
 9 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/core/modules/block/tests/src/Functional/BlockUiTest.php b/core/modules/block/tests/src/Functional/BlockUiTest.php
index 8e771747cf63..f9915c64e092 100644
--- a/core/modules/block/tests/src/Functional/BlockUiTest.php
+++ b/core/modules/block/tests/src/Functional/BlockUiTest.php
@@ -7,6 +7,8 @@
 use Drupal\language\Plugin\LanguageNegotiation\LanguageNegotiationUrl;
 use Drupal\Tests\BrowserTestBase;
 
+// cspell:ignore scriptalertxsssubjectscript
+
 /**
  * Tests that the block configuration UI exists and stores data correctly.
  *
@@ -333,13 +335,12 @@ public function testBlockPlacementIndicator() {
     // block placement indicator. Click the first 'Place block' link to bring up
     // the list of blocks to place in the first available region.
     $this->clickLink('Place block');
-    // Select the first available block.
+    // Select the first available block, which is the 'test_xss_title' plugin,
+    // with a default machine name 'scriptalertxsssubjectscript' that is used
+    // for the 'block-placement' querystring parameter.
     $this->clickLink('Place block');
-    $block = [];
-    $block['id'] = strtolower($this->randomMachineName());
-    $block['theme'] = 'classy';
     $this->submitForm([], 'Save block');
-    $this->assertSession()->addressEquals('admin/structure/block/list/classy?block-placement=' . Html::getClass($block['id']));
+    $this->assertSession()->addressEquals('admin/structure/block/list/classy?block-placement=scriptalertxsssubjectscript');
 
     // Removing a block will remove the block placement indicator.
     $this->clickLink('Remove');
diff --git a/core/modules/field_ui/tests/src/Functional/ManageFieldsFunctionalTest.php b/core/modules/field_ui/tests/src/Functional/ManageFieldsFunctionalTest.php
index a3a0ae48b203..fa1c89b1098f 100644
--- a/core/modules/field_ui/tests/src/Functional/ManageFieldsFunctionalTest.php
+++ b/core/modules/field_ui/tests/src/Functional/ManageFieldsFunctionalTest.php
@@ -682,7 +682,7 @@ public function testExternalDestinations() {
     ];
     $this->drupalPostForm('admin/structure/types/manage/article/fields/node.article.body/storage', [], 'Save field settings', $options);
     // The external redirect should not fire.
-    $this->assertUrl('admin/structure/types/manage/article/fields/node.article.body/storage', $options);
+    $this->assertUrl('admin/structure/types/manage/article/fields/node.article.body/storage?destinations%5B0%5D=http%3A//example.com');
     $this->assertSession()->statusCodeEquals(200);
     $this->assertRaw('Attempt to update field <em class="placeholder">Body</em> failed: <em class="placeholder">The internal path component &#039;http://example.com&#039; is external. You are not allowed to specify an external URL together with internal:/.</em>.');
   }
diff --git a/core/modules/forum/tests/src/Functional/ForumIndexTest.php b/core/modules/forum/tests/src/Functional/ForumIndexTest.php
index 4ee133435998..76f1cdd6fd9b 100644
--- a/core/modules/forum/tests/src/Functional/ForumIndexTest.php
+++ b/core/modules/forum/tests/src/Functional/ForumIndexTest.php
@@ -54,7 +54,7 @@ public function testForumIndexStatus() {
     // Create the forum topic, preselecting the forum ID via a URL parameter.
     $this->drupalGet("forum/$tid");
     $this->clickLink(t('Add new @node_type', ['@node_type' => 'Forum topic']));
-    $this->assertUrl('node/add/forum', ['query' => ['forum_id' => $tid]]);
+    $this->assertUrl("node/add/forum?forum_id=$tid");
     $this->drupalPostForm(NULL, $edit, t('Save'));
 
     // Check that the node exists in the database.
diff --git a/core/modules/language/tests/src/Functional/LanguageSwitchingTest.php b/core/modules/language/tests/src/Functional/LanguageSwitchingTest.php
index a45af25d06be..25848e7882d0 100644
--- a/core/modules/language/tests/src/Functional/LanguageSwitchingTest.php
+++ b/core/modules/language/tests/src/Functional/LanguageSwitchingTest.php
@@ -450,7 +450,7 @@ public function testLanguageSessionSwitchLinks() {
     // Click on the French link.
     $this->clickLink(t('French'));
     // There should be a query parameter to set the session language.
-    $this->assertUrl('user/2', ['query' => ['language' => 'fr']]);
+    $this->assertUrl('user/2?language=fr');
     // Click on the 'Home' Link.
     $this->clickLink(t('Home'));
     // There should be no query parameter.
diff --git a/core/modules/system/tests/src/Functional/Form/ConfirmFormTest.php b/core/modules/system/tests/src/Functional/Form/ConfirmFormTest.php
index d28ccb6b206a..2a0e059098ed 100644
--- a/core/modules/system/tests/src/Functional/Form/ConfirmFormTest.php
+++ b/core/modules/system/tests/src/Functional/Form/ConfirmFormTest.php
@@ -49,7 +49,8 @@ public function testConfirmForm() {
     // Test cancelling the form with a complex destination.
     $this->drupalGet('form-test/confirm-form-array-path');
     $this->clickLink(t('ConfirmFormArrayPathTestForm::getCancelText().'));
-    $this->assertUrl('form-test/confirm-form', ['query' => ['destination' => 'admin/config']], "The form's complex cancel link was followed.");
+    // Verify that the form's complex cancel link was followed.
+    $this->assertUrl('form-test/confirm-form?destination=admin/config');
   }
 
   /**
diff --git a/core/modules/system/tests/src/Functional/Form/RedirectTest.php b/core/modules/system/tests/src/Functional/Form/RedirectTest.php
index a87e1acf6c70..f875eedc8008 100644
--- a/core/modules/system/tests/src/Functional/Form/RedirectTest.php
+++ b/core/modules/system/tests/src/Functional/Form/RedirectTest.php
@@ -60,7 +60,9 @@ public function testRedirect() {
       'redirection' => FALSE,
     ];
     $this->drupalPostForm($path, $edit, t('Submit'), $options);
-    $this->assertUrl($path, $options, 'When redirect is set to FALSE, there should be no redirection, and the query parameters should be passed along.');
+    // When redirect is set to FALSE, there should be no redirection, and the
+    // query parameters should be passed along.
+    $this->assertUrl($path . '?foo=bar');
 
     // Test redirection back to the original path.
     $edit = [
@@ -76,7 +78,9 @@ public function testRedirect() {
       'destination' => '',
     ];
     $this->drupalPostForm($path, $edit, t('Submit'), $options);
-    $this->assertUrl($path, $options, 'When using an empty redirection string, there should be no redirection, and the query parameters should be passed along.');
+    // When using an empty redirection string, there should be no redirection,
+    // and the query parameters should be passed along.
+    $this->assertUrl($path . '?foo=bar');
   }
 
   /**
diff --git a/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxFormCacheTest.php b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxFormCacheTest.php
index b2f6b6793887..92e22ca0ab00 100644
--- a/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxFormCacheTest.php
+++ b/core/tests/Drupal/FunctionalJavascriptTests/Ajax/AjaxFormCacheTest.php
@@ -2,8 +2,6 @@
 
 namespace Drupal\FunctionalJavascriptTests\Ajax;
 
-use Drupal\Core\EventSubscriber\MainContentViewSubscriber;
-use Drupal\Core\Form\FormBuilderInterface;
 use Drupal\Core\Url;
 use Drupal\FunctionalJavascriptTests\WebDriverTestBase;
 
@@ -103,8 +101,6 @@ public function testQueryString() {
 
     $url->setOption('query', [
       'foo' => 'bar',
-      FormBuilderInterface::AJAX_FORM_REQUEST => 1,
-      MainContentViewSubscriber::WRAPPER_FORMAT => 'drupal_ajax',
     ]);
     $this->assertUrl($url);
   }
diff --git a/core/tests/Drupal/FunctionalTests/WebAssertTest.php b/core/tests/Drupal/FunctionalTests/WebAssertTest.php
index b829eee898af..8a662aa74af2 100644
--- a/core/tests/Drupal/FunctionalTests/WebAssertTest.php
+++ b/core/tests/Drupal/FunctionalTests/WebAssertTest.php
@@ -3,6 +3,7 @@
 namespace Drupal\FunctionalTests;
 
 use Drupal\Tests\BrowserTestBase;
+use Behat\Mink\Exception\ExpectationException;
 use PHPUnit\Framework\AssertionFailedError;
 
 /**
@@ -18,6 +19,7 @@ class WebAssertTest extends BrowserTestBase {
    */
   protected static $modules = [
     'test_page_test',
+    'dblog',
   ];
 
   /**
@@ -53,4 +55,31 @@ public function testResponseHeaderDoesNotExist() {
     $this->assertSession()->responseHeaderDoesNotExist('Null-Header');
   }
 
+  /**
+   * Tests that addressEquals distinguishes querystrings.
+   *
+   * @covers ::addressEquals
+   */
+  public function testAddressEqualsDistinguishesQuerystrings() {
+    // Insert 300 log messages.
+    $logger = $this->container->get('logger.factory')->get('pager_test');
+    for ($i = 0; $i < 300; $i++) {
+      $logger->debug($this->randomString());
+    }
+
+    // Get to the db log report.
+    $this->drupalLogin($this->drupalCreateUser([
+      'access site reports',
+    ]));
+    $this->drupalGet('admin/reports/dblog');
+    $this->assertSession()->addressEquals('admin/reports/dblog');
+
+    // Go to the second page, we expect the querystring to change to '?page=1'.
+    $this->drupalGet('admin/reports/dblog', ['query' => ['page' => 1]]);
+    $this->assertSession()->addressEquals('admin/reports/dblog?page=1');
+    $this->expectException(ExpectationException::class);
+    $this->expectExceptionMessage('Current page is "/admin/reports/dblog?page=1", but "/admin/reports/dblog" expected.');
+    $this->assertSession()->addressEquals('admin/reports/dblog');
+  }
+
 }
diff --git a/core/tests/Drupal/Tests/WebAssert.php b/core/tests/Drupal/Tests/WebAssert.php
index 16324fa12155..5b809a61ae1f 100644
--- a/core/tests/Drupal/Tests/WebAssert.php
+++ b/core/tests/Drupal/Tests/WebAssert.php
@@ -55,7 +55,13 @@ protected function cleanUrl($url) {
     if (parse_url($url, PHP_URL_HOST) === NULL && strpos($url, '/') !== 0) {
       $url = "/$url";
     }
-    return parent::cleanUrl($url);
+
+    $parts = parse_url($url);
+    $fragment = empty($parts['fragment']) ? '' : '#' . $parts['fragment'];
+    $query = empty($parts['query']) ? '' : '?' . $parts['query'];
+    $path = empty($parts['path']) ? '/' : $parts['path'];
+
+    return preg_replace('/^\/[^\.\/]+\.php\//', '/', $path) . $query . $fragment;
   }
 
   /**
-- 
GitLab