mirrors/shlink
1
0
Fork 0
mirror of https://github.com/shlinkio/shlink.git synced 2024-11-28 09:03:07 +03:00
Code Issues Projects Releases Packages Wiki Activity

Ensure query parameters are preserved verbatim when forwarded to long URL

Browse source
This commit is contained in:
Alejandro Celaya 2024-10-10 11:28:31 +02:00
parent a8e4b2fceb
commit 1773e6ecae
5 changed files with 50 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
CHANGELOG.md
View file

@ -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

2
config/autoload/rabbit.local.php.dist
View file

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

6
module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php
View file

@ -28,11 +28,15 @@ readonly class ShortUrlRedirectionBuilder implements ShortUrlRedirectionBuilderI
?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))

18
module/Core/test-api/Action/RedirectTest.php
View file

@ -122,4 +122,22 @@ class RedirectTest extends ApiTestCase
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'));
}
}

41
module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php
View file

@ -5,6 +5,7 @@ declare(strict_types=1);
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 @@ class ShortUrlRedirectionBuilderTest extends TestCase
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,
];
}
/**