Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 108 additions & 39 deletions Classes/Service/ResponseCacheService.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ public function buildEntryIdentifier(OperationInterface $operation, array $route
http_build_query($allowedQueryParams),
];

// An empty `identifierExpressions` array (the default) leaves the loop below a no-op, so
// entries built without this setting hash identically to before it existed.
foreach ($responseCacheSettings->getIdentifierExpressions() as $identifierExpression) {
$identifierExpressionValue = $this->evaluateIdentifierExpression($identifierExpression, $operation, $route, $request);
if ($identifierExpressionValue === null) {
Expand All @@ -77,11 +75,13 @@ public function buildEntryIdentifier(OperationInterface $operation, array $route
$keyParts[] = $identifierExpressionValue;
}

// serialize() keeps the parts apart - with a separator, a value containing it could produce
// the same key as different values (`a|b` + `c` and `a` + `b|c`).
// md5() here is a non-cryptographic cache key derivation, not a security control - there is
// no secret to protect and no adversarial value in collision resistance, since `security` is
// still fully re-evaluated on every cache hit (see `OperationResponseCache`), so a collision
// could not bypass access control.
return md5(implode('|', $keyParts)); // NOSONAR php:S4790 - weak hash algorithm, not used in a sensitive context here
return md5(serialize($keyParts)); // NOSONAR php:S4790 - weak hash algorithm, not used in a sensitive context here
}

/**
Expand Down Expand Up @@ -189,14 +189,11 @@ public function evaluateStoreTagExpressions(
): ?array {
$tags = [];
foreach ($tagExpressions as $tagExpression) {
$tag = $this->evaluateStoreTagExpression($tagExpression, $operation, $route, $request, $additionalVariables);
if ($tag === null) {
$expressionTags = $this->evaluateStoreTagExpression($tagExpression, $operation, $route, $request, $additionalVariables);
if ($expressionTags === null) {
return null;
}

if ($tag !== '') {
$tags[] = $tag;
}
array_push($tags, ...$expressionTags);
}

return $tags;
Expand Down Expand Up @@ -225,31 +222,29 @@ public function evaluateFlushTagExpressions(
): array {
$tags = [];
foreach ($tagExpressions as $tagExpression) {
$tag = $this->evaluateFlushTagExpression($tagExpression, $operation, $route, $request, $additionalVariables);
if ($tag !== null && $tag !== '') {
$tags[] = $tag;
}
array_push($tags, ...$this->evaluateFlushTagExpression($tagExpression, $operation, $route, $request, $additionalVariables));
}

return array_values(array_unique($tags));
}

/**
* Store-side single-expression evaluation: logs at error level and signals "do not store"
* (`null`) both when the expression itself fails and when it produces a non-empty tag the
* cache frontend rejects.
* (`null`) both when the expression itself fails and when it produces a tag the cache
* frontend rejects.
*
* @param array<string, mixed> $additionalVariables
* @return string[]|null
*/
private function evaluateStoreTagExpression(
string $tagExpression,
OperationInterface $operation,
array $route,
Request $request,
array $additionalVariables = []
): ?string {
): ?array {
try {
$tag = $this->evaluateTagExpression($tagExpression, $operation, $route, $request, $additionalVariables);
$tags = $this->evaluateTagExpression($tagExpression, $operation, $route, $request, $additionalVariables);
} catch (\Throwable $throwable) {
$this->logger?->error('t3api response cache tag expression evaluation failed - entry not stored', [
'tagExpression' => $tagExpression,
Expand All @@ -259,53 +254,56 @@ private function evaluateStoreTagExpression(
return null;
}

if ($tag !== '' && !$this->isValidTag($tag)) {
$this->logger?->error('t3api response cache tag expression produced an invalid tag - entry not stored', [
'tagExpression' => $tagExpression,
'tag' => $tag,
]);
foreach ($tags as $tag) {
if (!$this->isValidTag($tag)) {
$this->logger?->error('t3api response cache tag expression produced an invalid tag - entry not stored', [
'tagExpression' => $tagExpression,
'tag' => $tag,
]);

return null;
return null;
}
}

return $tag;
return $tags;
}

/**
* Flush-side single-expression evaluation: logs at warning level and signals "skip this tag"
* (`null`) both when the expression itself fails and when it produces a non-empty tag the
* cache frontend rejects - the other declared tags still flush.
* Flush-side single-expression evaluation: logs at warning level and skips the whole expression
* when it fails, or a single tag the cache frontend rejects - the other declared tags still flush.
*
* @param array<string, mixed> $additionalVariables
* @return string[]
*/
private function evaluateFlushTagExpression(
string $tagExpression,
OperationInterface $operation,
array $route,
Request $request,
array $additionalVariables = []
): ?string {
): array {
try {
$tag = $this->evaluateTagExpression($tagExpression, $operation, $route, $request, $additionalVariables);
$tags = $this->evaluateTagExpression($tagExpression, $operation, $route, $request, $additionalVariables);
} catch (\Throwable $throwable) {
$this->logger?->warning('t3api response cache tag expression evaluation failed - skipped', [
'tagExpression' => $tagExpression,
'exception' => $throwable,
]);

return null;
return [];
}

if ($tag !== '' && !$this->isValidTag($tag)) {
return array_values(array_filter($tags, function (string $tag) use ($tagExpression): bool {
if ($this->isValidTag($tag)) {
return true;
}
$this->logger?->warning('t3api response cache tag expression produced an invalid tag - skipped', [
'tagExpression' => $tagExpression,
'tag' => $tag,
]);

return null;
}

return $tag;
return false;
}));
}

/**
Expand All @@ -315,19 +313,38 @@ private function evaluateFlushTagExpression(
* evaluation failure - the store and flush sides apply their own failure policy (see
* `evaluateStoreTagExpression()` and `evaluateFlushTagExpression()`).
*
* An array result gives one tag per item (e.g. UIDs of related records). Empty strings give no tag.
*
* @param array<string, mixed> $additionalVariables
* @return string[]
* @throws \UnexpectedValueException when the result or an array item can not be converted to a string
*/
private function evaluateTagExpression(
string $tagExpression,
OperationInterface $operation,
array $route,
Request $request,
array $additionalVariables = []
): string {
return (string)$this->getConditionResolver()->evaluate(
): array {
$result = $this->getConditionResolver()->evaluate(
$tagExpression,
$this->getConditionVariables($operation, $route, $request, $additionalVariables)
);

$tags = [];
foreach (is_array($result) ? $result : [$result] as $value) {
if (!($value === null || is_scalar($value) || $value instanceof \Stringable)) {
throw new \UnexpectedValueException(
sprintf('Value of type `%s` can not be converted to a cache tag', get_debug_type($value)),
1793650001
);
}
if ((string)$value !== '') {
$tags[] = (string)$value;
}
}

return $tags;
}

/**
Expand Down Expand Up @@ -401,18 +418,21 @@ protected function getAllowedParameterNames(OperationInterface $operation): arra
* shared with requests it was meant to be kept separate from, so `null` here means "do not
* cache this request" rather than "treat as empty". A null/false expression result, on the
* other hand, casts to `''` and is a perfectly valid - if unhelpful - identifier contribution.
* A result which can not be converted to a string (see `normalizeIdentifierValue()`) fails
* closed the same way.
*/
protected function evaluateIdentifierExpression(
string $identifierExpression,
OperationInterface $operation,
array $route,
Request $request
): ?string {
): string|array|null {
try {
return (string)$this->getConditionResolver()->evaluate(
$value = $this->getConditionResolver()->evaluate(
$identifierExpression,
$this->getConditionVariables($operation, $route, $request)
);
$identifierValue = $this->normalizeIdentifierValue($value);
} catch (\Throwable $throwable) {
$this->logger?->error('t3api response cache identifier expression evaluation failed', [
'identifierExpression' => $identifierExpression,
Expand All @@ -421,6 +441,55 @@ protected function evaluateIdentifierExpression(

return null;
}

if ($identifierValue === null) {
$this->logger?->error('t3api response cache identifier expression result can not be converted to a string', [
'identifierExpression' => $identifierExpression,
'type' => get_debug_type($value),
]);
}

return $identifierValue;
}

/**
* Scalars and `Stringable` objects are converted to strings. Arrays (e.g. `frontend.user.userGroupIds`)
* stay arrays - so they never produce the same identifier as a string - with normalized and sorted
* items, so the order of items does not change the identifier. Returns null for values which can not
* be converted to a string (objects without `__toString()`, also inside arrays).
*
* @return string|array<mixed>|null
*/
protected function normalizeIdentifierValue(mixed $value): string|array|null
{
if (is_array($value)) {
$items = [];
foreach ($value as $key => $item) {
$normalizedItem = $this->normalizeIdentifierValue($item);
if ($normalizedItem === null) {
return null;
}
$items[$key] = $normalizedItem;
}
// Sorted by keys first, so a list is detected regardless of the insertion order of its keys
// (`[1 => 'a', 0 => 'b']`). Default flags: `SORT_STRING` would put key 10 before key 2.
ksort($items);
if (array_is_list($items)) {
// Compares the unambiguous representation - a numeric comparison treats e.g. `01` and `1`
// as equal, which would keep their original order.
usort($items, static fn(string|array $a, string|array $b): int => strcmp(serialize($a), serialize($b)));
} else {
ksort($items, SORT_STRING);
}

return $items;
}

if ($value === null || is_scalar($value) || $value instanceof \Stringable) {
return (string)$value;
}

return null;
}

/**
Expand Down
1 change: 1 addition & 0 deletions Documentation/Miscellaneous/Changelog/Index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Changelog

5.0.0
=====
- Fix response cache identifier for ``identifierExpressions`` returning an array (e.g. ``frontend.user.userGroupIds``) - array items are part of the identifier now. Tag expressions returning an array add a tag per item. See :ref:`response-cache`.
- [!!!] Validate origin and content type of requests changing data: requests from another origin not allowed by CORS ``allowOrigin`` return ``403 Forbidden``, JSON body without JSON ``Content-Type`` returns ``415 Unsupported Media Type``. Add origins of frontend applications to ``allowOrigin`` and send ``Content-Type: application/json``. See :ref:`cors_write_requests`.
- Fix response cache invalidation when the page cache is cleared from the backend (cache tags passed as a list were ignored, numeric tags caused errors).
- Log unexpected exceptions with an event ID. Outside of ``Development`` context HTTP 500 response contains a generic message with this event ID instead of the exception message. See :ref:`ThrowableSubscriber <serialization_subscribers_throwable_subscriber>`.
Expand Down
33 changes: 32 additions & 1 deletion Documentation/ResponseCache/Index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ configured separately, via the sibling
request context beyond URL and declared filters) you **must** declare a
suitable ``identifierExpressions`` entry (typically reading a TYPO3
Context aspect via ``context``, see "Cache key" below) - otherwise the
first user's response is served to everyone.
first user's response is served to everyone. If the response contains
records restricted by ``fe_group`` (also in relations), add
``frontend.user.userGroupIds`` to ``identifierExpressions``.

Configuration reference
========================
Expand Down Expand Up @@ -115,6 +117,31 @@ the error is logged, regardless of whether the other expressions would have
evaluated fine. Leaving the array empty (the default) leaves the identifier
exactly as it would be without this setting.

An expression may also return an array, e.g. ``groupIds`` above - the items
are sorted, so the same items in a different order produce the same
identifier. A result which can not be converted to a string (an object
without ``__toString()``) is treated like a failed expression.

For records restricted by ``fe_group`` add the group IDs of the current
frontend user:

.. code-block:: php

/**
* @ApiResource(
* attributes={
* "cache"={
* "identifierExpressions"={"frontend.user.userGroupIds"}
* }
* }
* )
*/

Each combination of frontend user groups gets separate cache entries;
anonymous visitors share one entry. Add it only to resources whose output
depends on user groups - every additional combination of groups multiplies
the number of stored entries.

.. note::
A non-empty ``cache`` block enables caching automatically - there is no
need to add ``"enabled"=true`` yourself. An empty (or absent) ``cache``
Expand Down Expand Up @@ -559,6 +586,10 @@ All three are evaluated by the same expression engine and variable set as
"Availability of ``object``" below for the full picture across every response
cache expression on this page.

An expression returning an array adds one tag per non-empty item - e.g. a
result ``['category_1', 'category_2']`` adds both tags. A result (or array item)
which can not be converted to a string is treated like a failed expression.

The canonical route-driven case is a write handler that bypasses
DataHandler/Extbase persistence and must flush a tag built from a matched
route parameter (see :ref:`response-cache-invalidation` below for the full
Expand Down
Loading
Loading