From 1ed6458b39dad70b41a76fdc46f4bec195c0caea Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 2 Oct 2021 09:32:04 +0200 Subject: [PATCH 1/5] Added forwardQuery property in short URLs, that determines if the query should be forwarded to the long URL --- data/migrations/Version20211002072605.php | 26 +++++++++++++++++++ .../Shlinkio.Shlink.Core.Entity.ShortUrl.php | 5 ++++ module/Core/src/Entity/ShortUrl.php | 6 +++++ .../Helper/ShortUrlRedirectionBuilder.php | 3 ++- 4 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 data/migrations/Version20211002072605.php diff --git a/data/migrations/Version20211002072605.php b/data/migrations/Version20211002072605.php new file mode 100644 index 00000000..5f8db987 --- /dev/null +++ b/data/migrations/Version20211002072605.php @@ -0,0 +1,26 @@ +getTable('short_urls'); + $this->skipIf($shortUrls->hasColumn('forward_query')); + $shortUrls->addColumn('forward_query', Types::BOOLEAN, ['default' => true]); + } + + public function down(Schema $schema): void + { + $shortUrls = $schema->getTable('short_urls'); + $this->skipIf(! $shortUrls->hasColumn('forward_query')); + $shortUrls->dropColumn('forward_query'); + } +} diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php index a9269d36..83fd7e79 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php @@ -100,4 +100,9 @@ return static function (ClassMetadata $metadata, array $emConfig): void { ->columnName('crawlable') ->option('default', false) ->build(); + + $builder->createField('forwardQuery', Types::BOOLEAN) + ->columnName('forward_query') + ->option('default', true) + ->build(); }; diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index 78527115..210310f5 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -43,6 +43,7 @@ class ShortUrl extends AbstractEntity private ?string $title = null; private bool $titleWasAutoResolved = false; private bool $crawlable = false; + private bool $forwardQuery = true; private function __construct() { @@ -207,6 +208,11 @@ class ShortUrl extends AbstractEntity return $this->crawlable; } + public function forwardQuery(): bool + { + return $this->forwardQuery; + } + public function update( ShortUrlEdit $shortUrlEdit, ?ShortUrlRelationResolverInterface $relationResolver = null, diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php b/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php index f83a7eb9..3251922d 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php @@ -21,9 +21,10 @@ class ShortUrlRedirectionBuilder implements ShortUrlRedirectionBuilderInterface public function buildShortUrlRedirect(ShortUrl $shortUrl, array $currentQuery, ?string $extraPath = null): string { $uri = Uri::createFromString($shortUrl->getLongUrl()); + $shouldForwardQuery = $shortUrl->forwardQuery(); return $uri - ->withQuery($this->resolveQuery($uri, $currentQuery)) + ->withQuery($shouldForwardQuery ? $this->resolveQuery($uri, $currentQuery) : $uri->getQuery()) ->withPath($this->resolvePath($uri, $extraPath)) ->__toString(); } From 8212d3c540879e2458a50a6ee13b228a0341c2a6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 2 Oct 2021 10:02:43 +0200 Subject: [PATCH 2/5] Allowed to set and update the forwardQuery param on short URLs --- docs/swagger/definitions/ShortUrlEdition.json | 48 +++++++++++ docs/swagger/paths/v1_short-urls.json | 82 +++++++------------ .../paths/v1_short-urls_{shortCode}.json | 43 +--------- module/Core/src/Entity/ShortUrl.php | 4 + module/Core/src/Model/ShortUrlEdit.php | 14 ++++ module/Core/src/Model/ShortUrlMeta.php | 7 ++ .../src/Validation/ShortUrlInputFilter.php | 6 +- 7 files changed, 106 insertions(+), 98 deletions(-) create mode 100644 docs/swagger/definitions/ShortUrlEdition.json diff --git a/docs/swagger/definitions/ShortUrlEdition.json b/docs/swagger/definitions/ShortUrlEdition.json new file mode 100644 index 00000000..94ef6135 --- /dev/null +++ b/docs/swagger/definitions/ShortUrlEdition.json @@ -0,0 +1,48 @@ +{ + "type": "object", + "properties": { + "longUrl": { + "description": "The long URL this short URL will redirect to", + "type": "string" + }, + "validSince": { + "description": "The date (in ISO-8601 format) from which this short code will be valid", + "type": "string", + "nullable": true + }, + "validUntil": { + "description": "The date (in ISO-8601 format) until which this short code will be valid", + "type": "string", + "nullable": true + }, + "maxVisits": { + "description": "The maximum number of allowed visits for this short code", + "type": "number", + "nullable": true + }, + "validateUrl": { + "description": "Tells if the long URL (if provided) should or should not be validated as a reachable URL. If not provided, it will fall back to app-level config", + "type": "boolean" + }, + "tags": { + "type": "array", + "items": { + "type": "string" + }, + "description": "The list of tags to set to the short URL." + }, + "title": { + "type": "string", + "description": "A descriptive title of the short URL.", + "nullable": true + }, + "crawlable": { + "type": "boolean", + "description": "Tells if this URL will be included as 'Allow' in Shlink's robots.txt." + }, + "forwardQuery": { + "type": "boolean", + "description": "Tells if the query params should be forwarded from the short URL to the long one, as explained in [the docs](https://shlink.io/documentation/some-features/#query-params-forwarding)." + } + } +} diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index 8cf22045..a4643058 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -225,63 +225,37 @@ "content": { "application/json": { "schema": { - "type": "object", - "required": [ - "longUrl" - ], - "properties": { - "longUrl": { - "description": "The URL to parse", - "type": "string" + "allOf": [ + { + "$ref": "../definitions/ShortUrlEdition.json" }, - "tags": { - "description": "The URL to parse", - "type": "array", - "items": { - "type": "string" + { + "type": "object", + "required": ["longUrl"], + "properties": { + "customSlug": { + "description": "A unique custom slug to be used instead of the generated short code", + "type": "string" + }, + "findIfExists": { + "description": "Will force existing matching URL to be returned if found, instead of creating a new one", + "type": "boolean" + }, + "domain": { + "description": "The domain to which the short URL will be attached", + "type": "string" + }, + "shortCodeLength": { + "description": "The length for generated short code. It has to be at least 4 and defaults to 5. It will be ignored when customSlug is provided", + "type": "number" + }, + "validateUrl": { + "description": "Tells if the long URL should or should not be validated as a reachable URL. If not provided, it will fall back to app-level config", + "type": "boolean" + } } - }, - "validSince": { - "description": "The date (in ISO-8601 format) from which this short code will be valid", - "type": "string" - }, - "validUntil": { - "description": "The date (in ISO-8601 format) until which this short code will be valid", - "type": "string" - }, - "customSlug": { - "description": "A unique custom slug to be used instead of the generated short code", - "type": "string" - }, - "maxVisits": { - "description": "The maximum number of allowed visits for this short code", - "type": "number" - }, - "findIfExists": { - "description": "Will force existing matching URL to be returned if found, instead of creating a new one", - "type": "boolean" - }, - "domain": { - "description": "The domain to which the short URL will be attached", - "type": "string" - }, - "shortCodeLength": { - "description": "The length for generated short code. It has to be at least 4 and defaults to 5. It will be ignored when customSlug is provided", - "type": "number" - }, - "validateUrl": { - "description": "Tells if the long URL should or should not be validated as a reachable URL. If not provided, it will fall back to app-level config", - "type": "boolean" - }, - "title": { - "type": "string", - "description": "A descriptive title of the short URL." - }, - "crawlable": { - "type": "boolean", - "description": "Tells if this URL will be included as 'Allow' in Shlink's robots.txt." } - } + ] } } } diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}.json b/docs/swagger/paths/v1_short-urls_{shortCode}.json index 8691c0b5..e37df965 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}.json @@ -112,48 +112,7 @@ "content": { "application/json": { "schema": { - "type": "object", - "properties": { - "longUrl": { - "description": "The long URL this short URL will redirect to", - "type": "string" - }, - "validSince": { - "description": "The date (in ISO-8601 format) from which this short code will be valid", - "type": "string", - "nullable": true - }, - "validUntil": { - "description": "The date (in ISO-8601 format) until which this short code will be valid", - "type": "string", - "nullable": true - }, - "maxVisits": { - "description": "The maximum number of allowed visits for this short code", - "type": "number", - "nullable": true - }, - "validateUrl": { - "description": "Tells if the long URL (if provided) should or should not be validated as a reachable URL. If not provided, it will fall back to app-level config", - "type": "boolean" - }, - "tags": { - "type": "array", - "items": { - "type": "string" - }, - "description": "The list of tags to set to the short URL." - }, - "title": { - "type": "string", - "description": "A descriptive title of the short URL.", - "nullable": true - }, - "crawlable": { - "type": "boolean", - "description": "Tells if this URL will be included as 'Allow' in Shlink's robots.txt." - } - } + "$ref": "../definitions/ShortUrlEdition.json" } } } diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index 210310f5..9fff1509 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -81,6 +81,7 @@ class ShortUrl extends AbstractEntity $instance->title = $meta->getTitle(); $instance->titleWasAutoResolved = $meta->titleWasAutoResolved(); $instance->crawlable = $meta->isCrawlable(); + $instance->forwardQuery = $meta->forwardQuery(); return $instance; } @@ -244,6 +245,9 @@ class ShortUrl extends AbstractEntity $this->title = $shortUrlEdit->title(); $this->titleWasAutoResolved = $shortUrlEdit->titleWasAutoResolved(); } + if ($shortUrlEdit->forwardQueryWasProvided()) { + $this->forwardQuery = $shortUrlEdit->forwardQuery(); + } } /** diff --git a/module/Core/src/Model/ShortUrlEdit.php b/module/Core/src/Model/ShortUrlEdit.php index 32c1ca1e..8e187cf6 100644 --- a/module/Core/src/Model/ShortUrlEdit.php +++ b/module/Core/src/Model/ShortUrlEdit.php @@ -32,6 +32,8 @@ final class ShortUrlEdit implements TitleResolutionModelInterface private ?bool $validateUrl = null; private bool $crawlablePropWasProvided = false; private bool $crawlable = false; + private bool $forwardQueryPropWasProvided = false; + private bool $forwardQuery = true; private function __construct() { @@ -64,6 +66,7 @@ final class ShortUrlEdit implements TitleResolutionModelInterface $this->tagsPropWasProvided = array_key_exists(ShortUrlInputFilter::TAGS, $data); $this->titlePropWasProvided = array_key_exists(ShortUrlInputFilter::TITLE, $data); $this->crawlablePropWasProvided = array_key_exists(ShortUrlInputFilter::CRAWLABLE, $data); + $this->forwardQueryPropWasProvided = array_key_exists(ShortUrlInputFilter::FORWARD_QUERY, $data); $this->longUrl = $inputFilter->getValue(ShortUrlInputFilter::LONG_URL); $this->validSince = parseDateField($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE)); @@ -73,6 +76,7 @@ final class ShortUrlEdit implements TitleResolutionModelInterface $this->tags = $inputFilter->getValue(ShortUrlInputFilter::TAGS); $this->title = $inputFilter->getValue(ShortUrlInputFilter::TITLE); $this->crawlable = $inputFilter->getValue(ShortUrlInputFilter::CRAWLABLE); + $this->forwardQuery = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::FORWARD_QUERY) ?? true; } public function longUrl(): ?string @@ -176,4 +180,14 @@ final class ShortUrlEdit implements TitleResolutionModelInterface { return $this->crawlablePropWasProvided; } + + public function forwardQuery(): bool + { + return $this->forwardQuery; + } + + public function forwardQueryWasProvided(): bool + { + return $this->forwardQueryPropWasProvided; + } } diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php index 0c982f17..74390281 100644 --- a/module/Core/src/Model/ShortUrlMeta.php +++ b/module/Core/src/Model/ShortUrlMeta.php @@ -32,6 +32,7 @@ final class ShortUrlMeta implements TitleResolutionModelInterface private ?string $title = null; private bool $titleWasAutoResolved = false; private bool $crawlable = false; + private bool $forwardQuery = true; private function __construct() { @@ -82,6 +83,7 @@ final class ShortUrlMeta implements TitleResolutionModelInterface $this->tags = $inputFilter->getValue(ShortUrlInputFilter::TAGS); $this->title = $inputFilter->getValue(ShortUrlInputFilter::TITLE); $this->crawlable = $inputFilter->getValue(ShortUrlInputFilter::CRAWLABLE); + $this->forwardQuery = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::FORWARD_QUERY) ?? true; } public function getLongUrl(): string @@ -195,4 +197,9 @@ final class ShortUrlMeta implements TitleResolutionModelInterface { return $this->crawlable; } + + public function forwardQuery(): bool + { + return $this->forwardQuery; + } } diff --git a/module/Core/src/Validation/ShortUrlInputFilter.php b/module/Core/src/Validation/ShortUrlInputFilter.php index fd5a92e7..47f6f8ac 100644 --- a/module/Core/src/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/Validation/ShortUrlInputFilter.php @@ -33,6 +33,7 @@ class ShortUrlInputFilter extends InputFilter public const TAGS = 'tags'; public const TITLE = 'title'; public const CRAWLABLE = 'crawlable'; + public const FORWARD_QUERY = 'forwardQuery'; private function __construct(array $data, bool $requireLongUrl) { @@ -89,9 +90,10 @@ class ShortUrlInputFilter extends InputFilter $this->add($this->createBooleanInput(self::FIND_IF_EXISTS, false)); - // This cannot be defined as a boolean input because it can actually have 3 values, true, false and null. - // Defining it as boolean will make null fall back to false, which is not the desired behavior. + // These cannot be defined as a boolean inputs, because they can actually have 3 values: true, false and null. + // Defining them as boolean will make null fall back to false, which is not the desired behavior. $this->add($this->createInput(self::VALIDATE_URL, false)); + $this->add($this->createInput(self::FORWARD_QUERY, false)); $domain = $this->createInput(self::DOMAIN, false); $domain->getValidatorChain()->attach(new Validation\HostAndPortValidator()); From 74a08b86cefaf31519a09c9169e340a9833a9ab2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 2 Oct 2021 10:16:56 +0200 Subject: [PATCH 3/5] Estended ShortUrlRedirectionBuilderTest covering short URLS withput query forwarding --- .../Helper/ShortUrlRedirectionBuilderTest.php | 76 +++++++++++++++---- 1 file changed, 62 insertions(+), 14 deletions(-) diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php index 985a3cea..829d77ea 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php @@ -6,27 +6,34 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl\Helper; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Options\TrackingOptions; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlRedirectionBuilder; class ShortUrlRedirectionBuilderTest extends TestCase { private ShortUrlRedirectionBuilder $redirectionBuilder; - private TrackingOptions $trackingOptions; protected function setUp(): void { - $this->trackingOptions = new TrackingOptions(['disable_track_param' => 'foobar']); - $this->redirectionBuilder = new ShortUrlRedirectionBuilder($this->trackingOptions); + $trackingOptions = new TrackingOptions(['disable_track_param' => 'foobar']); + $this->redirectionBuilder = new ShortUrlRedirectionBuilder($trackingOptions); } /** * @test * @dataProvider provideData */ - public function buildShortUrlRedirectBuildsExpectedUrl(string $expectedUrl, array $query, ?string $extraPath): void - { - $shortUrl = ShortUrl::withLongUrl('https://domain.com/foo/bar?some=thing'); + public function buildShortUrlRedirectBuildsExpectedUrl( + string $expectedUrl, + array $query, + ?string $extraPath, + ?bool $forwardQuery, + ): void { + $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ + 'longUrl' => 'https://domain.com/foo/bar?some=thing', + 'forwardQuery' => $forwardQuery, + ])); $result = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $query, $extraPath); self::assertEquals($expectedUrl, $result); @@ -34,18 +41,59 @@ class ShortUrlRedirectionBuilderTest extends TestCase public function provideData(): iterable { - yield ['https://domain.com/foo/bar?some=thing', [], null]; - yield ['https://domain.com/foo/bar?some=thing&else', ['else' => null], null]; - yield ['https://domain.com/foo/bar?some=thing&foo=bar', ['foo' => 'bar'], null]; - yield ['https://domain.com/foo/bar?some=thing&123=foo', ['123' => 'foo'], null]; - yield ['https://domain.com/foo/bar?some=thing&456=foo', [456 => 'foo'], null]; - yield ['https://domain.com/foo/bar?some=overwritten&foo=bar', ['foo' => 'bar', 'some' => 'overwritten'], null]; - yield ['https://domain.com/foo/bar?some=overwritten', ['foobar' => 'notrack', 'some' => 'overwritten'], null]; - yield ['https://domain.com/foo/bar/something/else-baz?some=thing', [], '/something/else-baz']; + yield ['https://domain.com/foo/bar?some=thing', [], null, true]; + yield ['https://domain.com/foo/bar?some=thing', [], null, null]; + yield ['https://domain.com/foo/bar?some=thing', [], null, false]; + yield ['https://domain.com/foo/bar?some=thing&else', ['else' => null], null, true]; + yield ['https://domain.com/foo/bar?some=thing&foo=bar', ['foo' => 'bar'], null, true]; + yield ['https://domain.com/foo/bar?some=thing&foo=bar', ['foo' => 'bar'], null, null]; + yield ['https://domain.com/foo/bar?some=thing', ['foo' => 'bar'], null, false]; + yield ['https://domain.com/foo/bar?some=thing&123=foo', ['123' => 'foo'], null, true]; + yield ['https://domain.com/foo/bar?some=thing&456=foo', [456 => 'foo'], null, true]; + yield ['https://domain.com/foo/bar?some=thing&456=foo', [456 => 'foo'], null, null]; + yield ['https://domain.com/foo/bar?some=thing', [456 => 'foo'], null, false]; + yield [ + 'https://domain.com/foo/bar?some=overwritten&foo=bar', + ['foo' => 'bar', 'some' => 'overwritten'], + null, + true, + ]; + yield [ + 'https://domain.com/foo/bar?some=overwritten', + ['foobar' => 'notrack', 'some' => 'overwritten'], + null, + true, + ]; + yield [ + 'https://domain.com/foo/bar?some=overwritten', + ['foobar' => 'notrack', 'some' => 'overwritten'], + null, + null, + ]; + yield [ + 'https://domain.com/foo/bar?some=thing', + ['foobar' => 'notrack', 'some' => 'overwritten'], + null, + false, + ]; + yield ['https://domain.com/foo/bar/something/else-baz?some=thing', [], '/something/else-baz', true]; yield [ 'https://domain.com/foo/bar/something/else-baz?some=thing&hello=world', ['hello' => 'world'], '/something/else-baz', + true, + ]; + yield [ + 'https://domain.com/foo/bar/something/else-baz?some=thing&hello=world', + ['hello' => 'world'], + '/something/else-baz', + null, + ]; + yield [ + 'https://domain.com/foo/bar/something/else-baz?some=thing', + ['hello' => 'world'], + '/something/else-baz', + false, ]; } } From e21f9dd1fbb13ebabbc695f62239c7e0b37871f9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 2 Oct 2021 10:31:23 +0200 Subject: [PATCH 4/5] Added forwardQuery prop to the SHortUrl serialization --- docs/swagger/definitions/ShortUrl.json | 17 +++++++++++++++++ .../Transformer/ShortUrlDataTransformer.php | 1 + .../Mercure/MercureUpdatesGeneratorTest.php | 1 + .../Rest/test-api/Action/ListShortUrlsTest.php | 6 ++++++ .../Rest/test-api/Fixtures/ShortUrlsFixture.php | 10 +++++++--- 5 files changed, 32 insertions(+), 3 deletions(-) diff --git a/docs/swagger/definitions/ShortUrl.json b/docs/swagger/definitions/ShortUrl.json index b2ffd3f6..a5dee481 100644 --- a/docs/swagger/definitions/ShortUrl.json +++ b/docs/swagger/definitions/ShortUrl.json @@ -1,5 +1,18 @@ { "type": "object", + "required": [ + "shortCode", + "shortUrl", + "longUrl", + "dateCreated", + "visitsCount", + "tags", + "meta", + "domain", + "title", + "crawlable", + "forwardQuery" + ], "properties": { "shortCode": { "type": "string", @@ -45,6 +58,10 @@ "crawlable": { "type": "boolean", "description": "Tells if this URL will be included as 'Allow' in Shlink's robots.txt." + }, + "forwardQuery": { + "type": "boolean", + "description": "Tells if this URL will forward the query params to the long URL when visited, as explained in [the docs](https://shlink.io/documentation/some-features/#query-params-forwarding)." } } } diff --git a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php index 61049626..554f9894 100644 --- a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php +++ b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php @@ -33,6 +33,7 @@ class ShortUrlDataTransformer implements DataTransformerInterface 'domain' => $shortUrl->getDomain(), 'title' => $shortUrl->title(), 'crawlable' => $shortUrl->crawlable(), + 'forwardQuery' => $shortUrl->forwardQuery(), ]; } diff --git a/module/Core/test/Mercure/MercureUpdatesGeneratorTest.php b/module/Core/test/Mercure/MercureUpdatesGeneratorTest.php index 86d1b3d5..14378b4f 100644 --- a/module/Core/test/Mercure/MercureUpdatesGeneratorTest.php +++ b/module/Core/test/Mercure/MercureUpdatesGeneratorTest.php @@ -60,6 +60,7 @@ class MercureUpdatesGeneratorTest extends TestCase 'domain' => null, 'title' => $title, 'crawlable' => false, + 'forwardQuery' => true, ], 'visit' => [ 'referer' => '', diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index 95d77dc6..fcc07719 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -27,6 +27,7 @@ class ListShortUrlsTest extends ApiTestCase 'domain' => null, 'title' => 'My cool title', 'crawlable' => true, + 'forwardQuery' => true, ]; private const SHORT_URL_DOCS = [ 'shortCode' => 'ghi789', @@ -43,6 +44,7 @@ class ListShortUrlsTest extends ApiTestCase 'domain' => null, 'title' => null, 'crawlable' => false, + 'forwardQuery' => true, ]; private const SHORT_URL_CUSTOM_SLUG_AND_DOMAIN = [ 'shortCode' => 'custom-with-domain', @@ -59,6 +61,7 @@ class ListShortUrlsTest extends ApiTestCase 'domain' => 'some-domain.com', 'title' => null, 'crawlable' => false, + 'forwardQuery' => true, ]; private const SHORT_URL_META = [ 'shortCode' => 'def456', @@ -77,6 +80,7 @@ class ListShortUrlsTest extends ApiTestCase 'domain' => null, 'title' => null, 'crawlable' => false, + 'forwardQuery' => true, ]; private const SHORT_URL_CUSTOM_SLUG = [ 'shortCode' => 'custom', @@ -93,6 +97,7 @@ class ListShortUrlsTest extends ApiTestCase 'domain' => null, 'title' => null, 'crawlable' => false, + 'forwardQuery' => false, ]; private const SHORT_URL_CUSTOM_DOMAIN = [ 'shortCode' => 'ghi789', @@ -111,6 +116,7 @@ class ListShortUrlsTest extends ApiTestCase 'domain' => 'example.com', 'title' => null, 'crawlable' => false, + 'forwardQuery' => true, ]; /** diff --git a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php index ccc83525..9510f8ed 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php @@ -51,9 +51,13 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf ]), $relationResolver), '2019-01-01 00:00:10'); $manager->persist($defShortUrl); - $customShortUrl = $this->setShortUrlDate(ShortUrl::fromMeta(ShortUrlMeta::fromRawData( - ['customSlug' => 'custom', 'maxVisits' => 2, 'apiKey' => $authorApiKey, 'longUrl' => 'https://shlink.io'], - )), '2019-01-01 00:00:20'); + $customShortUrl = $this->setShortUrlDate(ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ + 'customSlug' => 'custom', + 'maxVisits' => 2, + 'apiKey' => $authorApiKey, + 'longUrl' => 'https://shlink.io', + 'forwardQuery' => false, + ])), '2019-01-01 00:00:20'); $manager->persist($customShortUrl); $ghiShortUrl = $this->setShortUrlDate( From 0c95b978b4b3b61d4b5235f379e1f72faab0fe0c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 2 Oct 2021 10:45:00 +0200 Subject: [PATCH 5/5] Added option in CLI to disable query forwarding when creating Short URLs --- CHANGELOG.md | 3 +++ .../Command/ShortUrl/GenerateShortUrlCommand.php | 16 +++++++++++++++- module/Core/src/Util/UrlValidator.php | 2 +- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f15fbbdc..9d253e85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this The config generated with the installing tool still has precedence over the env vars, so it cannot be combined. Either you use the tool, or use env vars. * [#1149](https://github.com/shlinkio/shlink/issues/1149) Allowed to set custom defaults for the QR codes. +* [#1112](https://github.com/shlinkio/shlink/issues/1112) Added new option to define if the query string should be forwarded on a per-short URL basis. + + The new `forwardQuery=true|false` param can be provided during short URL creation or edition, via REST API or CLI command, allowing to override the default behavior which makes the query string to always be forwarded. ### Changed * [#1142](https://github.com/shlinkio/shlink/issues/1142) Replaced `doctrine/cache` package with `symfony/cache`. diff --git a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php index 6ed3e37c..e43b4ec5 100644 --- a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php @@ -104,7 +104,19 @@ class GenerateShortUrlCommand extends BaseCommand 'no-validate-url', null, InputOption::VALUE_NONE, - 'Forces the long URL to not be validated, regardless what is globally configured.', + '[DEPRECATED] Forces the long URL to not be validated, regardless what is globally configured.', + ) + ->addOption( + 'crawlable', + 'r', + InputOption::VALUE_NONE, + 'Tells if this URL will be included as "Allow" in Shlink\'s robots.txt.', + ) + ->addOption( + 'no-forward-query', + 'w', + InputOption::VALUE_NONE, + 'Disables the forwarding of the query string to the long URL, when the new short URL is visited.', ); } @@ -156,6 +168,8 @@ class GenerateShortUrlCommand extends BaseCommand ShortUrlInputFilter::SHORT_CODE_LENGTH => $shortCodeLength, ShortUrlInputFilter::VALIDATE_URL => $doValidateUrl, ShortUrlInputFilter::TAGS => $tags, + ShortUrlInputFilter::CRAWLABLE => $input->getOption('crawlable'), + ShortUrlInputFilter::FORWARD_QUERY => !$input->getOption('no-forward-query'), ])); $io->writeln([ diff --git a/module/Core/src/Util/UrlValidator.php b/module/Core/src/Util/UrlValidator.php index 5b84c5f8..bd0a5cfb 100644 --- a/module/Core/src/Util/UrlValidator.php +++ b/module/Core/src/Util/UrlValidator.php @@ -32,7 +32,7 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface */ public function validateUrl(string $url, ?bool $doValidate): void { - // If the URL validation is not enabled or it was explicitly set to not validate, skip check + // If the URL validation is not enabled, or it was explicitly set to not validate, skip check $doValidate = $doValidate ?? $this->options->isUrlValidationEnabled(); if (! $doValidate) { return;