Ensure query parameters are preserved verbatim when forwarded to long…

… URL

CHANGELOG.md

@@ -18,7 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 * *Nothing*
 
 ### Fixed
-* *Nothing*
+* [#2213](https://github.com/shlinkio/shlink/issues/2213) Fix spaces being replaced with underscores in query parameter names, when forwarded from short URL to long URL.
 
 
 ## [4.2.1] - 2024-10-04

config/autoload/rabbit.local.php.dist

@@ -7,7 +7,7 @@ return [
  'rabbitmq' => [
  'enabled' => true,
  'host' => 'shlink_rabbitmq',
- 'port' => '5672',
+ 'port' => 5672,
  'user' => 'rabbit',
  'password' => 'rabbit',
  ],

module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php

@@ -28,11 +28,15 @@ public function buildShortUrlRedirect(
  ?string $extraPath = null,
  ): string {
  $uri = new Uri($this->redirectionResolver->resolveLongUrl($shortUrl, $request));
- $currentQuery = $request->getQueryParams();
  $shouldForwardQuery = $shortUrl->forwardQuery();
  $baseQueryString = $uri->getQuery();
  $basePath = $uri->getPath();
 
+ // Get current query by manually parsing query string, instead of using $request->getQueryParams().
+ // That prevents some weird PHP logic in which some characters in param names are converted to ensure resulting
+ // names are valid variable names.
+ $currentQuery = Query::parse($request->getUri()->getQuery());
+
  return $uri
  ->withQuery($shouldForwardQuery ? $this->resolveQuery($baseQueryString, $currentQuery) : $baseQueryString)
  ->withPath($this->resolvePath($basePath, $extraPath))

module/Core/test-api/Action/RedirectTest.php

@@ -122,4 +122,22 @@ public function properRedirectHappensForNonHttpLongUrls(string $longUrl): void
  self::assertEquals(302, $response->getStatusCode());
  self::assertEquals($longUrl, $response->getHeaderLine('Location'));
  }
+
+ #[Test]
+ public function queryParametersAreProperlyForwarded(): void
+ {
+ $slug = 'forward-query-params';
+ $this->callApiWithKey('POST', '/short-urls', [
+ RequestOptions::JSON => [
+ 'longUrl' => 'https://example.com',
+ 'customSlug' => $slug,
+ 'forwardQuery' => true,
+ ],
+ ]);
+
+ $response = $this->callShortUrl($slug, [RequestOptions::QUERY => ['foo bar' => '123']]);
+
+ self::assertEquals(302, $response->getStatusCode());
+ self::assertEquals('https://example.com?foo%20bar=123', $response->getHeaderLine('Location'));
+ }
 }

module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php

@@ -5,6 +5,7 @@
 namespace ShlinkioTest\Shlink\Core\ShortUrl\Helper;
 
 use Laminas\Diactoros\ServerRequestFactory;
+use Laminas\Diactoros\Uri;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\Attributes\TestWith;
@@ -53,62 +54,70 @@ public function buildShortUrlRedirectBuildsExpectedUrl(
 
  public static function provideData(): iterable
  {
- $request = static fn (array $query = []) => ServerRequestFactory::fromGlobals()->withQueryParams($query);
+ $request = static fn (string $query = '') => ServerRequestFactory::fromGlobals()->withUri(
+ (new Uri())->withQuery($query),
+ );
 
  yield ['https://example.com/foo/bar?some=thing', $request(), null, true];
  yield ['https://example.com/foo/bar?some=thing', $request(), null, null];
  yield ['https://example.com/foo/bar?some=thing', $request(), null, false];
- yield ['https://example.com/foo/bar?some=thing&else', $request(['else' => null]), null, true];
- yield ['https://example.com/foo/bar?some=thing&foo=bar', $request(['foo' => 'bar']), null, true];
- yield ['https://example.com/foo/bar?some=thing&foo=bar', $request(['foo' => 'bar']), null, null];
- yield ['https://example.com/foo/bar?some=thing', $request(['foo' => 'bar']), null, false];
- yield ['https://example.com/foo/bar?some=thing&123=foo', $request(['123' => 'foo']), null, true];
- yield ['https://example.com/foo/bar?some=thing&456=foo', $request([456 => 'foo']), null, true];
- yield ['https://example.com/foo/bar?some=thing&456=foo', $request([456 => 'foo']), null, null];
- yield ['https://example.com/foo/bar?some=thing', $request([456 => 'foo']), null, false];
+ yield ['https://example.com/foo/bar?some=thing&else', $request('else'), null, true];
+ yield ['https://example.com/foo/bar?some=thing&foo=bar', $request('foo=bar'), null, true];
+ yield ['https://example.com/foo/bar?some=thing&foo=bar', $request('foo=bar'), null, null];
+ yield ['https://example.com/foo/bar?some=thing', $request('foo=bar'), null, false];
+ yield ['https://example.com/foo/bar?some=thing&123=foo', $request('123=foo'), null, true];
+ yield ['https://example.com/foo/bar?some=thing&456=foo', $request('456=foo'), null, true];
+ yield ['https://example.com/foo/bar?some=thing&456=foo', $request('456=foo'), null, null];
+ yield ['https://example.com/foo/bar?some=thing', $request('456=foo'), null, false];
  yield [
  'https://example.com/foo/bar?some=overwritten&foo=bar',
- $request(['foo' => 'bar', 'some' => 'overwritten']),
+ $request('foo=bar&some=overwritten'),
  null,
  true,
  ];
  yield [
  'https://example.com/foo/bar?some=overwritten',
- $request(['foobar' => 'notrack', 'some' => 'overwritten']),
+ $request('foobar=notrack&some=overwritten'),
  null,
  true,
  ];
  yield [
  'https://example.com/foo/bar?some=overwritten',
- $request(['foobar' => 'notrack', 'some' => 'overwritten']),
+ $request('foobar=notrack&some=overwritten'),
  null,
  null,
  ];
  yield [
  'https://example.com/foo/bar?some=thing',
- $request(['foobar' => 'notrack', 'some' => 'overwritten']),
+ $request('foobar=notrack&some=overwritten'),
  null,
  false,
  ];
  yield ['https://example.com/foo/bar/something/else-baz?some=thing', $request(), '/something/else-baz', true];
  yield [
  'https://example.com/foo/bar/something/else-baz?some=thing&hello=world',
- $request(['hello' => 'world']),
+ $request('hello=world',),
  '/something/else-baz',
  true,
  ];
  yield [
  'https://example.com/foo/bar/something/else-baz?some=thing&hello=world',
- $request(['hello' => 'world']),
+ $request('hello=world',),
  '/something/else-baz',
  null,
  ];
  yield [
  'https://example.com/foo/bar/something/else-baz?some=thing',
- $request(['hello' => 'world']),
+ $request('hello=world',),
  '/something/else-baz',
  false,
  ];
+ yield [
+ 'https://example.com/foo/bar/something/else-baz?some=thing&parameter%20with%20spaces=world',
+ $request('parameter with spaces=world',),
+ '/something/else-baz',
+ true,
+ ];
  }
 
  /**