shlinkio/shlink
1
0
Fork 0
mirror of https://github.com/shlinkio/shlink.git synced 2025-12-10 11:05:50 -06:00
Code Issues Packages Projects Releases 146 Wiki Activity

Make RedirectCondition->matchValue nullable

Browse Source
This commit is contained in:
Alejandro Celaya 2025-07-22 08:20:21 +02:00
parent 56df880a93
commit 5b5d0aae49
10 changed files with 66 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
docs/swagger/definitions/SetShortUrlRedirectRule.json
View File

@ -31,7 +31,7 @@
"type": ["string", "null"]
},
"matchValue": {
"type": "string"
"type": ["string", "null"]
}
}
}

1
module/Core/config/entities-mappings/Shlinkio.Shlink.Core.RedirectRule.Entity.RedirectCondition.php
View File

@ -39,5 +39,6 @@ return static function (ClassMetadata $metadata, array $emConfig): void {
fieldWithUtf8Charset($builder->createField('matchValue', Types::STRING), $emConfig)
->columnName('match_value')
->length(512)
->nullable()
->build();
};

26
module/Core/migrations/Version20250722060208.php Normal file
View File

@ -0,0 +1,26 @@
<?php
declare(strict_types=1);
namespace ShlinkMigrations;
use Doctrine\DBAL\Platforms\MySQLPlatform;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;
/**
* Make the redirect_condition match_value column nullable, so that we can support new valueless-query-param and
* any-value-query-param conditions consistently.
*/
final class Version20250722060208 extends AbstractMigration
{
public function up(Schema $schema): void
{
$schema->getTable('redirect_conditions')->getColumn('match_value')->setNotnull(false);
}
public function isTransactional(): bool
{
return ! ($this->connection->getDatabasePlatform() instanceof MySQLPlatform);
}
}

22
module/Core/src/RedirectRule/Entity/RedirectCondition.php
View File

@ -26,7 +26,7 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable
{
private function __construct(
public readonly RedirectConditionType $type,
private readonly string $matchValue,
private readonly string|null $matchValue = null,
private readonly string|null $matchKey = null,
) {
}
@ -38,12 +38,12 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable
public static function forAnyValueQueryParam(string $param): self
{
return new self(RedirectConditionType::ANY_VALUE_QUERY_PARAM, $param);
return new self(RedirectConditionType::ANY_VALUE_QUERY_PARAM, matchKey: $param);
}
public static function forValuelessQueryParam(string $param): self
{
return new self(RedirectConditionType::VALUELESS_QUERY_PARAM, $param);
return new self(RedirectConditionType::VALUELESS_QUERY_PARAM, matchKey: $param);
}
public static function forLanguage(string $language): self
@ -131,19 +131,19 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable
private function matchesValuelessQueryParam(ServerRequestInterface $request): bool
{
$query = $request->getQueryParams();
return array_key_exists($this->matchValue, $query) && empty($query[$this->matchValue]);
return $this->matchKey !== null && array_key_exists($this->matchKey, $query) && empty($query[$this->matchKey]);
}
private function matchesAnyValueQueryParam(ServerRequestInterface $request): bool
{
$query = $request->getQueryParams();
return array_key_exists($this->matchValue, $query);
return $this->matchKey !== null && array_key_exists($this->matchKey, $query);
}
private function matchesLanguage(ServerRequestInterface $request): bool
{
$acceptLanguage = trim($request->getHeaderLine('Accept-Language'));
if ($acceptLanguage === '' || $acceptLanguage === '*') {
if ($acceptLanguage === '' || $acceptLanguage === '*' || $this->matchValue === null) {
return false;
}
@ -173,13 +173,17 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable
private function matchesRemoteIpAddress(ServerRequestInterface $request): bool
{
$remoteAddress = ipAddressFromRequest($request);
return $remoteAddress !== null && IpAddressUtils::ipAddressMatchesGroups($remoteAddress, [$this->matchValue]);
return (
$this->matchValue !== null
&& $remoteAddress !== null
&& IpAddressUtils::ipAddressMatchesGroups($remoteAddress, [$this->matchValue])
);
}
private function matchesGeolocationCountryCode(ServerRequestInterface $request): bool
{
$geolocation = geolocationFromRequest($request);
if ($geolocation === null) {
if ($geolocation === null || $this->matchValue === null) {
return false;
}
@ -189,7 +193,7 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable
private function matchesGeolocationCityName(ServerRequestInterface $request): bool
{
$geolocation = geolocationFromRequest($request);
if ($geolocation === null) {
if ($geolocation === null || $this->matchValue === null) {
return false;
}

6
module/Core/src/RedirectRule/Model/RedirectConditionType.php
View File

@ -31,11 +31,7 @@ enum RedirectConditionType: string
// RedirectConditionType::LANGUAGE => TODO Validate at least format,
RedirectConditionType::IP_ADDRESS => IpAddressUtils::isStaticIpCidrOrWildcard($value),
RedirectConditionType::GEOLOCATION_COUNTRY_CODE => contains($value, ISO_COUNTRY_CODES),
RedirectConditionType::QUERY_PARAM,
RedirectConditionType::ANY_VALUE_QUERY_PARAM,
RedirectConditionType::VALUELESS_QUERY_PARAM => $value !== '',
// FIXME We should at least validate the value is not empty
// default => $value !== '',
RedirectConditionType::QUERY_PARAM => $value !== '',
default => true,
};
}

2
module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php
View File

@ -75,7 +75,7 @@ class RedirectRulesInputFilter extends InputFilter
]));
$redirectConditionInputFilter->add($type);
$value = InputFactory::basic(self::CONDITION_MATCH_VALUE, required: true);
$value = InputFactory::basic(self::CONDITION_MATCH_VALUE, required: true)->setAllowEmpty(true);
$value->getValidatorChain()->attach(new Callback(
function (string $value, array $context): bool {
$conditionType = RedirectConditionType::tryFrom($context[self::CONDITION_TYPE]);

4
module/Core/test/RedirectRule/Model/RedirectConditionTypeTest.php
View File

@ -14,9 +14,9 @@ class RedirectConditionTypeTest extends TestCase
#[Test]
#[TestWith([RedirectConditionType::QUERY_PARAM, '', false])]
#[TestWith([RedirectConditionType::QUERY_PARAM, 'foo', true])]
#[TestWith([RedirectConditionType::ANY_VALUE_QUERY_PARAM, '', false])]
#[TestWith([RedirectConditionType::ANY_VALUE_QUERY_PARAM, '', true])]
#[TestWith([RedirectConditionType::ANY_VALUE_QUERY_PARAM, 'foo', true])]
#[TestWith([RedirectConditionType::VALUELESS_QUERY_PARAM, '', false])]
#[TestWith([RedirectConditionType::VALUELESS_QUERY_PARAM, '', true])]
#[TestWith([RedirectConditionType::VALUELESS_QUERY_PARAM, 'foo', true])]
public function isValidFailsForEmptyQueryParams(
RedirectConditionType $conditionType,

17
module/Rest/test-api/Action/ListRedirectRulesTest.php
View File

@ -17,11 +17,6 @@ class ListRedirectRulesTest extends ApiTestCase
'matchKey' => null,
'matchValue' => 'en',
];
private const array QUERY_FOO_BAR_CONDITION = [
'type' => 'query-param',
'matchKey' => 'foo',
'matchValue' => 'bar',
];
#[Test]
public function errorIsReturnedWhenInvalidUrlIsFetched(): void
@ -45,7 +40,11 @@ class ListRedirectRulesTest extends ApiTestCase
'priority' => 1,
'conditions' => [
self::LANGUAGE_EN_CONDITION,
self::QUERY_FOO_BAR_CONDITION,
[
'type' => 'any-value-query-param',
'matchKey' => 'foo',
'matchValue' => null,
],
],
],
[
@ -57,7 +56,11 @@ class ListRedirectRulesTest extends ApiTestCase
'matchKey' => 'hello',
'matchValue' => 'world',
],
self::QUERY_FOO_BAR_CONDITION,
[
'type' => 'query-param',
'matchKey' => 'foo',
'matchValue' => 'bar',
],
],
],
[

17
module/Rest/test-api/Action/SetRedirectRulesTest.php
View File

@ -18,11 +18,6 @@ class SetRedirectRulesTest extends ApiTestCase
'matchKey' => null,
'matchValue' => 'en',
];
private const array QUERY_FOO_BAR_CONDITION = [
'type' => 'query-param',
'matchKey' => 'foo',
'matchValue' => 'bar',
];
#[Test]
public function errorIsReturnedWhenInvalidUrlIsProvided(): void
@ -132,7 +127,11 @@ class SetRedirectRulesTest extends ApiTestCase
'priority' => 1,
'conditions' => [
self::LANGUAGE_EN_CONDITION,
self::QUERY_FOO_BAR_CONDITION,
[
'type' => 'any-value-query-param',
'matchKey' => 'foo',
'matchValue' => null,
],
],
],
[
@ -144,7 +143,11 @@ class SetRedirectRulesTest extends ApiTestCase
'matchKey' => 'hello',
'matchValue' => 'world',
],
self::QUERY_FOO_BAR_CONDITION,
[
'type' => 'query-param',
'matchKey' => 'foo',
'matchValue' => 'bar',
],
],
],
]])]

2
module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php
View File

@ -41,7 +41,7 @@ class ShortUrlRedirectRulesFixture extends AbstractFixture implements DependentF
priority: 1,
longUrl: 'https://example.com/english-and-foo-query',
conditions: new ArrayCollection(
[RedirectCondition::forLanguage('en'), RedirectCondition::forQueryParam('foo', 'bar')],
[RedirectCondition::forLanguage('en'), RedirectCondition::forAnyValueQueryParam('foo')],
),
);
$manager->persist($englishAndFooQueryRule);