Merge pull request #2064 from shlinkio/develop

Release 4.0.3

.github/ISSUE_TEMPLATE/Bug.yml

@@ -58,5 +58,10 @@ body:
  validations:
  required: true
  attributes:
- label: How to reproduce
- value: '<!-- Provide steps to reproduce the bug. -->'
+ label: Minimum steps to reproduce
+ value: |
+ <!--
+ Emphasis in MINIMUM: What is the simplest way to reproduce the bug?
+ Avoid things like "Create a kubernetes cluster", or anything related with cloud providers, as that is rarely the root cause and the bug may be closed as "not reproducible".
+ If you can provide a simple docker compose config, that's even better.
+ -->

CHANGELOG.md

@@ -4,6 +4,24 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org).
 
+## [4.0.3] - 2024-03-15
+### Added
+* *Nothing*
+
+### Changed
+* *Nothing*
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* [#2058](https://github.com/shlinkio/shlink/issues/2058) Fix DB credentials provided as env vars being casted to `int` if they include only numbers.
+* [#2060](https://github.com/shlinkio/shlink/issues/2060) Fix error when trying to redirect to a non-http long URL.
+
+
 ## [4.0.2] - 2024-03-09
 ### Added
 * *Nothing*

config/autoload/entity-manager.global.php

@@ -16,6 +16,10 @@
  'mssql' => 'pdo_sqlsrv',
  default => 'pdo_mysql',
  };
+ $readCredentialAsString = static function (EnvVars $envVar): string|null {
+ $value = $envVar->loadFromEnv();
+ return $value === null ? null : (string) $value;
+ };
  $resolveDefaultPort = static fn () => match ($driver) {
  'postgres' => '5432',
  'mssql' => '1433',
@@ -28,6 +32,7 @@
  'postgres' => 'utf8',
  default => null,
  };
+
  $resolveConnection = static fn () => match ($driver) {
  null, 'sqlite' => [
  'driver' => 'pdo_sqlite',
@@ -36,8 +41,8 @@
  default => [
  'driver' => $resolveDriver(),
  'dbname' => EnvVars::DB_NAME->loadFromEnv('shlink'),
- 'user' => EnvVars::DB_USER->loadFromEnv(),
- 'password' => EnvVars::DB_PASSWORD->loadFromEnv(),
+ 'user' => $readCredentialAsString(EnvVars::DB_USER),
+ 'password' => $readCredentialAsString(EnvVars::DB_PASSWORD),
  'host' => EnvVars::DB_HOST->loadFromEnv(EnvVars::DB_UNIX_SOCKET->loadFromEnv()),
  'port' => EnvVars::DB_PORT->loadFromEnv($resolveDefaultPort()),
  'unix_socket' => $isMysqlCompatible ? EnvVars::DB_UNIX_SOCKET->loadFromEnv() : null,

module/Core/functions/array-utils.php

@@ -17,7 +17,6 @@ function contains(mixed $value, array $array): bool
 
 /**
  * @param array[] $multiArray
- * @return array
  */
 function flatten(array $multiArray): array
 {

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

@@ -5,7 +5,7 @@
 namespace Shlinkio\Shlink\Core\ShortUrl\Helper;
 
 use GuzzleHttp\Psr7\Query;
-use Laminas\Diactoros\Uri;
+use GuzzleHttp\Psr7\Uri;
 use Laminas\Stdlib\ArrayUtils;
 use Psr\Http\Message\ServerRequestInterface;
 use Shlinkio\Shlink\Core\Options\TrackingOptions;
@@ -30,31 +30,32 @@ public function buildShortUrlRedirect(
  $uri = new Uri($this->redirectionResolver->resolveLongUrl($shortUrl, $request));
  $currentQuery = $request->getQueryParams();
  $shouldForwardQuery = $shortUrl->forwardQuery();
+ $baseQueryString = $uri->getQuery();
+ $basePath = $uri->getPath();
 
  return $uri
- ->withQuery($shouldForwardQuery ? $this->resolveQuery($uri, $currentQuery) : $uri->getQuery())
- ->withPath($this->resolvePath($uri, $extraPath))
+ ->withQuery($shouldForwardQuery ? $this->resolveQuery($baseQueryString, $currentQuery) : $baseQueryString)
+ ->withPath($this->resolvePath($basePath, $extraPath))
  ->__toString();
  }
 
- private function resolveQuery(Uri $uri, array $currentQuery): string
+ private function resolveQuery(string $baseQueryString, array $currentQuery): string
  {
- $hardcodedQuery = Query::parse($uri->getQuery());
+ $hardcodedQuery = Query::parse($baseQueryString);
 
  $disableTrackParam = $this->trackingOptions->disableTrackParam;
  if ($disableTrackParam !== null) {
  unset($currentQuery[$disableTrackParam]);
  }
 
  // We want to merge preserving numeric keys, as some params might be numbers
- $mergedQuery = ArrayUtils::merge($hardcodedQuery, $currentQuery, true);
+ $mergedQuery = ArrayUtils::merge($hardcodedQuery, $currentQuery, preserveNumericKeys: true);
 
  return Query::build($mergedQuery);
  }
 
- private function resolvePath(Uri $uri, ?string $extraPath): string
+ private function resolvePath(string $basePath, ?string $extraPath): string
  {
- $hardcodedPath = $uri->getPath();
- return $extraPath === null ? $hardcodedPath : sprintf('%s%s', $hardcodedPath, $extraPath);
+ return $extraPath === null ? $basePath : sprintf('%s%s', $basePath, $extraPath);
  }
 }

module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php

@@ -87,7 +87,7 @@ private function tryToResolveRedirect(
  }
 
  /**
- * @return array{0: string, 1: string|null}
+ * @return array{string, string|null}
  */
  private function resolvePotentialShortCodeAndExtraPath(UriInterface $uri, int $shortCodeSegments): array
  {

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

@@ -7,6 +7,7 @@
 use GuzzleHttp\RequestOptions;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\Attributes\TestWith;
 use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;
 
 use const ShlinkioTest\Shlink\ANDROID_USER_AGENT;
@@ -15,28 +16,28 @@
 
 class RedirectTest extends ApiTestCase
 {
- #[Test, DataProvider('provideUserAgents')]
- public function properRedirectHappensBasedOnUserAgent(array $options, string $expectedRedirect): void
+ #[Test, DataProvider('provideRequestOptions')]
+ public function properRedirectHappensBasedOnRedirectRules(array $options, string $expectedRedirect): void
  {
  $response = $this->callShortUrl('def456', $options);
 
  self::assertEquals(302, $response->getStatusCode());
  self::assertEquals($expectedRedirect, $response->getHeaderLine('Location'));
  }
 
- public static function provideUserAgents(): iterable
+ public static function provideRequestOptions(): iterable
  {
  yield 'android' => [
  [
  RequestOptions::HEADERS => ['User-Agent' => ANDROID_USER_AGENT],
  ],
- 'https://blog.alejandrocelaya.com/android',
+ 'android://foo/bar',
  ];
  yield 'ios' => [
  [
  RequestOptions::HEADERS => ['User-Agent' => IOS_USER_AGENT],
  ],
- 'https://blog.alejandrocelaya.com/ios',
+ 'fb://profile/33138223345',
  ];
  yield 'desktop' => [
  [
@@ -86,4 +87,27 @@ public static function provideUserAgents(): iterable
  'https://blog.alejandrocelaya.com/2017/12/09/acmailer-7-0-the-most-important-release-in-a-long-time/',
  ];
  }
+
+ /**
+ * @param non-empty-string $longUrl
+ */
+ #[Test]
+ #[TestWith(['android://foo/bar'])]
+ #[TestWith(['fb://profile/33138223345'])]
+ #[TestWith(['viber://pa?chatURI=1234'])]
+ public function properRedirectHappensForNonHttpLongUrls(string $longUrl): void
+ {
+ $slug = 'non-http-schema';
+ $this->callApiWithKey('POST', '/short-urls', [
+ RequestOptions::JSON => [
+ 'longUrl' => $longUrl,
+ 'customSlug' => $slug,
+ ],
+ ]);
+
+ $response = $this->callShortUrl($slug);
+
+ self::assertEquals(302, $response->getStatusCode());
+ self::assertEquals($longUrl, $response->getHeaderLine('Location'));
+ }
 }

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

@@ -7,6 +7,7 @@
 use Laminas\Diactoros\ServerRequestFactory;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\Attributes\TestWith;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Psr\Http\Message\ServerRequestInterface;
@@ -37,7 +38,7 @@ public function buildShortUrlRedirectBuildsExpectedUrl(
  ?bool $forwardQuery,
  ): void {
  $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([
- 'longUrl' => 'https://domain.com/foo/bar?some=thing',
+ 'longUrl' => 'https://example.com/foo/bar?some=thing',
  'forwardQuery' => $forwardQuery,
  ]));
  $this->redirectionResolver->expects($this->once())->method('resolveLongUrl')->with(
@@ -54,59 +55,81 @@ public static function provideData(): iterable
  {
  $request = static fn (array $query = []) => ServerRequestFactory::fromGlobals()->withQueryParams($query);
 
- yield ['https://domain.com/foo/bar?some=thing', $request(), null, true];
- yield ['https://domain.com/foo/bar?some=thing', $request(), null, null];
- yield ['https://domain.com/foo/bar?some=thing', $request(), null, false];
- yield ['https://domain.com/foo/bar?some=thing&else', $request(['else' => null]), null, true];
- yield ['https://domain.com/foo/bar?some=thing&foo=bar', $request(['foo' => 'bar']), null, true];
- yield ['https://domain.com/foo/bar?some=thing&foo=bar', $request(['foo' => 'bar']), null, null];
- yield ['https://domain.com/foo/bar?some=thing', $request(['foo' => 'bar']), null, false];
- yield ['https://domain.com/foo/bar?some=thing&123=foo', $request(['123' => 'foo']), null, true];
- yield ['https://domain.com/foo/bar?some=thing&456=foo', $request([456 => 'foo']), null, true];
- yield ['https://domain.com/foo/bar?some=thing&456=foo', $request([456 => 'foo']), null, null];
- yield ['https://domain.com/foo/bar?some=thing', $request([456 => 'foo']), null, false];
+ 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://domain.com/foo/bar?some=overwritten&foo=bar',
+ 'https://example.com/foo/bar?some=overwritten&foo=bar',
  $request(['foo' => 'bar', 'some' => 'overwritten']),
  null,
  true,
  ];
  yield [
- 'https://domain.com/foo/bar?some=overwritten',
+ 'https://example.com/foo/bar?some=overwritten',
  $request(['foobar' => 'notrack', 'some' => 'overwritten']),
  null,
  true,
  ];
  yield [
- 'https://domain.com/foo/bar?some=overwritten',
+ 'https://example.com/foo/bar?some=overwritten',
  $request(['foobar' => 'notrack', 'some' => 'overwritten']),
  null,
  null,
  ];
  yield [
- 'https://domain.com/foo/bar?some=thing',
+ 'https://example.com/foo/bar?some=thing',
  $request(['foobar' => 'notrack', 'some' => 'overwritten']),
  null,
  false,
  ];
- yield ['https://domain.com/foo/bar/something/else-baz?some=thing', $request(), '/something/else-baz', true];
+ yield ['https://example.com/foo/bar/something/else-baz?some=thing', $request(), '/something/else-baz', true];
  yield [
- 'https://domain.com/foo/bar/something/else-baz?some=thing&hello=world',
+ 'https://example.com/foo/bar/something/else-baz?some=thing&hello=world',
  $request(['hello' => 'world']),
  '/something/else-baz',
  true,
  ];
  yield [
- 'https://domain.com/foo/bar/something/else-baz?some=thing&hello=world',
+ 'https://example.com/foo/bar/something/else-baz?some=thing&hello=world',
  $request(['hello' => 'world']),
  '/something/else-baz',
  null,
  ];
  yield [
- 'https://domain.com/foo/bar/something/else-baz?some=thing',
+ 'https://example.com/foo/bar/something/else-baz?some=thing',
  $request(['hello' => 'world']),
  '/something/else-baz',
  false,
  ];
  }
+
+ /**
+ * @param non-empty-string $longUrl
+ */
+ #[Test]
+ #[TestWith(['android://foo/bar'])]
+ #[TestWith(['fb://profile/33138223345'])]
+ #[TestWith(['viber://pa?chatURI=1234'])]
+ public function buildShortUrlRedirectBuildsNonHttpUrls(string $longUrl): void
+ {
+ $shortUrl = ShortUrl::withLongUrl($longUrl);
+ $request = ServerRequestFactory::fromGlobals();
+
+ $this->redirectionResolver->expects($this->once())->method('resolveLongUrl')->with(
+ $shortUrl,
+ $request,
+ )->willReturn($shortUrl->getLongUrl());
+
+ $result = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request);
+
+ self::assertEquals($longUrl, $result);
+ }
 }

module/Rest/test-api/Action/ListRedirectRulesTest.php

@@ -66,7 +66,7 @@ public function errorIsReturnedWhenInvalidUrlIsFetched(): void
  'conditions' => [self::LANGUAGE_EN_CONDITION],
  ],
  [
- 'longUrl' => 'https://blog.alejandrocelaya.com/android',
+ 'longUrl' => 'android://foo/bar',
  'priority' => 4,
  'conditions' => [
  [
@@ -77,7 +77,7 @@ public function errorIsReturnedWhenInvalidUrlIsFetched(): void
  ],
  ],
  [
- 'longUrl' => 'https://blog.alejandrocelaya.com/ios',
+ 'longUrl' => 'fb://profile/33138223345',
  'priority' => 5,
  'conditions' => [
  [

module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php

@@ -49,7 +49,7 @@ public function load(ObjectManager $manager): void
  $androidRule = new ShortUrlRedirectRule(
  shortUrl: $defShortUrl,
  priority: 4,
- longUrl: 'https://blog.alejandrocelaya.com/android',
+ longUrl: 'android://foo/bar',
  conditions: new ArrayCollection([RedirectCondition::forDevice(DeviceType::ANDROID)]),
  );
  $manager->persist($androidRule);
@@ -65,7 +65,7 @@ public function load(ObjectManager $manager): void
  $iosRule = new ShortUrlRedirectRule(
  shortUrl: $defShortUrl,
  priority: 5,
- longUrl: 'https://blog.alejandrocelaya.com/ios',
+ longUrl: 'fb://profile/33138223345',
  conditions: new ArrayCollection([RedirectCondition::forDevice(DeviceType::IOS)]),
  );
  $manager->persist($iosRule);