Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/remove league uri #1991

Merged
merged 2 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this

### Changed
* [#1935](https://github.com/shlinkio/shlink/issues/1935) Replace dependency on abandoned `php-middleware/request-id` with userland simple middleware.
* [#1988](https://github.com/shlinkio/shlink/issues/1988) Remove dependency on `league\uri` package.

### Deprecated
* *Nothing*
Expand Down
1 change: 0 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
"laminas/laminas-inputfilter": "^2.27",
"laminas/laminas-servicemanager": "^3.21",
"laminas/laminas-stdlib": "^3.17",
"league/uri": "^6.8",
"matomo/matomo-php-tracker": "^3.2",
"mezzio/mezzio": "^3.17",
"mezzio/mezzio-fastroute": "^3.11",
Expand Down
24 changes: 10 additions & 14 deletions module/Core/src/Config/NotFoundRedirectResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

namespace Shlinkio\Shlink\Core\Config;

use League\Uri\Exceptions\SyntaxError;
use League\Uri\Uri;
use Laminas\Diactoros\Exception\InvalidArgumentException;
use Laminas\Diactoros\Uri;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\UriInterface;
use Psr\Log\LoggerInterface;
Expand Down Expand Up @@ -51,8 +51,8 @@ public function resolveRedirectResponse(
private function resolvePlaceholders(UriInterface $currentUri, string $redirectUrl): string
{
try {
$redirectUri = Uri::createFromString($redirectUrl);
} catch (SyntaxError $e) {
$redirectUri = new Uri($redirectUrl);
} catch (InvalidArgumentException $e) {
$this->logger->warning('It was not possible to parse "{url}" as a valid URL: {e}', [
'e' => $e,
'url' => $redirectUrl,
Expand All @@ -63,26 +63,22 @@ private function resolvePlaceholders(UriInterface $currentUri, string $redirectU
$path = $currentUri->getPath();
$domain = $currentUri->getAuthority();

$replacePlaceholderForPattern = static fn (string $pattern, string $replace, ?string $value): string|null =>
$value === null ? null : str_replace($pattern, $replace, $value);

$replacePlaceholders = static function (
callable $modifier,
?string $value,
string $value,
) use (
$replacePlaceholderForPattern,
$path,
$domain,
): string|null {
$value = $replacePlaceholderForPattern($modifier(self::DOMAIN_PLACEHOLDER), $modifier($domain), $value);
return $replacePlaceholderForPattern($modifier(self::ORIGINAL_PATH_PLACEHOLDER), $modifier($path), $value);
): string {
$value = str_replace(urlencode(self::DOMAIN_PLACEHOLDER), $modifier($domain), $value);
return str_replace(urlencode(self::ORIGINAL_PATH_PLACEHOLDER), $modifier($path), $value);
};

$replacePlaceholdersInPath = static function (string $path) use ($replacePlaceholders): string {
$result = $replacePlaceholders(static fn (mixed $v) => $v, $path);
return str_replace('//', '/', $result ?? '');
return str_replace('//', '/', $result);
};
$replacePlaceholdersInQuery = static fn (?string $query): string|null => $replacePlaceholders(
$replacePlaceholdersInQuery = static fn (string $query): string => $replacePlaceholders(
urlencode(...),
$query,
);
Expand Down
10 changes: 5 additions & 5 deletions module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
namespace Shlinkio\Shlink\Core\ShortUrl\Helper;

use GuzzleHttp\Psr7\Query;
use Laminas\Diactoros\Uri;
use Laminas\Stdlib\ArrayUtils;
use League\Uri\Uri;
use Psr\Http\Message\ServerRequestInterface;
use Shlinkio\Shlink\Core\Model\DeviceType;
use Shlinkio\Shlink\Core\Options\TrackingOptions;
Expand All @@ -27,7 +27,7 @@ public function buildShortUrlRedirect(
): string {
$currentQuery = $request->getQueryParams();
$device = DeviceType::matchFromUserAgent($request->getHeaderLine('User-Agent'));
$uri = Uri::createFromString($shortUrl->longUrlForDevice($device));
$uri = new Uri($shortUrl->longUrlForDevice($device));
$shouldForwardQuery = $shortUrl->forwardQuery();

return $uri
Expand All @@ -36,9 +36,9 @@ public function buildShortUrlRedirect(
->__toString();
}

private function resolveQuery(Uri $uri, array $currentQuery): ?string
private function resolveQuery(Uri $uri, array $currentQuery): string
{
$hardcodedQuery = Query::parse($uri->getQuery() ?? '');
$hardcodedQuery = Query::parse($uri->getQuery());

$disableTrackParam = $this->trackingOptions->disableTrackParam;
if ($disableTrackParam !== null) {
Expand All @@ -48,7 +48,7 @@ private function resolveQuery(Uri $uri, array $currentQuery): ?string
// We want to merge preserving numeric keys, as some params might be numbers
$mergedQuery = ArrayUtils::merge($hardcodedQuery, $currentQuery, true);

return empty($mergedQuery) ? null : Query::build($mergedQuery);
return Query::build($mergedQuery);
}

private function resolvePath(Uri $uri, ?string $extraPath): string
Expand Down
2 changes: 2 additions & 0 deletions module/Core/test-api/Action/RedirectTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ class RedirectTest extends ApiTestCase
public function properRedirectHappensBasedOnUserAgent(?string $userAgent, string $expectedRedirect): void
{
$response = $this->callShortUrl('def456', $userAgent);

self::assertEquals(302, $response->getStatusCode());
self::assertEquals($expectedRedirect, $response->getHeaderLine('Location'));
}

Expand Down
24 changes: 12 additions & 12 deletions module/Core/test/Config/NotFoundRedirectResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,14 @@ public static function provideRedirects(): iterable
yield 'base URL with trailing slash' => [
$uri = new Uri('/'),
self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)),
new NotFoundRedirectOptions(baseUrl: 'baseUrl'),
'baseUrl',
new NotFoundRedirectOptions(baseUrl: 'https://example.com/baseUrl'),
'https://example.com/baseUrl',
];
yield 'base URL without trailing slash' => [
$uri = new Uri(''),
self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)),
new NotFoundRedirectOptions(baseUrl: 'https://example.com/baseUrl'),
'https://example.com/baseUrl',
];
yield 'base URL with domain placeholder' => [
$uri = new Uri('https://s.test'),
Expand All @@ -72,17 +78,11 @@ public static function provideRedirects(): iterable
new NotFoundRedirectOptions(baseUrl: 'https://redirect-here.com/?domain={DOMAIN}'),
'https://redirect-here.com/?domain=s.test',
];
yield 'base URL without trailing slash' => [
$uri = new Uri(''),
self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)),
new NotFoundRedirectOptions(baseUrl: 'baseUrl'),
'baseUrl',
];
yield 'regular 404' => [
$uri = new Uri('/foo/bar'),
self::notFoundType(ServerRequestFactory::fromGlobals()->withUri($uri)),
new NotFoundRedirectOptions(regular404: 'regular404'),
'regular404',
new NotFoundRedirectOptions(regular404: 'https://example.com/regular404'),
'https://example.com/regular404',
];
yield 'regular 404 with path placeholder in query' => [
$uri = new Uri('/foo/bar'),
Expand All @@ -101,8 +101,8 @@ public static function provideRedirects(): iterable
yield 'invalid short URL' => [
new Uri('/foo'),
self::notFoundType(self::requestForRoute(RedirectAction::class)),
new NotFoundRedirectOptions(invalidShortUrl: 'invalidShortUrl'),
'invalidShortUrl',
new NotFoundRedirectOptions(invalidShortUrl: 'https://example.com/invalidShortUrl'),
'https://example.com/invalidShortUrl',
];
yield 'invalid short URL with path placeholder' => [
new Uri('/foo'),
Expand Down
Loading