From b6e1c65c4c0a0d927bf7df8bcf6d84c424b9bbd2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 25 Mar 2023 09:52:47 +0100 Subject: [PATCH 1/5] Enforce a schema to be provided when short URLs are created --- config/constants.php | 1 + .../ShortUrl/ListShortUrlsCommandTest.php | 6 +-- module/Core/src/ShortUrl/Entity/ShortUrl.php | 3 +- .../Model/Validation/ShortUrlInputFilter.php | 40 +++++++++++------- .../Repository/DomainRepositoryTest.php | 2 +- .../CrawlableShortCodesQueryTest.php | 2 +- .../Repository/ShortUrlListRepositoryTest.php | 42 +++++++++---------- .../Repository/ShortUrlRepositoryTest.php | 12 +++--- .../Tag/Repository/TagRepositoryTest.php | 6 +-- .../Visit/Repository/VisitRepositoryTest.php | 10 +++-- .../NotifyNewShortUrlToMercureTest.php | 4 +- .../PublishingUpdatesGeneratorTest.php | 8 ++-- .../NotifyNewShortUrlToRabbitMqTest.php | 4 +- .../RabbitMq/NotifyVisitToRabbitMqTest.php | 6 +-- .../NotifyNewShortUrlToRedisTest.php | 2 +- .../Importer/ImportedLinksProcessorTest.php | 10 ++--- .../test/ShortUrl/Entity/ShortUrlTest.php | 10 +++-- .../Helper/ShortUrlStringifierTest.php | 2 +- .../ExtraPathRedirectMiddlewareTest.php | 2 +- .../ShortUrl/Model/ShortUrlCreationTest.php | 6 +-- .../ShortUrl/Model/ShortUrlEditionTest.php | 22 ++++++---- .../test/ShortUrl/ShortUrlResolverTest.php | 14 ++++--- .../test/ShortUrl/ShortUrlServiceTest.php | 10 ++--- .../ShortUrlDataTransformerTest.php | 8 ++-- .../Visit/Geolocation/VisitLocatorTest.php | 8 ++-- .../test-api/Action/CreateShortUrlTest.php | 2 + 26 files changed, 135 insertions(+), 107 deletions(-) diff --git a/config/constants.php b/config/constants.php index cc802301..77f81fdd 100644 --- a/config/constants.php +++ b/config/constants.php @@ -13,6 +13,7 @@ const DEFAULT_REDIRECT_STATUS_CODE = RedirectStatus::STATUS_302; // Deprecated. const DEFAULT_REDIRECT_CACHE_LIFETIME = 30; const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory'; const TITLE_TAG_VALUE = '/]*>(.*?)<\/title>/i'; // Matches the value inside a html title tag +const LOOSE_URI_MATCHER = '/(.+)\:\/\/(.+)/i'; // Matches anything starting with a schema. const DEFAULT_QR_CODE_SIZE = 300; const DEFAULT_QR_CODE_MARGIN = 0; const DEFAULT_QR_CODE_FORMAT = 'png'; diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index 6b1c790d..52d1eeb3 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -49,7 +49,7 @@ class ListShortUrlsCommandTest extends TestCase // The paginator will return more than one page $data = []; for ($i = 0; $i < 50; $i++) { - $data[] = ShortUrl::withLongUrl('url_' . $i); + $data[] = ShortUrl::withLongUrl('https://url_' . $i); } $this->shortUrlService->expects($this->exactly(3))->method('listShortUrls')->withAnyParameters() @@ -71,7 +71,7 @@ class ListShortUrlsCommandTest extends TestCase // The paginator will return more than one page $data = []; for ($i = 0; $i < 30; $i++) { - $data[] = ShortUrl::withLongUrl('url_' . $i); + $data[] = ShortUrl::withLongUrl('https://url_' . $i); } $this->shortUrlService->expects($this->once())->method('listShortUrls')->with( @@ -114,7 +114,7 @@ class ListShortUrlsCommandTest extends TestCase ShortUrlsParams::emptyInstance(), )->willReturn(new Paginator(new ArrayAdapter([ ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo.com', + 'longUrl' => 'https://foo.com', 'tags' => ['foo', 'bar', 'baz'], 'apiKey' => $apiKey, ])), diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index 15f1998c..7f5cd406 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -33,6 +33,7 @@ use function Shlinkio\Shlink\Core\enumValues; use function Shlinkio\Shlink\Core\generateRandomShortCode; use function Shlinkio\Shlink\Core\normalizeDate; use function Shlinkio\Shlink\Core\normalizeOptionalDate; +use function str_contains; class ShortUrl extends AbstractEntity { @@ -68,7 +69,7 @@ class ShortUrl extends AbstractEntity */ public static function createFake(): self { - return self::withLongUrl('foo'); + return self::withLongUrl('https://foo'); } /** diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php index 9c10d3ff..90b7e8e8 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php @@ -12,8 +12,10 @@ use Shlinkio\Shlink\Common\Validation; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Rest\Entity\ApiKey; +use function preg_match; use function substr; +use const Shlinkio\Shlink\LOOSE_URI_MATCHER; use const Shlinkio\Shlink\MIN_SHORT_CODES_LENGTH; /** @@ -59,27 +61,13 @@ class ShortUrlInputFilter extends InputFilter private function initialize(bool $requireLongUrl, UrlShortenerOptions $options): void { - $longUrlNotEmptyCommonOptions = [ - Validator\NotEmpty::OBJECT, - Validator\NotEmpty::SPACE, - Validator\NotEmpty::EMPTY_ARRAY, - Validator\NotEmpty::BOOLEAN, - Validator\NotEmpty::STRING, - ]; - $longUrlInput = $this->createInput(self::LONG_URL, $requireLongUrl); - $longUrlInput->getValidatorChain()->attach(new Validator\NotEmpty([ - ...$longUrlNotEmptyCommonOptions, - Validator\NotEmpty::NULL, - ])); + $longUrlInput->getValidatorChain()->merge($this->longUrlValidators()); $this->add($longUrlInput); $deviceLongUrlsInput = $this->createInput(self::DEVICE_LONG_URLS, false); $deviceLongUrlsInput->getValidatorChain()->attach( - new DeviceLongUrlsValidator(new Validator\NotEmpty([ - ...$longUrlNotEmptyCommonOptions, - ...($requireLongUrl ? [Validator\NotEmpty::NULL] : []), - ])), + new DeviceLongUrlsValidator($this->longUrlValidators(allowNull: ! $requireLongUrl)) ); $this->add($deviceLongUrlsInput); @@ -129,4 +117,24 @@ class ShortUrlInputFilter extends InputFilter $this->add($this->createBooleanInput(self::CRAWLABLE, false)); } + + private function longUrlValidators(bool $allowNull = false): Validator\ValidatorChain + { + $emptyModifiers = [ + Validator\NotEmpty::OBJECT, + Validator\NotEmpty::SPACE, + Validator\NotEmpty::EMPTY_ARRAY, + Validator\NotEmpty::BOOLEAN, + Validator\NotEmpty::STRING, + ]; + if (! $allowNull) { + $emptyModifiers[] = Validator\NotEmpty::NULL; + } + + return (new Validator\ValidatorChain()) + ->attach(new Validator\NotEmpty($emptyModifiers)) + ->attach(new Validator\Callback( + fn (?string $value) => ($allowNull && $value === null) || preg_match(LOOSE_URI_MATCHER, $value) === 1 + )); + } } diff --git a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php index 24824ee4..58817f38 100644 --- a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php +++ b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php @@ -132,7 +132,7 @@ class DomainRepositoryTest extends DatabaseTestCase { return ShortUrl::create( ShortUrlCreation::fromRawData( - ['domain' => $domain->authority, 'apiKey' => $apiKey, 'longUrl' => 'foo'], + ['domain' => $domain->authority, 'apiKey' => $apiKey, 'longUrl' => 'https://foo'], ), new class ($domain) implements ShortUrlRelationResolverInterface { public function __construct(private Domain $domain) diff --git a/module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php b/module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php index 47f13567..d630520b 100644 --- a/module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php @@ -24,7 +24,7 @@ class CrawlableShortCodesQueryTest extends DatabaseTestCase public function invokingQueryReturnsExpectedResult(): void { $createShortUrl = fn (bool $crawlable) => ShortUrl::create( - ShortUrlCreation::fromRawData(['crawlable' => $crawlable, 'longUrl' => 'foo.com']), + ShortUrlCreation::fromRawData(['crawlable' => $crawlable, 'longUrl' => 'https://foo.com']), ); $shortUrl1 = $createShortUrl(true); diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php index 97c6dd22..e2e98a14 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php @@ -43,7 +43,7 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase { $count = 5; for ($i = 0; $i < $count; $i++) { - $this->getEntityManager()->persist(ShortUrl::withLongUrl((string) $i)); + $this->getEntityManager()->persist(ShortUrl::withLongUrl('https://' . $i)); } $this->getEntityManager()->flush(); @@ -54,12 +54,12 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase public function findListProperlyFiltersResult(): void { $foo = ShortUrl::create( - ShortUrlCreation::fromRawData(['longUrl' => 'foo', 'tags' => ['bar']]), + ShortUrlCreation::fromRawData(['longUrl' => 'foo', 'tags' => ['https://bar']]), $this->relationResolver, ); $this->getEntityManager()->persist($foo); - $bar = ShortUrl::withLongUrl('bar'); + $bar = ShortUrl::withLongUrl('https://bar'); $visits = map(range(0, 5), function () use ($bar) { $visit = Visit::forValidShortUrl($bar, Visitor::botInstance()); $this->getEntityManager()->persist($visit); @@ -69,7 +69,7 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase $bar->setVisits(new ArrayCollection($visits)); $this->getEntityManager()->persist($bar); - $foo2 = ShortUrl::withLongUrl('foo_2'); + $foo2 = ShortUrl::withLongUrl('https://foo_2'); $visits2 = map(range(0, 3), function () use ($foo2) { $visit = Visit::forValidShortUrl($foo2, Visitor::emptyInstance()); $this->getEntityManager()->persist($visit); @@ -147,7 +147,7 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase #[Test] public function findListProperlyMapsFieldNamesToColumnNamesWhenOrdering(): void { - $urls = ['a', 'z', 'c', 'b']; + $urls = ['https://a', 'https://z', 'https://c', 'https://b']; foreach ($urls as $url) { $this->getEntityManager()->persist(ShortUrl::withLongUrl($url)); } @@ -159,37 +159,37 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase ); self::assertCount(count($urls), $result); - self::assertEquals('a', $result[0]->getLongUrl()); - self::assertEquals('b', $result[1]->getLongUrl()); - self::assertEquals('c', $result[2]->getLongUrl()); - self::assertEquals('z', $result[3]->getLongUrl()); + self::assertEquals('https://a', $result[0]->getLongUrl()); + self::assertEquals('https://b', $result[1]->getLongUrl()); + self::assertEquals('https://c', $result[2]->getLongUrl()); + self::assertEquals('https://z', $result[3]->getLongUrl()); } #[Test] public function findListReturnsOnlyThoseWithMatchingTags(): void { $shortUrl1 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo1', + 'longUrl' => 'https://foo1', 'tags' => ['foo', 'bar'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl1); $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo2', + 'longUrl' => 'https://foo2', 'tags' => ['foo', 'baz'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl2); $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo3', + 'longUrl' => 'https://foo3', 'tags' => ['foo'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl3); $shortUrl4 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo4', + 'longUrl' => 'https://foo4', 'tags' => ['bar', 'baz'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl4); $shortUrl5 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo5', + 'longUrl' => 'https://foo5', 'tags' => ['bar', 'baz'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl5); @@ -278,17 +278,17 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase public function findListReturnsOnlyThoseWithMatchingDomains(): void { $shortUrl1 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo1', + 'longUrl' => 'https://foo1', 'domain' => null, ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl1); $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo2', + 'longUrl' => 'https://foo2', 'domain' => null, ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl2); $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo3', + 'longUrl' => 'https://foo3', 'domain' => 'another.com', ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl3); @@ -314,22 +314,22 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase public function findListReturnsOnlyThoseWithoutExcludedUrls(): void { $shortUrl1 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo1', + 'longUrl' => 'https://foo1', 'validUntil' => Chronos::now()->addDays(1)->toAtomString(), 'maxVisits' => 100, ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl1); $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo2', + 'longUrl' => 'https://foo2', 'validUntil' => Chronos::now()->subDays(1)->toAtomString(), ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl2); $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo3', + 'longUrl' => 'https://foo3', ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl3); $shortUrl4 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo4', + 'longUrl' => 'https://foo4', 'maxVisits' => 3, ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl4); diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php index da1755ac..3eefcc8c 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php @@ -34,16 +34,18 @@ class ShortUrlRepositoryTest extends DatabaseTestCase #[Test] public function findOneWithDomainFallbackReturnsProperData(): void { - $regularOne = ShortUrl::create(ShortUrlCreation::fromRawData(['customSlug' => 'Foo', 'longUrl' => 'foo'])); + $regularOne = ShortUrl::create( + ShortUrlCreation::fromRawData(['customSlug' => 'Foo', 'longUrl' => 'https://foo']), + ); $this->getEntityManager()->persist($regularOne); $withDomain = ShortUrl::create(ShortUrlCreation::fromRawData( - ['domain' => 'example.com', 'customSlug' => 'domain-short-code', 'longUrl' => 'foo'], + ['domain' => 'example.com', 'customSlug' => 'domain-short-code', 'longUrl' => 'https://foo'], )); $this->getEntityManager()->persist($withDomain); $withDomainDuplicatingRegular = ShortUrl::create(ShortUrlCreation::fromRawData( - ['domain' => 's.test', 'customSlug' => 'Foo', 'longUrl' => 'foo_with_domain'], + ['domain' => 's.test', 'customSlug' => 'Foo', 'longUrl' => 'https://foo_with_domain'], )); $this->getEntityManager()->persist($withDomainDuplicatingRegular); @@ -102,7 +104,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase public function shortCodeIsInUseLooksForShortUrlInProperSetOfTables(): void { $shortUrlWithoutDomain = ShortUrl::create( - ShortUrlCreation::fromRawData(['customSlug' => 'my-cool-slug', 'longUrl' => 'foo']), + ShortUrlCreation::fromRawData(['customSlug' => 'my-cool-slug', 'longUrl' => 'https://foo']), ); $this->getEntityManager()->persist($shortUrlWithoutDomain); @@ -396,7 +398,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase public function importedShortUrlsAreFoundWhenExpected(): void { $buildImported = static fn (string $shortCode, ?string $domain = null) => - new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), $domain, $shortCode, null); + new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], Chronos::now(), $domain, $shortCode, null); $shortUrlWithoutDomain = ShortUrl::fromImport($buildImported('my-cool-slug'), true); $this->getEntityManager()->persist($shortUrlWithoutDomain); diff --git a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php index 47b4b41d..437b1de4 100644 --- a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php @@ -74,7 +74,7 @@ class TagRepositoryTest extends DatabaseTestCase [$firstUrlTags] = array_chunk($names, 3); $secondUrlTags = [$names[0]]; $metaWithTags = static fn (array $tags, ?ApiKey $apiKey) => ShortUrlCreation::fromRawData( - ['longUrl' => 'longUrl', 'tags' => $tags, 'apiKey' => $apiKey], + ['longUrl' => 'https://longUrl', 'tags' => $tags, 'apiKey' => $apiKey], ); $shortUrl = ShortUrl::create($metaWithTags($firstUrlTags, $apiKey), $this->relationResolver); @@ -241,14 +241,14 @@ class TagRepositoryTest extends DatabaseTestCase [$firstUrlTags, $secondUrlTags] = array_chunk($names, 3); $shortUrl = ShortUrl::create( - ShortUrlCreation::fromRawData(['apiKey' => $authorApiKey, 'longUrl' => 'longUrl', 'tags' => $firstUrlTags]), + ShortUrlCreation::fromRawData(['apiKey' => $authorApiKey, 'longUrl' => 'https://longUrl', 'tags' => $firstUrlTags]), $this->relationResolver, ); $this->getEntityManager()->persist($shortUrl); $shortUrl2 = ShortUrl::create( ShortUrlCreation::fromRawData( - ['domain' => $domain->authority, 'longUrl' => 'longUrl', 'tags' => $secondUrlTags], + ['domain' => $domain->authority, 'longUrl' => 'https://longUrl', 'tags' => $secondUrlTags], ), $this->relationResolver, ); diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index 3e0ca9bf..e5117078 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -266,7 +266,7 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($apiKey1); $shortUrl = ShortUrl::create( ShortUrlCreation::fromRawData( - ['apiKey' => $apiKey1, 'domain' => $domain->authority, 'longUrl' => 'longUrl'], + ['apiKey' => $apiKey1, 'domain' => $domain->authority, 'longUrl' => 'https://longUrl'], ), $this->relationResolver, ); @@ -275,13 +275,15 @@ class VisitRepositoryTest extends DatabaseTestCase $apiKey2 = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $this->getEntityManager()->persist($apiKey2); - $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData(['apiKey' => $apiKey2, 'longUrl' => 'longUrl'])); + $shortUrl2 = ShortUrl::create( + ShortUrlCreation::fromRawData(['apiKey' => $apiKey2, 'longUrl' => 'https://longUrl']), + ); $this->getEntityManager()->persist($shortUrl2); $this->createVisitsForShortUrl($shortUrl2, 5); $shortUrl3 = ShortUrl::create( ShortUrlCreation::fromRawData( - ['apiKey' => $apiKey2, 'domain' => $domain->authority, 'longUrl' => 'longUrl'], + ['apiKey' => $apiKey2, 'domain' => $domain->authority, 'longUrl' => 'https://longUrl'], ), $this->relationResolver, ); @@ -320,7 +322,7 @@ class VisitRepositoryTest extends DatabaseTestCase #[Test] public function findOrphanVisitsReturnsExpectedResult(): void { - $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'longUrl'])); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'https://longUrl'])); $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, 7); diff --git a/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php b/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php index ceec7235..20d6830d 100644 --- a/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php +++ b/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php @@ -58,7 +58,7 @@ class NotifyNewShortUrlToMercureTest extends TestCase #[Test] public function expectedNotificationIsPublished(): void { - $shortUrl = ShortUrl::withLongUrl('longUrl'); + $shortUrl = ShortUrl::withLongUrl('https://longUrl'); $update = Update::forTopicAndPayload('', []); $this->em->expects($this->once())->method('find')->with(ShortUrl::class, '123')->willReturn($shortUrl); @@ -75,7 +75,7 @@ class NotifyNewShortUrlToMercureTest extends TestCase #[Test] public function messageIsPrintedIfPublishingFails(): void { - $shortUrl = ShortUrl::withLongUrl('longUrl'); + $shortUrl = ShortUrl::withLongUrl('https://longUrl'); $update = Update::forTopicAndPayload('', []); $e = new Exception('Error'); diff --git a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php index d2d5b0e2..9d28f2cd 100644 --- a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php +++ b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php @@ -37,7 +37,7 @@ class PublishingUpdatesGeneratorTest extends TestCase { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'customSlug' => 'foo', - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', 'title' => $title, ])); $visit = Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()); @@ -50,7 +50,7 @@ class PublishingUpdatesGeneratorTest extends TestCase 'shortUrl' => [ 'shortCode' => $shortUrl->getShortCode(), 'shortUrl' => 'http:/' . $shortUrl->getShortCode(), - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', 'deviceLongUrls' => $shortUrl->deviceLongUrls(), 'dateCreated' => $shortUrl->getDateCreated()->toAtomString(), 'visitsCount' => 0, @@ -115,7 +115,7 @@ class PublishingUpdatesGeneratorTest extends TestCase { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'customSlug' => 'foo', - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', 'title' => 'The title', ])); @@ -125,7 +125,7 @@ class PublishingUpdatesGeneratorTest extends TestCase self::assertEquals(['shortUrl' => [ 'shortCode' => $shortUrl->getShortCode(), 'shortUrl' => 'http:/' . $shortUrl->getShortCode(), - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', 'deviceLongUrls' => $shortUrl->deviceLongUrls(), 'dateCreated' => $shortUrl->getDateCreated()->toAtomString(), 'visitsCount' => 0, diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php index a3bd9fcc..fc44fd87 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php @@ -70,7 +70,7 @@ class NotifyNewShortUrlToRabbitMqTest extends TestCase $shortUrlId = '123'; $update = Update::forTopicAndPayload(Topic::NEW_SHORT_URL->value, []); $this->em->expects($this->once())->method('find')->with(ShortUrl::class, $shortUrlId)->willReturn( - ShortUrl::withLongUrl('longUrl'), + ShortUrl::withLongUrl('https://longUrl'), ); $this->updatesGenerator->expects($this->once())->method('newShortUrlUpdate')->with( $this->isInstanceOf(ShortUrl::class), @@ -87,7 +87,7 @@ class NotifyNewShortUrlToRabbitMqTest extends TestCase $shortUrlId = '123'; $update = Update::forTopicAndPayload(Topic::NEW_SHORT_URL->value, []); $this->em->expects($this->once())->method('find')->with(ShortUrl::class, $shortUrlId)->willReturn( - ShortUrl::withLongUrl('longUrl'), + ShortUrl::withLongUrl('https://longUrl'), ); $this->updatesGenerator->expects($this->once())->method('newShortUrlUpdate')->with( $this->isInstanceOf(ShortUrl::class), diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php index e8a0f0d5..0002d3b1 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php @@ -98,7 +98,7 @@ class NotifyVisitToRabbitMqTest extends TestCase yield 'non-orphan visit' => [ Visit::forValidShortUrl( ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'customSlug' => 'bar', ])), $visitor, @@ -152,7 +152,7 @@ class NotifyVisitToRabbitMqTest extends TestCase { yield 'legacy non-orphan visit' => [ true, - $visit = Visit::forValidShortUrl(ShortUrl::withLongUrl('longUrl'), Visitor::emptyInstance()), + $visit = Visit::forValidShortUrl(ShortUrl::withLongUrl('https://longUrl'), Visitor::emptyInstance()), noop(...), function (MockObject & PublishingHelperInterface $helper) use ($visit): void { $helper->method('publishUpdate')->with(self::callback(function (Update $update) use ($visit): bool { @@ -183,7 +183,7 @@ class NotifyVisitToRabbitMqTest extends TestCase ]; yield 'non-legacy non-orphan visit' => [ false, - Visit::forValidShortUrl(ShortUrl::withLongUrl('longUrl'), Visitor::emptyInstance()), + Visit::forValidShortUrl(ShortUrl::withLongUrl('https://longUrl'), Visitor::emptyInstance()), function (MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator): void { $update = Update::forTopicAndPayload('', []); $updatesGenerator->expects(self::never())->method('newOrphanVisitUpdate'); diff --git a/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php b/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php index 894abceb..abbe23b9 100644 --- a/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php +++ b/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php @@ -54,7 +54,7 @@ class NotifyNewShortUrlToRedisTest extends TestCase $shortUrlId = '123'; $update = Update::forTopicAndPayload(Topic::NEW_SHORT_URL->value, []); $this->em->expects($this->once())->method('find')->with(ShortUrl::class, $shortUrlId)->willReturn( - ShortUrl::withLongUrl('longUrl'), + ShortUrl::withLongUrl('https://longUrl'), ); $this->updatesGenerator->expects($this->once())->method('newShortUrlUpdate')->with( $this->isInstanceOf(ShortUrl::class), diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php index c8132699..d69d8480 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -117,11 +117,11 @@ class ImportedLinksProcessorTest extends TestCase public function alreadyImportedUrlsAreSkipped(): void { $urls = [ - new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), null, 'foo', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'bar', [], Chronos::now(), null, 'bar', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz', [], Chronos::now(), null, 'baz', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz2', [], Chronos::now(), null, 'baz2', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz3', [], Chronos::now(), null, 'baz3', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], Chronos::now(), null, 'foo', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://bar', [], Chronos::now(), null, 'bar', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz', [], Chronos::now(), null, 'baz', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz2', [], Chronos::now(), null, 'baz2', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz3', [], Chronos::now(), null, 'baz3', null), ]; $this->em->method('getRepository')->with(ShortUrl::class)->willReturn($this->repo); diff --git a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php index 54e21461..4a11d26a 100644 --- a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php +++ b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php @@ -43,7 +43,9 @@ class ShortUrlTest extends TestCase public static function provideInvalidShortUrls(): iterable { yield 'with custom slug' => [ - ShortUrl::create(ShortUrlCreation::fromRawData(['customSlug' => 'custom-slug', 'longUrl' => 'longUrl'])), + ShortUrl::create( + ShortUrlCreation::fromRawData(['customSlug' => 'custom-slug', 'longUrl' => 'https://longUrl']), + ), 'The short code cannot be regenerated on ShortUrls where a custom slug was provided.', ]; yield 'already persisted' => [ @@ -68,7 +70,7 @@ class ShortUrlTest extends TestCase { yield 'no custom slug' => [ShortUrl::createFake()]; yield 'imported with custom slug' => [ShortUrl::fromImport( - new ImportedShlinkUrl(ImportSource::BITLY, 'longUrl', [], Chronos::now(), null, 'custom-slug', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://url', [], Chronos::now(), null, 'custom-slug', null), true, )]; } @@ -77,7 +79,7 @@ class ShortUrlTest extends TestCase public function shortCodesHaveExpectedLength(?int $length, int $expectedLength): void { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData( - [ShortUrlInputFilter::SHORT_CODE_LENGTH => $length, 'longUrl' => 'longUrl'], + [ShortUrlInputFilter::SHORT_CODE_LENGTH => $length, 'longUrl' => 'https://longUrl'], )); self::assertEquals($expectedLength, strlen($shortUrl->getShortCode())); @@ -92,7 +94,7 @@ class ShortUrlTest extends TestCase #[Test] public function deviceLongUrlsAreUpdated(): void { - $shortUrl = ShortUrl::withLongUrl('foo'); + $shortUrl = ShortUrl::withLongUrl('https://foo'); $shortUrl->update(ShortUrlEdition::fromRawData([ ShortUrlInputFilter::DEVICE_LONG_URLS => [ diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php index 1ccd6eac..e74d6182 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php @@ -29,7 +29,7 @@ class ShortUrlStringifierTest extends TestCase { $shortUrlWithShortCode = fn (string $shortCode, ?string $domain = null) => ShortUrl::create( ShortUrlCreation::fromRawData([ - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', 'customSlug' => $shortCode, 'domain' => $domain, ]), diff --git a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php index bbe8e770..3965fe18 100644 --- a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php +++ b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php @@ -136,7 +136,7 @@ class ExtraPathRedirectMiddlewareTest extends TestCase $type->method('isInvalidShortUrl')->willReturn(true); $request = ServerRequestFactory::fromGlobals()->withAttribute(NotFoundType::class, $type) ->withUri(new Uri('https://s.test/shortCode/bar/baz')); - $shortUrl = ShortUrl::withLongUrl('longUrl'); + $shortUrl = ShortUrl::withLongUrl('https://longUrl'); $currentIteration = 1; $this->resolver->expects($this->exactly($expectedResolveCalls))->method('resolveEnabledShortUrl')->with( diff --git a/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php b/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php index a46474c0..6712c49f 100644 --- a/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php +++ b/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php @@ -115,7 +115,7 @@ class ShortUrlCreationTest extends TestCase $creation = ShortUrlCreation::fromRawData([ 'validSince' => Chronos::parse('2015-01-01')->toAtomString(), 'customSlug' => $customSlug, - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', ], new UrlShortenerOptions(multiSegmentSlugsEnabled: $multiSegmentEnabled, mode: $shortUrlMode)); self::assertTrue($creation->hasValidSince()); @@ -161,7 +161,7 @@ class ShortUrlCreationTest extends TestCase { $creation = ShortUrlCreation::fromRawData([ 'title' => $title, - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', ]); self::assertEquals($expectedTitle, $creation->title); @@ -184,7 +184,7 @@ class ShortUrlCreationTest extends TestCase { $creation = ShortUrlCreation::fromRawData([ 'domain' => $domain, - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', ]); self::assertSame($expectedDomain, $creation->domain); diff --git a/module/Core/test/ShortUrl/Model/ShortUrlEditionTest.php b/module/Core/test/ShortUrl/Model/ShortUrlEditionTest.php index 720c290f..5d77d806 100644 --- a/module/Core/test/ShortUrl/Model/ShortUrlEditionTest.php +++ b/module/Core/test/ShortUrl/Model/ShortUrlEditionTest.php @@ -31,23 +31,29 @@ class ShortUrlEditionTest extends TestCase yield 'null' => [null, [], []]; yield 'empty' => [[], [], []]; yield 'only new urls' => [[ - DeviceType::DESKTOP->value => 'foo', - DeviceType::IOS->value => 'bar', + DeviceType::DESKTOP->value => 'https://foo', + DeviceType::IOS->value => 'https://bar', ], [ - DeviceType::DESKTOP->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::DESKTOP->value, 'foo'), - DeviceType::IOS->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::IOS->value, 'bar'), + DeviceType::DESKTOP->value => DeviceLongUrlPair::fromRawTypeAndLongUrl( + DeviceType::DESKTOP->value, + 'https://foo', + ), + DeviceType::IOS->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::IOS->value, 'https://bar'), ], []]; yield 'only urls to remove' => [[ DeviceType::ANDROID->value => null, DeviceType::IOS->value => null, ], [], [DeviceType::ANDROID, DeviceType::IOS]]; yield 'both' => [[ - DeviceType::DESKTOP->value => 'bar', - DeviceType::IOS->value => 'foo', + DeviceType::DESKTOP->value => 'https://bar', + DeviceType::IOS->value => 'https://foo', DeviceType::ANDROID->value => null, ], [ - DeviceType::DESKTOP->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::DESKTOP->value, 'bar'), - DeviceType::IOS->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::IOS->value, 'foo'), + DeviceType::DESKTOP->value => DeviceLongUrlPair::fromRawTypeAndLongUrl( + DeviceType::DESKTOP->value, + 'https://bar', + ), + DeviceType::IOS->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::IOS->value, 'https://foo'), ], [DeviceType::ANDROID]]; } } diff --git a/module/Core/test/ShortUrl/ShortUrlResolverTest.php b/module/Core/test/ShortUrl/ShortUrlResolverTest.php index 0ecafd3a..f2b89586 100644 --- a/module/Core/test/ShortUrl/ShortUrlResolverTest.php +++ b/module/Core/test/ShortUrl/ShortUrlResolverTest.php @@ -45,7 +45,7 @@ class ShortUrlResolverTest extends TestCase #[Test, DataProvider('provideAdminApiKeys')] public function shortCodeIsProperlyParsed(?ApiKey $apiKey): void { - $shortUrl = ShortUrl::withLongUrl('expected_url'); + $shortUrl = ShortUrl::withLongUrl('https://expected_url'); $shortCode = $shortUrl->getShortCode(); $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($shortCode); @@ -76,7 +76,7 @@ class ShortUrlResolverTest extends TestCase #[Test] public function shortCodeToEnabledShortUrlProperlyParsesShortCode(): void { - $shortUrl = ShortUrl::withLongUrl('expected_url'); + $shortUrl = ShortUrl::withLongUrl('https://expected_url'); $shortCode = $shortUrl->getShortCode(); $this->repo->expects($this->once())->method('findOneWithDomainFallback')->with( @@ -111,7 +111,9 @@ class ShortUrlResolverTest extends TestCase $now = Chronos::now(); yield 'maxVisits reached' => [(function () { - $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => 'longUrl'])); + $shortUrl = ShortUrl::create( + ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => 'https://longUrl']), + ); $shortUrl->setVisits(new ArrayCollection(map( range(0, 4), fn () => Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()), @@ -120,16 +122,16 @@ class ShortUrlResolverTest extends TestCase return $shortUrl; })()]; yield 'future validSince' => [ShortUrl::create(ShortUrlCreation::fromRawData( - ['validSince' => $now->addMonth()->toAtomString(), 'longUrl' => 'longUrl'], + ['validSince' => $now->addMonth()->toAtomString(), 'longUrl' => 'https://longUrl'], ))]; yield 'past validUntil' => [ShortUrl::create(ShortUrlCreation::fromRawData( - ['validUntil' => $now->subMonth()->toAtomString(), 'longUrl' => 'longUrl'], + ['validUntil' => $now->subMonth()->toAtomString(), 'longUrl' => 'https://longUrl'], ))]; yield 'mixed' => [(function () use ($now) { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'maxVisits' => 3, 'validUntil' => $now->subMonth()->toAtomString(), - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', ])); $shortUrl->setVisits(new ArrayCollection(map( range(0, 4), diff --git a/module/Core/test/ShortUrl/ShortUrlServiceTest.php b/module/Core/test/ShortUrl/ShortUrlServiceTest.php index 60959215..409c937f 100644 --- a/module/Core/test/ShortUrl/ShortUrlServiceTest.php +++ b/module/Core/test/ShortUrl/ShortUrlServiceTest.php @@ -57,7 +57,7 @@ class ShortUrlServiceTest extends TestCase ShortUrlEdition $shortUrlEdit, ?ApiKey $apiKey, ): void { - $originalLongUrl = 'originalLongUrl'; + $originalLongUrl = 'https://originalLongUrl'; $shortUrl = ShortUrl::withLongUrl($originalLongUrl); $this->urlResolver->expects($this->once())->method('resolveShortUrl')->with( @@ -103,16 +103,16 @@ class ShortUrlServiceTest extends TestCase yield 'long URL and API key' => [new InvokedCount(1), ShortUrlEdition::fromRawData([ 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(), 'maxVisits' => 10, - 'longUrl' => 'modifiedLongUrl', + 'longUrl' => 'https://modifiedLongUrl', ]), ApiKey::create()]; yield 'long URL with validation' => [new InvokedCount(1), ShortUrlEdition::fromRawData([ - 'longUrl' => 'modifiedLongUrl', + 'longUrl' => 'https://modifiedLongUrl', 'validateUrl' => true, ]), null]; yield 'device redirects' => [new InvokedCount(0), ShortUrlEdition::fromRawData([ 'deviceLongUrls' => [ - DeviceType::IOS->value => 'iosLongUrl', - DeviceType::ANDROID->value => 'androidLongUrl', + DeviceType::IOS->value => 'https://iosLongUrl', + DeviceType::ANDROID->value => 'https://androidLongUrl', ], ]), null]; } diff --git a/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php b/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php index 34474ea3..27916063 100644 --- a/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php +++ b/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php @@ -44,7 +44,7 @@ class ShortUrlDataTransformerTest extends TestCase ]]; yield 'max visits only' => [ShortUrl::create(ShortUrlCreation::fromRawData([ 'maxVisits' => $maxVisits, - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', ])), [ 'validSince' => null, 'validUntil' => null, @@ -52,7 +52,7 @@ class ShortUrlDataTransformerTest extends TestCase ]]; yield 'max visits and valid since' => [ ShortUrl::create(ShortUrlCreation::fromRawData( - ['validSince' => $now, 'maxVisits' => $maxVisits, 'longUrl' => 'longUrl'], + ['validSince' => $now, 'maxVisits' => $maxVisits, 'longUrl' => 'https://longUrl'], )), [ 'validSince' => $now->toAtomString(), @@ -62,7 +62,7 @@ class ShortUrlDataTransformerTest extends TestCase ]; yield 'both dates' => [ ShortUrl::create(ShortUrlCreation::fromRawData( - ['validSince' => $now, 'validUntil' => $now->subDays(10), 'longUrl' => 'longUrl'], + ['validSince' => $now, 'validUntil' => $now->subDays(10), 'longUrl' => 'https://longUrl'], )), [ 'validSince' => $now->toAtomString(), @@ -75,7 +75,7 @@ class ShortUrlDataTransformerTest extends TestCase 'validSince' => $now, 'validUntil' => $now->subDays(5), 'maxVisits' => $maxVisits, - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', ])), [ 'validSince' => $now->toAtomString(), diff --git a/module/Core/test/Visit/Geolocation/VisitLocatorTest.php b/module/Core/test/Visit/Geolocation/VisitLocatorTest.php index 4621f1a7..70fc6243 100644 --- a/module/Core/test/Visit/Geolocation/VisitLocatorTest.php +++ b/module/Core/test/Visit/Geolocation/VisitLocatorTest.php @@ -47,8 +47,10 @@ class VisitLocatorTest extends TestCase ): void { $unlocatedVisits = map( range(1, 200), - fn (int $i) => - Visit::forValidShortUrl(ShortUrl::withLongUrl(sprintf('short_code_%s', $i)), Visitor::emptyInstance()), + fn (int $i) => Visit::forValidShortUrl( + ShortUrl::withLongUrl(sprintf('https://short_code_%s', $i)), + Visitor::emptyInstance(), + ), ); $this->repo->expects($this->once())->method($expectedRepoMethodName)->willReturn($unlocatedVisits); @@ -85,7 +87,7 @@ class VisitLocatorTest extends TestCase bool $isNonLocatableAddress, ): void { $unlocatedVisits = [ - Visit::forValidShortUrl(ShortUrl::withLongUrl('foo'), Visitor::emptyInstance()), + Visit::forValidShortUrl(ShortUrl::withLongUrl('https://foo'), Visitor::emptyInstance()), ]; $this->repo->expects($this->once())->method($expectedRepoMethodName)->willReturn($unlocatedVisits); diff --git a/module/Rest/test-api/Action/CreateShortUrlTest.php b/module/Rest/test-api/Action/CreateShortUrlTest.php index d93fc9f1..108a0f6f 100644 --- a/module/Rest/test-api/Action/CreateShortUrlTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlTest.php @@ -268,6 +268,8 @@ class CreateShortUrlTest extends ApiTestCase yield 'missing long url v3' => [[], '3', 'https://shlink.io/api/error/invalid-data']; yield 'empty long url v2' => [['longUrl' => null], '2', 'INVALID_ARGUMENT']; yield 'empty long url v3' => [['longUrl' => ' '], '3', 'https://shlink.io/api/error/invalid-data']; + yield 'missing url schema v2' => [['longUrl' => 'foo.com'], '2', 'INVALID_ARGUMENT']; + yield 'missing url schema v3' => [['longUrl' => 'foo.com'], '3', 'https://shlink.io/api/error/invalid-data']; yield 'empty device long url v2' => [[ 'longUrl' => 'foo', 'deviceLongUrls' => [ From 26f237069cef0b1be94c78fae4bf95d3930e882b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 25 Mar 2023 10:55:30 +0100 Subject: [PATCH 2/5] Fixed unit tests --- .../Model/Validation/ShortUrlInputFilter.php | 4 ++- .../Importer/ImportedLinksProcessorTest.php | 30 ++++++++++--------- .../test/ShortUrl/Entity/ShortUrlTest.php | 18 +++++------ .../ShortUrl/Model/ShortUrlCreationTest.php | 29 ++++++++++-------- 4 files changed, 44 insertions(+), 37 deletions(-) diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php index 90b7e8e8..762a9230 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php @@ -12,6 +12,7 @@ use Shlinkio\Shlink\Common\Validation; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Rest\Entity\ApiKey; +use function is_string; use function preg_match; use function substr; @@ -134,7 +135,8 @@ class ShortUrlInputFilter extends InputFilter return (new Validator\ValidatorChain()) ->attach(new Validator\NotEmpty($emptyModifiers)) ->attach(new Validator\Callback( - fn (?string $value) => ($allowNull && $value === null) || preg_match(LOOSE_URI_MATCHER, $value) === 1 + // Non-strings is always allowed. Other validators will take care of those + static fn (mixed $value) => ! is_string($value) || preg_match(LOOSE_URI_MATCHER, $value) === 1, )); } } diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php index d69d8480..95f7a7f8 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -67,9 +67,9 @@ class ImportedLinksProcessorTest extends TestCase public function newUrlsWithNoErrorsAreAllPersisted(): void { $urls = [ - new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), null, 'foo', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'bar', [], Chronos::now(), null, 'bar', 'foo'), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz', [], Chronos::now(), null, 'baz', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], Chronos::now(), null, 'foo', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://bar', [], Chronos::now(), null, 'bar', 'foo'), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz', [], Chronos::now(), null, 'baz', null), ]; $expectedCalls = count($urls); @@ -90,9 +90,9 @@ class ImportedLinksProcessorTest extends TestCase public function newUrlsWithErrorsAreSkipped(): void { $urls = [ - new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), null, 'foo', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'bar', [], Chronos::now(), null, 'bar', 'foo'), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz', [], Chronos::now(), null, 'baz', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], Chronos::now(), null, 'foo', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://bar', [], Chronos::now(), null, 'bar', 'foo'), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz', [], Chronos::now(), null, 'baz', null), ]; $this->em->method('getRepository')->with(ShortUrl::class)->willReturn($this->repo); @@ -126,8 +126,10 @@ class ImportedLinksProcessorTest extends TestCase $this->em->method('getRepository')->with(ShortUrl::class)->willReturn($this->repo); $this->repo->expects($this->exactly(count($urls)))->method('findOneByImportedUrl')->willReturnCallback( - fn (ImportedShlinkUrl $url): ?ShortUrl - => contains(['foo', 'baz2', 'baz3'], $url->longUrl) ? ShortUrl::fromImport($url, true) : null, + fn (ImportedShlinkUrl $url): ?ShortUrl => contains( + ['https://foo', 'https://baz2', 'https://baz3'], + $url->longUrl, + ) ? ShortUrl::fromImport($url, true) : null, ); $this->shortCodeHelper->expects($this->exactly(2))->method('ensureShortCodeUniqueness')->willReturn(true); $this->em->expects($this->exactly(2))->method('persist')->with($this->isInstanceOf(ShortUrl::class)); @@ -143,11 +145,11 @@ class ImportedLinksProcessorTest extends TestCase public function nonUniqueShortCodesAreAskedToUser(): void { $urls = [ - new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), null, 'foo', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'bar', [], Chronos::now(), null, 'bar', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz', [], Chronos::now(), null, 'baz', 'foo'), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz2', [], Chronos::now(), null, 'baz2', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz3', [], Chronos::now(), null, 'baz3', 'bar'), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], Chronos::now(), null, 'foo', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://bar', [], Chronos::now(), null, 'bar', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz', [], Chronos::now(), null, 'baz', 'foo'), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz2', [], Chronos::now(), null, 'baz2', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz3', [], Chronos::now(), null, 'baz3', 'bar'), ]; $this->em->method('getRepository')->with(ShortUrl::class)->willReturn($this->repo); @@ -191,7 +193,7 @@ class ImportedLinksProcessorTest extends TestCase { $now = Chronos::now(); $createImportedUrl = static fn (array $visits) => - new ImportedShlinkUrl(ImportSource::BITLY, 's', [], $now, null, 's', null, $visits); + new ImportedShlinkUrl(ImportSource::BITLY, 'https://s', [], $now, null, 's', null, $visits); yield 'new short URL' => [$createImportedUrl([ new ImportedShlinkVisit('', '', $now, null), diff --git a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php index 4a11d26a..bd83fd9a 100644 --- a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php +++ b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php @@ -98,26 +98,26 @@ class ShortUrlTest extends TestCase $shortUrl->update(ShortUrlEdition::fromRawData([ ShortUrlInputFilter::DEVICE_LONG_URLS => [ - DeviceType::ANDROID->value => 'android', - DeviceType::IOS->value => 'ios', + DeviceType::ANDROID->value => 'https://android', + DeviceType::IOS->value => 'https://ios', ], ])); self::assertEquals([ - DeviceType::ANDROID->value => 'android', - DeviceType::IOS->value => 'ios', + DeviceType::ANDROID->value => 'https://android', + DeviceType::IOS->value => 'https://ios', DeviceType::DESKTOP->value => null, ], $shortUrl->deviceLongUrls()); $shortUrl->update(ShortUrlEdition::fromRawData([ ShortUrlInputFilter::DEVICE_LONG_URLS => [ DeviceType::ANDROID->value => null, - DeviceType::DESKTOP->value => 'desktop', + DeviceType::DESKTOP->value => 'https://desktop', ], ])); self::assertEquals([ DeviceType::ANDROID->value => null, - DeviceType::IOS->value => 'ios', - DeviceType::DESKTOP->value => 'desktop', + DeviceType::IOS->value => 'https://ios', + DeviceType::DESKTOP->value => 'https://desktop', ], $shortUrl->deviceLongUrls()); $shortUrl->update(ShortUrlEdition::fromRawData([ @@ -129,7 +129,7 @@ class ShortUrlTest extends TestCase self::assertEquals([ DeviceType::ANDROID->value => null, DeviceType::IOS->value => null, - DeviceType::DESKTOP->value => 'desktop', + DeviceType::DESKTOP->value => 'https://desktop', ], $shortUrl->deviceLongUrls()); } @@ -139,7 +139,7 @@ class ShortUrlTest extends TestCase $range = range(1, 1000); // Use a "big" number to reduce false negatives $allFor = static fn (ShortUrlMode $mode): bool => every($range, static function () use ($mode): bool { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData( - [ShortUrlInputFilter::LONG_URL => 'foo'], + [ShortUrlInputFilter::LONG_URL => 'https://foo'], new UrlShortenerOptions(mode: $mode), )); $shortCode = $shortUrl->getShortCode(); diff --git a/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php b/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php index 6712c49f..adecf9e9 100644 --- a/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php +++ b/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php @@ -33,37 +33,37 @@ class ShortUrlCreationTest extends TestCase { yield [[]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::VALID_SINCE => '', ShortUrlInputFilter::VALID_UNTIL => '', ShortUrlInputFilter::CUSTOM_SLUG => 'foobar', ShortUrlInputFilter::MAX_VISITS => 'invalid', ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::VALID_SINCE => '2017', ShortUrlInputFilter::MAX_VISITS => 5, ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::VALID_SINCE => new stdClass(), ShortUrlInputFilter::VALID_UNTIL => 'foo', ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::VALID_UNTIL => 500, ShortUrlInputFilter::DOMAIN => 4, ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::SHORT_CODE_LENGTH => 3, ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::CUSTOM_SLUG => '', ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::CUSTOM_SLUG => ' ', ]]; yield [[ @@ -73,33 +73,36 @@ class ShortUrlCreationTest extends TestCase ShortUrlInputFilter::LONG_URL => null, ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'missing_schema', + ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::DEVICE_LONG_URLS => [ 'invalid' => 'https://shlink.io', ], ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::DEVICE_LONG_URLS => [ DeviceType::DESKTOP->value => '', ], ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::DEVICE_LONG_URLS => [ DeviceType::DESKTOP->value => null, ], ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::DEVICE_LONG_URLS => [ DeviceType::IOS->value => ' ', ], ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::DEVICE_LONG_URLS => [ - DeviceType::IOS->value => 'bar', + DeviceType::IOS->value => 'https://bar', DeviceType::ANDROID->value => [], ], ]]; From 4dfc5ae681ea4dbd48e9c40265021c4727dd5271 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 25 Mar 2023 11:03:35 +0100 Subject: [PATCH 3/5] Fix DB tests --- .../Repository/ShortUrlListRepositoryTest.php | 2 +- .../Repository/ShortUrlRepositoryTest.php | 93 ++++++++++--------- .../Visit/Repository/VisitRepositoryTest.php | 12 +-- 3 files changed, 57 insertions(+), 50 deletions(-) diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php index e2e98a14..46c08d25 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php @@ -54,7 +54,7 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase public function findListProperlyFiltersResult(): void { $foo = ShortUrl::create( - ShortUrlCreation::fromRawData(['longUrl' => 'foo', 'tags' => ['https://bar']]), + ShortUrlCreation::fromRawData(['longUrl' => 'https://foo', 'tags' => ['bar']]), $this->relationResolver, ); $this->getEntityManager()->persist($foo); diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php index 3eefcc8c..441927a1 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php @@ -108,9 +108,9 @@ class ShortUrlRepositoryTest extends DatabaseTestCase ); $this->getEntityManager()->persist($shortUrlWithoutDomain); - $shortUrlWithDomain = ShortUrl::create( - ShortUrlCreation::fromRawData(['domain' => 's.test', 'customSlug' => 'another-slug', 'longUrl' => 'foo']), - ); + $shortUrlWithDomain = ShortUrl::create(ShortUrlCreation::fromRawData( + ['domain' => 's.test', 'customSlug' => 'another-slug', 'longUrl' => 'https://foo'], + )); $this->getEntityManager()->persist($shortUrlWithDomain); $this->getEntityManager()->flush(); @@ -133,13 +133,13 @@ class ShortUrlRepositoryTest extends DatabaseTestCase public function findOneLooksForShortUrlInProperSetOfTables(): void { $shortUrlWithoutDomain = ShortUrl::create( - ShortUrlCreation::fromRawData(['customSlug' => 'my-cool-slug', 'longUrl' => 'foo']), + ShortUrlCreation::fromRawData(['customSlug' => 'my-cool-slug', 'longUrl' => 'https://foo']), ); $this->getEntityManager()->persist($shortUrlWithoutDomain); - $shortUrlWithDomain = ShortUrl::create( - ShortUrlCreation::fromRawData(['domain' => 's.test', 'customSlug' => 'another-slug', 'longUrl' => 'foo']), - ); + $shortUrlWithDomain = ShortUrl::create(ShortUrlCreation::fromRawData( + ['domain' => 's.test', 'customSlug' => 'another-slug', 'longUrl' => 'https://foo'], + )); $this->getEntityManager()->persist($shortUrlWithDomain); $this->getEntityManager()->flush(); @@ -159,14 +159,14 @@ class ShortUrlRepositoryTest extends DatabaseTestCase #[Test] public function findOneMatchingReturnsNullForNonExistingShortUrls(): void { - self::assertNull($this->repo->findOneMatching(ShortUrlCreation::fromRawData(['longUrl' => 'foobar']))); + self::assertNull($this->repo->findOneMatching(ShortUrlCreation::fromRawData(['longUrl' => 'https://foobar']))); self::assertNull($this->repo->findOneMatching( - ShortUrlCreation::fromRawData(['longUrl' => 'foobar', 'tags' => ['foo', 'bar']]), + ShortUrlCreation::fromRawData(['longUrl' => 'https://foobar', 'tags' => ['foo', 'bar']]), )); self::assertNull($this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => Chronos::parse('2020-03-05 20:18:30'), 'customSlug' => 'this_slug_does_not_exist', - 'longUrl' => 'foobar', + 'longUrl' => 'https://foobar', 'tags' => ['foo', 'bar'], ]))); } @@ -177,49 +177,54 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $start = Chronos::parse('2020-03-05 20:18:30'); $end = Chronos::parse('2021-03-05 20:18:30'); - $shortUrl = ShortUrl::create( - ShortUrlCreation::fromRawData(['validSince' => $start, 'longUrl' => 'foo', 'tags' => ['foo', 'bar']]), - $this->relationResolver, - ); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData( + ['validSince' => $start, 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar']], + ), $this->relationResolver); $this->getEntityManager()->persist($shortUrl); - $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData(['validUntil' => $end, 'longUrl' => 'bar'])); + $shortUrl2 = ShortUrl::create( + ShortUrlCreation::fromRawData(['validUntil' => $end, 'longUrl' => 'https://bar']), + ); $this->getEntityManager()->persist($shortUrl2); $shortUrl3 = ShortUrl::create( - ShortUrlCreation::fromRawData(['validSince' => $start, 'validUntil' => $end, 'longUrl' => 'baz']), + ShortUrlCreation::fromRawData(['validSince' => $start, 'validUntil' => $end, 'longUrl' => 'https://baz']), ); $this->getEntityManager()->persist($shortUrl3); $shortUrl4 = ShortUrl::create( - ShortUrlCreation::fromRawData(['customSlug' => 'custom', 'validUntil' => $end, 'longUrl' => 'foo']), + ShortUrlCreation::fromRawData(['customSlug' => 'custom', 'validUntil' => $end, 'longUrl' => 'https://foo']), ); $this->getEntityManager()->persist($shortUrl4); - $shortUrl5 = ShortUrl::create(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => 'foo'])); + $shortUrl5 = ShortUrl::create(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => 'https://foo'])); $this->getEntityManager()->persist($shortUrl5); - $shortUrl6 = ShortUrl::create(ShortUrlCreation::fromRawData(['domain' => 's.test', 'longUrl' => 'foo'])); + $shortUrl6 = ShortUrl::create( + ShortUrlCreation::fromRawData(['domain' => 's.test', 'longUrl' => 'https://foo']), + ); $this->getEntityManager()->persist($shortUrl6); $this->getEntityManager()->flush(); self::assertSame( $shortUrl, - $this->repo->findOneMatching( - ShortUrlCreation::fromRawData(['validSince' => $start, 'longUrl' => 'foo', 'tags' => ['foo', 'bar']]), - ), + $this->repo->findOneMatching(ShortUrlCreation::fromRawData( + ['validSince' => $start, 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar']] + )), ); self::assertSame( $shortUrl2, - $this->repo->findOneMatching(ShortUrlCreation::fromRawData(['validUntil' => $end, 'longUrl' => 'bar'])), + $this->repo->findOneMatching( + ShortUrlCreation::fromRawData(['validUntil' => $end, 'longUrl' => 'https://bar']), + ), ); self::assertSame( $shortUrl3, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, 'validUntil' => $end, - 'longUrl' => 'baz', + 'longUrl' => 'https://baz', ])), ); self::assertSame( @@ -227,16 +232,18 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'customSlug' => 'custom', 'validUntil' => $end, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', ])), ); self::assertSame( $shortUrl5, - $this->repo->findOneMatching(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => 'foo'])), + $this->repo->findOneMatching(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => 'https://foo'])), ); self::assertSame( $shortUrl6, - $this->repo->findOneMatching(ShortUrlCreation::fromRawData(['domain' => 's.test', 'longUrl' => 'foo'])), + $this->repo->findOneMatching( + ShortUrlCreation::fromRawData(['domain' => 's.test', 'longUrl' => 'https://foo']), + ), ); } @@ -246,7 +253,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $start = Chronos::parse('2020-03-05 20:18:30'); $tags = ['foo', 'bar']; $meta = ShortUrlCreation::fromRawData( - ['validSince' => $start, 'maxVisits' => 50, 'longUrl' => 'foo', 'tags' => $tags], + ['validSince' => $start, 'maxVisits' => 50, 'longUrl' => 'https://foo', 'tags' => $tags], ); $shortUrl1 = ShortUrl::create($meta, $this->relationResolver); @@ -295,14 +302,14 @@ class ShortUrlRepositoryTest extends DatabaseTestCase 'validSince' => $start, 'apiKey' => $apiKey, 'domain' => $rightDomain->authority, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl); $nonDomainShortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'apiKey' => $apiKey, - 'longUrl' => 'non-domain', + 'longUrl' => 'https://non-domain', ]), $this->relationResolver); $this->getEntityManager()->persist($nonDomainShortUrl); @@ -310,26 +317,26 @@ class ShortUrlRepositoryTest extends DatabaseTestCase self::assertSame( $shortUrl, - $this->repo->findOneMatching( - ShortUrlCreation::fromRawData(['validSince' => $start, 'longUrl' => 'foo', 'tags' => ['foo', 'bar']]), - ), + $this->repo->findOneMatching(ShortUrlCreation::fromRawData( + ['validSince' => $start, 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar']], + )), ); self::assertSame($shortUrl, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, 'apiKey' => $apiKey, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ]))); self::assertSame($shortUrl, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, 'apiKey' => $adminApiKey, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ]))); self::assertNull($this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, 'apiKey' => $otherApiKey, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ]))); @@ -338,7 +345,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, 'domain' => $rightDomain->authority, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ])), ); @@ -348,7 +355,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase 'validSince' => $start, 'domain' => $rightDomain->authority, 'apiKey' => $rightDomainApiKey, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ])), ); @@ -358,7 +365,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase 'validSince' => $start, 'domain' => $rightDomain->authority, 'apiKey' => $apiKey, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ])), ); @@ -367,7 +374,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase 'validSince' => $start, 'domain' => $rightDomain->authority, 'apiKey' => $wrongDomainApiKey, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ])), ); @@ -376,20 +383,20 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $nonDomainShortUrl, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'apiKey' => $apiKey, - 'longUrl' => 'non-domain', + 'longUrl' => 'https://non-domain', ])), ); self::assertSame( $nonDomainShortUrl, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'apiKey' => $adminApiKey, - 'longUrl' => 'non-domain', + 'longUrl' => 'https://non-domain', ])), ); self::assertNull( $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'apiKey' => $otherApiKey, - 'longUrl' => 'non-domain', + 'longUrl' => 'https://non-domain', ])), ); } diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index e5117078..6eb2fe4e 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -371,7 +371,7 @@ class VisitRepositoryTest extends DatabaseTestCase #[Test] public function countOrphanVisitsReturnsExpectedResult(): void { - $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'longUrl'])); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'https://longUrl'])); $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, 7); @@ -408,15 +408,15 @@ class VisitRepositoryTest extends DatabaseTestCase #[Test] public function findNonOrphanVisitsReturnsExpectedResult(): void { - $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => '1'])); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'https://1'])); $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, 7); - $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => '2'])); + $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'https://2'])); $this->getEntityManager()->persist($shortUrl2); $this->createVisitsForShortUrl($shortUrl2, 4); - $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => '3'])); + $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'https://3'])); $this->getEntityManager()->persist($shortUrl3); $this->createVisitsForShortUrl($shortUrl3, 10); @@ -475,7 +475,7 @@ class VisitRepositoryTest extends DatabaseTestCase ?ApiKey $apiKey = null, ): array { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ - ShortUrlInputFilter::LONG_URL => 'longUrl', + ShortUrlInputFilter::LONG_URL => 'https://longUrl', ShortUrlInputFilter::TAGS => $tags, ShortUrlInputFilter::API_KEY => $apiKey, ]), $this->relationResolver); @@ -489,7 +489,7 @@ class VisitRepositoryTest extends DatabaseTestCase $shortUrlWithDomain = ShortUrl::create(ShortUrlCreation::fromRawData([ 'customSlug' => $shortCode, 'domain' => $domain, - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', ])); $this->getEntityManager()->persist($shortUrlWithDomain); $this->createVisitsForShortUrl($shortUrlWithDomain, 3); From 1d155298c1a80bf14c1758a126119882e38cc0f2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 25 Mar 2023 11:07:19 +0100 Subject: [PATCH 4/5] Fix API tests --- module/Core/src/ShortUrl/Entity/ShortUrl.php | 1 - .../src/ShortUrl/Model/Validation/ShortUrlInputFilter.php | 2 +- .../test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php | 2 +- module/Core/test-db/Tag/Repository/TagRepositoryTest.php | 7 +++---- module/Rest/test-api/Action/EditShortUrlTest.php | 2 +- 5 files changed, 6 insertions(+), 8 deletions(-) diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index 7f5cd406..e5646bd4 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -33,7 +33,6 @@ use function Shlinkio\Shlink\Core\enumValues; use function Shlinkio\Shlink\Core\generateRandomShortCode; use function Shlinkio\Shlink\Core\normalizeDate; use function Shlinkio\Shlink\Core\normalizeOptionalDate; -use function str_contains; class ShortUrl extends AbstractEntity { diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php index 762a9230..af7e8986 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php @@ -68,7 +68,7 @@ class ShortUrlInputFilter extends InputFilter $deviceLongUrlsInput = $this->createInput(self::DEVICE_LONG_URLS, false); $deviceLongUrlsInput->getValidatorChain()->attach( - new DeviceLongUrlsValidator($this->longUrlValidators(allowNull: ! $requireLongUrl)) + new DeviceLongUrlsValidator($this->longUrlValidators(allowNull: ! $requireLongUrl)), ); $this->add($deviceLongUrlsInput); diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php index 441927a1..074acdd4 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php @@ -210,7 +210,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase self::assertSame( $shortUrl, $this->repo->findOneMatching(ShortUrlCreation::fromRawData( - ['validSince' => $start, 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar']] + ['validSince' => $start, 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar']], )), ); self::assertSame( diff --git a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php index 437b1de4..6cccf199 100644 --- a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php @@ -240,10 +240,9 @@ class TagRepositoryTest extends DatabaseTestCase [$firstUrlTags, $secondUrlTags] = array_chunk($names, 3); - $shortUrl = ShortUrl::create( - ShortUrlCreation::fromRawData(['apiKey' => $authorApiKey, 'longUrl' => 'https://longUrl', 'tags' => $firstUrlTags]), - $this->relationResolver, - ); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData( + ['apiKey' => $authorApiKey, 'longUrl' => 'https://longUrl', 'tags' => $firstUrlTags], + ), $this->relationResolver); $this->getEntityManager()->persist($shortUrl); $shortUrl2 = ShortUrl::create( diff --git a/module/Rest/test-api/Action/EditShortUrlTest.php b/module/Rest/test-api/Action/EditShortUrlTest.php index 8ac26a27..22833970 100644 --- a/module/Rest/test-api/Action/EditShortUrlTest.php +++ b/module/Rest/test-api/Action/EditShortUrlTest.php @@ -96,7 +96,7 @@ class EditShortUrlTest extends ApiTestCase public static function provideLongUrls(): iterable { yield 'valid URL' => ['https://shlink.io', self::STATUS_OK, null]; - yield 'invalid URL' => ['htt:foo', self::STATUS_BAD_REQUEST, 'INVALID_URL']; + yield 'invalid URL' => ['http://foo', self::STATUS_BAD_REQUEST, 'INVALID_URL']; } #[Test, DataProvider('provideInvalidUrls')] From 71807e698cfb208a5f1df088fad3d54e05f72d48 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 25 Mar 2023 11:16:28 +0100 Subject: [PATCH 5/5] Update changelog --- CHANGELOG.md | 17 +++++++++++++++++ .../ShortUrl/Model/ShortUrlCreationTest.php | 6 ++++++ 2 files changed, 23 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e288f92..8f89cd31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ 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). +## [Unreleased] +### Added +* *Nothing* + +### Changed +* *Nothing* + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* [#1715](https://github.com/shlinkio/shlink/issues/1715) Fix short URL creation/edition allowing long URLs without schema. Now a validation error is thrown. + + ## [3.5.2] - 2023-02-16 ### Added * *Nothing* diff --git a/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php b/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php index adecf9e9..bb82ee8e 100644 --- a/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php +++ b/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php @@ -99,6 +99,12 @@ class ShortUrlCreationTest extends TestCase DeviceType::IOS->value => ' ', ], ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => 'https://foo', + ShortUrlInputFilter::DEVICE_LONG_URLS => [ + DeviceType::ANDROID->value => 'missing_schema', + ], + ]]; yield [[ ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::DEVICE_LONG_URLS => [