From 6d62cf239e1983a9fe47d210dbf34fdb2796a66c Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Thu, 26 Jan 2023 09:17:16 +0000
Subject: [PATCH] Issue #3303067 by catch, nod_, Wim Leers, olli, alexpott:
Compress aggregate URL query strings
---
.../Drupal/Component/Utility/UrlHelper.php | 43 +++++++++++
.../Core/Asset/CssCollectionOptimizerLazy.php | 16 +++--
.../Core/Asset/JsCollectionOptimizerLazy.php | 17 +++--
.../src/Controller/AssetControllerBase.php | 18 ++++-
.../Asset/AssetOptimizationTest.php | 71 ++++++++++++++++++-
.../Tests/Component/Utility/UrlHelperTest.php | 24 +++++++
6 files changed, 171 insertions(+), 18 deletions(-)
diff --git a/core/lib/Drupal/Component/Utility/UrlHelper.php b/core/lib/Drupal/Component/Utility/UrlHelper.php
index 79e275bdc05f..ef118716d0b8 100644
--- a/core/lib/Drupal/Component/Utility/UrlHelper.php
+++ b/core/lib/Drupal/Component/Utility/UrlHelper.php
@@ -62,6 +62,49 @@ public static function buildQuery(array $query, $parent = '') {
return implode('&', $params);
}
+ /**
+ * Compresses a string for use in a query parameter.
+ *
+ * While RFC 1738 doesn't specify a maximum length for query strings,
+ * browsers or server configurations may restrict URLs and/or query strings to
+ * a certain length, often 1000 or 2000 characters. This method can be used to
+ * compress a string into a URL-safe query parameter which will be shorter
+ * than if it was used directly.
+ *
+ * @see \Drupal\Component\Utility\UrlHelper::uncompressQueryParameter()
+ *
+ * @param string $data
+ * The data to compress.
+ *
+ * @return string
+ * The data compressed into a URL-safe string.
+ */
+ public static function compressQueryParameter(string $data): string {
+ // Use 'base64url' encoding. Note that the '=' sign is only used for padding
+ // on the right of the string, and is otherwise not part of the data.
+ // @see https://datatracker.ietf.org/doc/html/rfc4648#section-5
+ // @see https://www.php.net/manual/en/function.base64-encode.php#123098
+ return str_replace(['+', '/', '='], ['-', '_', ''], base64_encode(gzcompress($data)));
+ }
+
+ /**
+ * Takes a compressed parameter and converts it back to the original.
+ *
+ * @see \Drupal\Component\Utility\UrlHelper::compressQueryParameter()
+ *
+ * @param string $compressed
+ * A string as compressed by
+ * \Drupal\Component\Utility\UrlHelper::compressQueryParameter().
+ *
+ * @return string|bool
+ * The uncompressed data or FALSE on failure.
+ */
+ public static function uncompressQueryParameter(string $compressed): string|bool {
+ // Because this comes from user data, suppress the PHP warning that
+ // gzcompress() throws if the base64-encoded string is invalid.
+ return @gzuncompress(base64_decode(str_replace(['-', '_'], ['+', '/'], $compressed)));
+ }
+
/**
* Filters a URL query parameter array to remove unwanted elements.
*
diff --git a/core/lib/Drupal/Core/Asset/CssCollectionOptimizerLazy.php b/core/lib/Drupal/Core/Asset/CssCollectionOptimizerLazy.php
index a54c33cb9272..34bfaad71c7f 100644
--- a/core/lib/Drupal/Core/Asset/CssCollectionOptimizerLazy.php
+++ b/core/lib/Drupal/Core/Asset/CssCollectionOptimizerLazy.php
@@ -102,18 +102,22 @@ public function optimize(array $css_assets, array $libraries) {
$css_assets[$order]['data'] = $uri;
}
}
- // Generate a URL for each group of assets, but do not process them inline,
- // this is done using optimizeGroup() when the asset path is requested.
- $ajax_page_state = $this->requestStack->getCurrentRequest()->get('ajax_page_state');
- $already_loaded = isset($ajax_page_state) ? explode(',', $ajax_page_state['libraries']) : [];
+
+ // All asset group URLs will have exactly the same query arguments, except
+ // for the delta, so prepare them in advance.
$query_args = [
'language' => $this->languageManager->getCurrentLanguage()->getId(),
'theme' => $this->themeManager->getActiveTheme()->getName(),
- 'include' => implode(',', $this->dependencyResolver->getMinimalRepresentativeSubset($libraries)),
+ 'include' => UrlHelper::compressQueryParameter(implode(',', $this->dependencyResolver->getMinimalRepresentativeSubset($libraries))),
];
+ $ajax_page_state = $this->requestStack->getCurrentRequest()->get('ajax_page_state');
+ $already_loaded = isset($ajax_page_state) ? explode(',', $ajax_page_state['libraries']) : [];
if ($already_loaded) {
- $query_args['exclude'] = implode(',', $this->dependencyResolver->getMinimalRepresentativeSubset($already_loaded));
+ $query_args['exclude'] = UrlHelper::compressQueryParameter(implode(',', $this->dependencyResolver->getMinimalRepresentativeSubset($already_loaded)));
}
+
+ // Generate a URL for each group of assets, but do not process them inline,
+ // this is done using optimizeGroup() when the asset path is requested.
foreach ($css_assets as $order => $css_asset) {
if (!empty($css_asset['preprocessed'])) {
$query = ['delta' => "$order"] + $query_args;
diff --git a/core/lib/Drupal/Core/Asset/JsCollectionOptimizerLazy.php b/core/lib/Drupal/Core/Asset/JsCollectionOptimizerLazy.php
index 688459a69b03..e2b315203563 100644
--- a/core/lib/Drupal/Core/Asset/JsCollectionOptimizerLazy.php
+++ b/core/lib/Drupal/Core/Asset/JsCollectionOptimizerLazy.php
@@ -110,20 +110,23 @@ public function optimize(array $js_assets, array $libraries) {
}
}
if ($libraries) {
- // Generate a URL for the group, but do not process it inline, this is
- // done by \Drupal\system\controller\JsAssetController.
- $ajax_page_state = $this->requestStack->getCurrentRequest()
- ->get('ajax_page_state');
- $already_loaded = isset($ajax_page_state) ? explode(',', $ajax_page_state['libraries']) : [];
+ // All group URLs have the same query arguments apart from the delta and
+ // scope, so prepare them in advance.
$language = $this->languageManager->getCurrentLanguage()->getId();
$query_args = [
'language' => $language,
'theme' => $this->themeManager->getActiveTheme()->getName(),
- 'include' => implode(',', $this->dependencyResolver->getMinimalRepresentativeSubset($libraries)),
+ 'include' => UrlHelper::compressQueryParameter(implode(',', $this->dependencyResolver->getMinimalRepresentativeSubset($libraries))),
];
+ $ajax_page_state = $this->requestStack->getCurrentRequest()
+ ->get('ajax_page_state');
+ $already_loaded = isset($ajax_page_state) ? explode(',', $ajax_page_state['libraries']) : [];
if ($already_loaded) {
- $query_args['exclude'] = implode(',', $this->dependencyResolver->getMinimalRepresentativeSubset($already_loaded));
+ $query_args['exclude'] = UrlHelper::compressQueryParameter(implode(',', $this->dependencyResolver->getMinimalRepresentativeSubset($already_loaded)));
}
+
+ // Generate a URL for the group, but do not process it inline, this is
+ // done by \Drupal\system\controller\JsAssetController.
foreach ($js_assets as $order => $js_asset) {
if (!empty($js_asset['preprocessed'])) {
$query = [
diff --git a/core/modules/system/src/Controller/AssetControllerBase.php b/core/modules/system/src/Controller/AssetControllerBase.php
index cb83318b3917..023bc031f74a 100644
--- a/core/modules/system/src/Controller/AssetControllerBase.php
+++ b/core/modules/system/src/Controller/AssetControllerBase.php
@@ -2,6 +2,7 @@
namespace Drupal\system\Controller;
+use Drupal\Component\Utility\UrlHelper;
use Drupal\Core\Asset\AssetCollectionGrouperInterface;
use Drupal\Core\Asset\AssetCollectionOptimizerInterface;
use Drupal\Core\Asset\AssetDumperUriInterface;
@@ -131,6 +132,9 @@ public function deliver(Request $request, string $file_name) {
if (!$request->query->has('language')) {
throw new BadRequestHttpException('The language must be passed as a query argument');
}
+ if (!$request->query->has('include')) {
+ throw new BadRequestHttpException('The libraries to include must be passed as a query argument');
+ }
$file_parts = explode('_', basename($file_name, '.' . $this->fileExtension), 2);
// The hash is the second segment of the filename.
@@ -147,9 +151,19 @@ public function deliver(Request $request, string $file_name) {
$this->themeManager->setActiveTheme($active_theme);
$attached_assets = new AttachedAssets();
- $attached_assets->setLibraries(explode(',', $request->query->get('include')));
+ $include_string = UrlHelper::uncompressQueryParameter($request->query->get('include'));
+
+ if (!$include_string) {
+ throw new BadRequestHttpException('The libraries to include are encoded incorrectly.');
+ }
+ $attached_assets->setLibraries(explode(',', $include_string));
+
if ($request->query->has('exclude')) {
- $attached_assets->setAlreadyLoadedLibraries(explode(',', $request->query->get('exclude')));
+ $exclude_string = UrlHelper::uncompressQueryParameter($request->query->get('exclude'));
+ if (!$exclude_string) {
+ throw new BadRequestHttpException('The libraries to exclude are encoded incorrectly.');
+ }
+ $attached_assets->setAlreadyLoadedLibraries(explode(',', $exclude_string));
}
$groups = $this->getGroups($attached_assets, $request);
diff --git a/core/tests/Drupal/FunctionalTests/Asset/AssetOptimizationTest.php b/core/tests/Drupal/FunctionalTests/Asset/AssetOptimizationTest.php
index c7b71cd84dcd..c59daabe74ad 100644
--- a/core/tests/Drupal/FunctionalTests/Asset/AssetOptimizationTest.php
+++ b/core/tests/Drupal/FunctionalTests/Asset/AssetOptimizationTest.php
@@ -116,6 +116,15 @@ protected function assertInvalidAggregates(string $url): void {
$session->visit($this->omitTheme($url));
$this->assertSession()->statusCodeEquals(400);
+ $session->visit($this->omitInclude($url));
+ $this->assertSession()->statusCodeEquals(400);
+
+ $session->visit($this->invalidInclude($url));
+ $this->assertSession()->statusCodeEquals(400);
+
+ $session->visit($this->invalidExclude($url));
+ $this->assertSession()->statusCodeEquals(400);
+
$session->visit($this->setInvalidLibrary($url));
$this->assertSession()->statusCodeEquals(200);
@@ -164,19 +173,21 @@ protected function replaceGroupHash(string $url): string {
}
/**
- * Replaces the 'libraries' entry in the given URL with an invalid value.
+ * Replaces the 'include' entry in the given URL with an invalid value.
*
* @param string $url
* The source URL.
*
* @return string
- * The URL with the 'library' query set to an invalid value.
+ * The URL with the 'include' query set to an invalid value.
*/
protected function setInvalidLibrary(string $url): string {
// First replace the hash, so we don't get served the actual file on disk.
$url = $this->replaceGroupHash($url);
$parts = UrlHelper::parse($url);
- $parts['query']['libraries'] = ['system/llama'];
+ $include = explode(',', UrlHelper::uncompressQueryParameter($parts['query']['include']));
+ $include[] = 'system/llama';
+ $parts['query']['include'] = UrlHelper::compressQueryParameter(implode(',', $include));
$query = UrlHelper::buildQuery($parts['query']);
return $this->getAbsoluteUrl($parts['path'] . '?' . $query . '#' . $parts['fragment']);
@@ -200,4 +211,58 @@ protected function omitTheme(string $url): string {
return $this->getAbsoluteUrl($parts['path'] . '?' . $query . '#' . $parts['fragment']);
}
+ /**
+ * Removes the 'include' query parameter from the given URL.
+ *
+ * @param string $url
+ * The source URL.
+ *
+ * @return string
+ * The URL with the 'include' parameter omitted.
+ */
+ protected function omitInclude(string $url): string {
+ // First replace the hash, so we don't get served the actual file on disk.
+ $url = $this->replaceGroupHash($url);
+ $parts = UrlHelper::parse($url);
+ unset($parts['query']['include']);
+ $query = UrlHelper::buildQuery($parts['query']);
+ return $this->getAbsoluteUrl($parts['path'] . '?' . $query . '#' . $parts['fragment']);
+ }
+
+ /**
+ * Replaces the 'include' query parameter with an invalid value.
+ *
+ * @param string $url
+ * The source URL.
+ *
+ * @return string
+ * The URL with 'include' set to an arbitrary string.
+ */
+ protected function invalidInclude(string $url): string {
+ // First replace the hash, so we don't get served the actual file on disk.
+ $url = $this->replaceGroupHash($url);
+ $parts = UrlHelper::parse($url);
+ $parts['query']['include'] = 'abcdefghijklmnop';
+ $query = UrlHelper::buildQuery($parts['query']);
+ return $this->getAbsoluteUrl($parts['path'] . '?' . $query . '#' . $parts['fragment']);
+ }
+
+ /**
+ * Adds an invalid 'exclude' query parameter with an invalid value.
+ *
+ * @param string $url
+ * The source URL.
+ *
+ * @return string
+ * The URL with 'exclude' set to an arbitrary string.
+ */
+ protected function invalidExclude(string $url): string {
+ // First replace the hash, so we don't get served the actual file on disk.
+ $url = $this->replaceGroupHash($url);
+ $parts = UrlHelper::parse($url);
+ $parts['query']['exclude'] = 'abcdefghijklmnop';
+ $query = UrlHelper::buildQuery($parts['query']);
+ return $this->getAbsoluteUrl($parts['path'] . '?' . $query . '#' . $parts['fragment']);
+ }
+
}
diff --git a/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php b/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php
index c4d76e4604f5..a69954592e25 100644
--- a/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php
+++ b/core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php
@@ -106,6 +106,30 @@ public function providerTestInvalidAbsolute() {
return $this->dataEnhanceWithScheme($data);
}
+ /**
+ * Tests that we get the same thing out that we put in.
+ */
+ public function testCompressUncompress() {
+ $data = [];
+ while (count($data) < 30) {
+ $data[] = 'drupal/drupal' . count($data);
+ }
+ $data = implode(',', $data);
+ $compressed = UrlHelper::compressQueryParameter($data);
+ $uncompressed = UrlHelper::uncompressQueryParameter($compressed);
+ $this->assertEquals($data, $uncompressed);
+ $this->assertLessThan(strlen($uncompressed), strlen($compressed));
+ }
+
+ /**
+ * Tests passing an invalid string as a compressed query parameter.
+ */
+ public function testUncompressInvalidString() {
+ // Pass an invalid string to ::uncompressQueryParameter() and ensure it
+ // doesn't result in a PHP warning.
+ $this->assertFalse(UrlHelper::uncompressQueryParameter('llama'));
+ }
+
/**
* Tests invalid absolute URLs.
*
--
GitLab