diff --git a/README.md b/README.md index f771101..70deb2a 100644 --- a/README.md +++ b/README.md @@ -131,6 +131,9 @@ contract raises a dedicated exception, each implementing `HttpQueryException`. | `FilterOperatorNotAllowed` | A comparison uses an operator not allowed for its field. | | `FilterValueNotAllowed` | A compared value falls outside the permitted set or the expected kind. | +A filter exception message names the field, the operator, or the value kind, never a compared value or the raw filter +expression, since a value such as a name or an email may be personal data. + ### Prefix matching with `=sw=` `=sw=` compares the value as a **literal prefix** of the column. It renders as an anchored `LIKE` so a B-tree index on @@ -318,7 +321,8 @@ rendered from the same page. $cursorPage->map(transformation: static fn(array $row): array => ['id' => $row['id']]); ``` -An invalid cursor token raises `CursorIsInvalid` when it is decoded. +An invalid cursor token raises `CursorIsInvalid` when it is decoded. Its message never carries the token, since the +token encodes the ordering key values of the last row, a client name among them when the listing is sorted by name. A cursor token round-trips through the client, so its values are untrusted input. `cursorKey` declares the `ValueKind` every cursor value of a field must match, and `Criteria::fromQuery` then raises `CursorIsInvalid` for a token whose diff --git a/src/Exceptions/CursorIsInvalid.php b/src/Exceptions/CursorIsInvalid.php index 5efcaf8..27432e1 100644 --- a/src/Exceptions/CursorIsInvalid.php +++ b/src/Exceptions/CursorIsInvalid.php @@ -13,12 +13,13 @@ * * A cursor token is produced by the library and must round-trip through its codec. A token that * was truncated, tampered with, or generated elsewhere fails to decode. A token that decodes but - * carries a value outside the kind declared for its cursor key is rejected the same way. + * carries a value outside the kind declared for its cursor key is rejected the same way. The message + * never carries the token, since its decoded ordering keys may hold personal data. */ final class CursorIsInvalid extends InvalidArgumentException implements HttpQueryException { - private const string KIND_MISMATCH = 'Cursor token <%s> does not match the %s kind for cursor key <%s>.'; - private const string NOT_DECODABLE = 'Cursor token <%s> is invalid and could not be decoded.'; + private const string KIND_MISMATCH = 'Cursor token does not match the %s kind for cursor key <%s>.'; + private const string NOT_DECODABLE = 'Cursor token is invalid and could not be decoded.'; private function __construct(string $reason, ?Throwable $previous = null) { @@ -28,15 +29,13 @@ private function __construct(string $reason, ?Throwable $previous = null) /** * Creates a CursorIsInvalid from the offending token and the optional underlying cause. * - * @param string $token The opaque cursor token that could not be decoded. + * @param string $token The opaque cursor token that could not be decoded, kept out of the message. * @param Throwable|null $previous The underlying decoding failure preserved in the chain, if any. * @return CursorIsInvalid The composed exception describing the invalid cursor token. */ public static function from(string $token, ?Throwable $previous = null): CursorIsInvalid { - $template = CursorIsInvalid::NOT_DECODABLE; - - return new CursorIsInvalid(reason: sprintf($template, $token), previous: $previous); + return new CursorIsInvalid(reason: CursorIsInvalid::NOT_DECODABLE, previous: $previous); } /** @@ -44,13 +43,13 @@ public static function from(string $token, ?Throwable $previous = null): CursorI * * @param ValueKind $kind The value kind declared for the cursor key. * @param string $field The cursor key whose value was rejected. - * @param string $token The opaque cursor token carrying the rejected value. + * @param string $token The opaque cursor token carrying the rejected value, kept out of the message. * @return CursorIsInvalid The composed exception describing the kind mismatch. */ public static function kindMismatch(ValueKind $kind, string $field, string $token): CursorIsInvalid { $template = CursorIsInvalid::KIND_MISMATCH; - return new CursorIsInvalid(reason: sprintf($template, $token, $kind->value, $field)); + return new CursorIsInvalid(reason: sprintf($template, $kind->value, $field)); } } diff --git a/src/Exceptions/FilterExpressionIsInvalid.php b/src/Exceptions/FilterExpressionIsInvalid.php index e13533b..b3a3de6 100644 --- a/src/Exceptions/FilterExpressionIsInvalid.php +++ b/src/Exceptions/FilterExpressionIsInvalid.php @@ -10,27 +10,26 @@ * Raised when an RSQL filter expression cannot be parsed into the filter tree. * * The expression violates the FIQL/RSQL grammar, for example an unknown operator, an unclosed - * group or quote, a missing selector, or trailing characters after a complete expression. + * group or quote, a missing selector, or trailing characters after a complete expression. The message + * never carries the expression, since its compared values may hold personal data. */ final class FilterExpressionIsInvalid extends InvalidArgumentException implements HttpQueryException { - private const string REASON_TEMPLATE = 'Filter expression <%s> is invalid and could not be parsed.'; + private const string REASON = 'Filter expression is invalid and could not be parsed.'; - private function __construct(string $expression) + private function __construct() { - $template = FilterExpressionIsInvalid::REASON_TEMPLATE; - - parent::__construct(message: sprintf($template, $expression)); + parent::__construct(message: FilterExpressionIsInvalid::REASON); } /** * Creates a FilterExpressionIsInvalid from the offending expression. * - * @param string $expression The RSQL filter expression that failed to parse. + * @param string $expression The RSQL filter expression that failed to parse, kept out of the message. * @return FilterExpressionIsInvalid The composed exception describing the invalid expression. */ public static function from(string $expression): FilterExpressionIsInvalid { - return new FilterExpressionIsInvalid(expression: $expression); + return new FilterExpressionIsInvalid(); } } diff --git a/src/Exceptions/FilterShapeNotSupported.php b/src/Exceptions/FilterShapeNotSupported.php index 41947ab..e34e025 100644 --- a/src/Exceptions/FilterShapeNotSupported.php +++ b/src/Exceptions/FilterShapeNotSupported.php @@ -10,27 +10,26 @@ * Raised when an incoming filter is not a comparison or a flat AND group of comparisons. * * The supported shape is a conjunction of comparisons. An OR group, or a group nested inside - * another group, cannot be flattened into that shape and is rejected. + * another group, cannot be flattened into that shape and is rejected. The message never carries the + * expression, since its compared values may hold personal data. */ final class FilterShapeNotSupported extends InvalidArgumentException implements HttpQueryException { - private const string REASON_TEMPLATE = 'Filter shape <%s> is not supported.'; + private const string REASON = 'Filter shape is not supported.'; - private function __construct(string $expression) + private function __construct() { - $template = FilterShapeNotSupported::REASON_TEMPLATE; - - parent::__construct(message: sprintf($template, $expression)); + parent::__construct(message: FilterShapeNotSupported::REASON); } /** * Creates a FilterShapeNotSupported from the raw filter query string of the rejected filter. * - * @param string $expression The raw filter query string whose shape is not supported. + * @param string $expression The raw filter query string whose shape is not supported, kept out of the message. * @return FilterShapeNotSupported The composed exception describing the unsupported shape. */ public static function from(string $expression): FilterShapeNotSupported { - return new FilterShapeNotSupported(expression: $expression); + return new FilterShapeNotSupported(); } } diff --git a/src/Exceptions/FilterValueNotAllowed.php b/src/Exceptions/FilterValueNotAllowed.php index 2ada6a9..d2a0a33 100644 --- a/src/Exceptions/FilterValueNotAllowed.php +++ b/src/Exceptions/FilterValueNotAllowed.php @@ -11,12 +11,13 @@ * Raised when an incoming filter compares a value the field does not accept. * * A value is rejected when it falls outside the permitted set declared for the field, or when it - * does not match the value kind the field expects. + * does not match the value kind the field expects. The message names the field and the kind, never the + * value, since a compared value may hold personal data. */ final class FilterValueNotAllowed extends InvalidArgumentException implements HttpQueryException { - private const string KIND_MISMATCH = 'Value <%s> does not match the %s kind for filter field <%s>.'; - private const string NOT_PERMITTED = 'Value <%s> is not permitted for filter field <%s>.'; + private const string KIND_MISMATCH = 'Value does not match the %s kind for filter field <%s>.'; + private const string NOT_PERMITTED = 'Value is not permitted for filter field <%s>.'; private function __construct(string $reason) { @@ -28,27 +29,27 @@ private function __construct(string $reason) * * @param ValueKind $kind The value kind the field expects. * @param string $field The field whose value was rejected. - * @param string $value The value that does not match the kind. + * @param string $value The value that does not match the kind, kept out of the message. * @return FilterValueNotAllowed The composed exception describing the kind mismatch. */ public static function kindMismatch(ValueKind $kind, string $field, string $value): FilterValueNotAllowed { $template = FilterValueNotAllowed::KIND_MISMATCH; - return new FilterValueNotAllowed(reason: sprintf($template, $value, $kind->value, $field)); + return new FilterValueNotAllowed(reason: sprintf($template, $kind->value, $field)); } /** * Creates a FilterValueNotAllowed signaling that the value falls outside the permitted set. * * @param string $field The field whose value was rejected. - * @param string $value The value that falls outside the permitted set. + * @param string $value The value that falls outside the permitted set, kept out of the message. * @return FilterValueNotAllowed The composed exception describing the disallowed value. */ public static function notPermitted(string $field, string $value): FilterValueNotAllowed { $template = FilterValueNotAllowed::NOT_PERMITTED; - return new FilterValueNotAllowed(reason: sprintf($template, $value, $field)); + return new FilterValueNotAllowed(reason: sprintf($template, $field)); } } diff --git a/tests/Unit/Cursor/CriteriaTest.php b/tests/Unit/Cursor/CriteriaTest.php index 2109211..578d431 100644 --- a/tests/Unit/Cursor/CriteriaTest.php +++ b/tests/Unit/Cursor/CriteriaTest.php @@ -100,6 +100,24 @@ public function testKeysetWhenEffectiveSortIsEmptyThenThrowsSortIsRequired(): vo $criteria->keyset(); } + public function testFromQueryWhenNameCursorBreaksKindThenMessageOmitsTheToken(): void + { + /** @Given an opaque token carrying a client name and an identifier that is not a UUID */ + $token = Token::fromKeys(keys: ['Maria Silva', 'not-a-uuid'])->toString(); + + /** @And a schema declaring the identifier cursor key as a UUID and a default sort by name */ + $schema = Schema::create() + ->cursorKey(field: 'id', valueKind: ValueKind::UUID) + ->defaultSort(sort: Sort::fromExpression(expression: 'name,id')); + + /** @Then an exception carrying neither the token nor the client name is raised */ + $this->expectException(CursorIsInvalid::class); + $this->expectExceptionMessageMatches('/^Cursor token does not match the uuid kind for cursor key \.$/'); + + /** @When building the criteria from a request carrying that cursor */ + Criteria::fromQuery(schema: $schema, request: Query::from(parameters: ['page' => ['cursor' => $token]])); + } + public function testFromQueryWhenCursorValueFitsDeclaredKindThenKeysetCarriesIt(): void { /** @Given an opaque token carrying a store timestamp and a UUID identifier */ @@ -299,12 +317,9 @@ public function testFromQueryWhenCursorValueBreaksDeclaredKindThenThrowsCursorIs ->cursorKey(field: 'id', valueKind: ValueKind::UUID) ->defaultSort(sort: Sort::fromExpression(expression: '-created_at,-id')); - /** @And the reason template naming the token, the kind, and the cursor key */ - $template = 'Cursor token <%s> does not match the uuid kind for cursor key .'; - - /** @Then an exception indicating the cursor token is invalid is raised */ + /** @Then an exception naming the kind and the cursor key is raised */ $this->expectException(CursorIsInvalid::class); - $this->expectExceptionMessage(sprintf($template, $token)); + $this->expectExceptionMessage('Cursor token does not match the uuid kind for cursor key .'); /** @When building the criteria from a request carrying that cursor */ Criteria::fromQuery(schema: $schema, request: Query::from(parameters: ['page' => ['cursor' => $token]])); diff --git a/tests/Unit/Cursor/KeysetTest.php b/tests/Unit/Cursor/KeysetTest.php index c66a8aa..d898bce 100644 --- a/tests/Unit/Cursor/KeysetTest.php +++ b/tests/Unit/Cursor/KeysetTest.php @@ -151,7 +151,7 @@ public function testCriteriaWhenTokenCannotBeDecodedThenThrowsCursorIsInvalid(): /** @Then an exception indicating the cursor is invalid is raised */ $this->expectException(CursorIsInvalid::class); - $this->expectExceptionMessage('Cursor token is invalid and could not be decoded.'); + $this->expectExceptionMessage('Cursor token is invalid and could not be decoded.'); /** @When the criteria is parsed from the request */ Criteria::fromQuery(schema: $this->schema, request: $request); diff --git a/tests/Unit/FilterTest.php b/tests/Unit/FilterTest.php index 815075e..8f934a2 100644 --- a/tests/Unit/FilterTest.php +++ b/tests/Unit/FilterTest.php @@ -125,9 +125,9 @@ public function testFromQueryWhenOrGroupThenThrowsFilterShapeNotSupported(): voi /** @Given a query carrying an OR expression of two comparisons */ $query = Query::from(parameters: ['filter' => 'a==1,b==2']); - /** @Then an exception carrying the raw filter query string is raised */ + /** @Then an exception indicating the filter shape is not supported is raised */ $this->expectException(FilterShapeNotSupported::class); - $this->expectExceptionMessage('Filter shape is not supported.'); + $this->expectExceptionMessage('Filter shape is not supported.'); /** @When building the criteria from the query */ Criteria::fromQuery(schema: $this->schema, request: $query); @@ -168,9 +168,9 @@ public function testFromQueryWhenNestedGroupThenThrowsFilterShapeNotSupported(): /** @Given a query whose parentheses nest an OR group inside an AND group */ $query = Query::from(parameters: ['filter' => '(a==1,b==2);c==3']); - /** @Then an exception carrying the raw filter query string is raised */ + /** @Then an exception indicating the filter shape is not supported is raised */ $this->expectException(FilterShapeNotSupported::class); - $this->expectExceptionMessage('Filter shape <(a==1,b==2);c==3> is not supported.'); + $this->expectExceptionMessage('Filter shape is not supported.'); /** @When building the criteria from the query */ Criteria::fromQuery(schema: $this->schema, request: $query); @@ -221,7 +221,7 @@ public function testFromQueryWhenValueBreaksKindThenThrowsFilterValueNotAllowed( /** @Then an exception indicating the value does not match the expected kind is raised */ $this->expectException(FilterValueNotAllowed::class); - $this->expectExceptionMessage('does not match the datetime kind'); + $this->expectExceptionMessage('Value does not match the datetime kind for filter field .'); /** @When building the criteria from the query */ Criteria::fromQuery(schema: $schema, request: $query); @@ -242,6 +242,26 @@ public function testFromQueryWhenParenthesizedComparisonThenComparisonIsReturned ); } + public function testFromQueryWhenPersonalValueBreaksKindThenMessageOmitsTheValue(): void + { + /** @Given a schema expecting a UUID kind on the client_id field */ + $schema = Schema::create()->filterable( + field: 'client_id', + operators: [Operator::EQUAL], + valueKind: ValueKind::UUID + ); + + /** @And a query whose value is a client name instead of a UUID */ + $query = Query::from(parameters: ['filter' => 'client_id=="Maria Silva"']); + + /** @Then an exception naming only the kind and the field is raised */ + $this->expectException(FilterValueNotAllowed::class); + $this->expectExceptionMessageMatches('/^Value does not match the uuid kind for filter field \.$/'); + + /** @When building the criteria from the query */ + Criteria::fromQuery(schema: $schema, request: $query); + } + public function testFromQueryWhenDateTimeValueMatchesKindThenComparisonIsReturned(): void { /** @Given a schema allowing a date-time field under the greater-than operator */ @@ -292,7 +312,7 @@ public function testFromQueryWhenValueNotPermittedThenThrowsFilterValueNotAllowe /** @Then an exception indicating the value is not permitted is raised */ $this->expectException(FilterValueNotAllowed::class); - $this->expectExceptionMessage('Value is not permitted for filter field .'); + $this->expectExceptionMessage('Value is not permitted for filter field .'); /** @When building the criteria from the query */ Criteria::fromQuery(schema: $schema, request: $query); @@ -343,6 +363,19 @@ public function testFromQueryWhenComparisonOperatorThenComparisonCarriesThatOper self::assertEquals([Comparison::of(field: 'a', values: ['b'], operator: $expected)], $comparisons); } + public function testFromQueryWhenMalformedPersonalValueThenMessageOmitsTheExpression(): void + { + /** @Given a query whose filter leaves the quoted client name unclosed */ + $query = Query::from(parameters: ['filter' => 'name=="Maria Silva']); + + /** @Then an exception carrying no part of the filter expression is raised */ + $this->expectException(FilterExpressionIsInvalid::class); + $this->expectExceptionMessageMatches('/^Filter expression is invalid and could not be parsed\.$/'); + + /** @When building the criteria from the query */ + Criteria::fromQuery(schema: $this->schema, request: $query); + } + public function testFromQueryWhenOperatorNotAllowedThenThrowsFilterOperatorNotAllowed(): void { /** @Given a schema allowing the status field under equality only */ @@ -403,7 +436,7 @@ public function testFromQueryWhenMalformedExpressionThenThrowsFilterExpressionIs /** @Then an exception indicating the filter expression is invalid is raised */ $this->expectException(FilterExpressionIsInvalid::class); - $this->expectExceptionMessage('could not be parsed'); + $this->expectExceptionMessage('Filter expression is invalid and could not be parsed.'); /** @When building the criteria from the query */ Criteria::fromQuery(schema: $this->schema, request: $query); @@ -428,6 +461,19 @@ public function testFromQueryWhenInListHasDisallowedValueThenThrowsFilterValueNo Criteria::fromQuery(schema: $schema, request: $query); } + public function testFromQueryWhenDisjunctionOfPersonalValuesThenMessageOmitsTheExpression(): void + { + /** @Given a query whose filter is an OR group of two client names */ + $query = Query::from(parameters: ['filter' => 'name=="Maria Silva",name=="Joao Souza"']); + + /** @Then an exception carrying no part of the filter expression is raised */ + $this->expectException(FilterShapeNotSupported::class); + $this->expectExceptionMessageMatches('/^Filter shape is not supported\.$/'); + + /** @When building the criteria from the query */ + Criteria::fromQuery(schema: $this->schema, request: $query); + } + public function testFromQueryWhenFilterNestedBeyondTheMaximumDepthThenThrowsFilterExpressionIsInvalid(): void { /** @Given a filter template wrapping a comparison in placeholders */ @@ -440,7 +486,7 @@ public function testFromQueryWhenFilterNestedBeyondTheMaximumDepthThenThrowsFilt /** @Then an exception indicating the filter expression is invalid is raised */ $this->expectException(FilterExpressionIsInvalid::class); - $this->expectExceptionMessage('could not be parsed'); + $this->expectExceptionMessage('Filter expression is invalid and could not be parsed.'); /** @When building the criteria from the query */ Criteria::fromQuery(schema: $this->schema, request: $query);