Drop If-None-Match tokens the server could never have issued

CHANGELOG.md

@@ -65,6 +65,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Recording is off by default: `QueryRepositoryModule` binds `SemanticLoggerInterface` annotated with `#[CacheLog]` → `Koriym\SemanticLogger\NullSemanticLogger`, the same rule the package already applies to its cache engine, and the recording binding moves out of the internal `DonutCacheModule` into the two log modules. Nothing is recorded, nothing accumulates and no flush is owed until an app installs one — a healthy session was measured to contain no failure entries at all, against ~1.4 KB accumulated per request.
 
 ### Fixed
+- A client-chosen `If-None-Match` token containing a PSR-6 reserved character (`{}()/\@:`) reached the ETag pool as a cache key and threw, turning a request header into a 500 that logged like a pool outage. Such a token can never have been issued by this server, so `EntityTags` drops it and the request is answered in full; `*` is likewise dropped (RFC 9110 §13.1.2 gives it existence semantics this package does not implement).
 - An embedded child was materialized twice per parent store: the storage re-invoked the request instead of reusing the execution `setCacheDependency()` and the renderer already paid for, so a `type: "view"` entry could hold a body and a view from two different runs of a non-idempotent child.
 - A donut refresh advanced `Last-Modified` even when the recomposed content was byte-identical, so an unchanged representation looked changed; the recorded content is compared and the original time carried over. `Age` is now the time since the state was stored (`storedAt`) instead of being derived from `Last-Modified`, which is the content's change time — entries stored before `storedAt` existed fall back to the old derivation.
 - `putStatic()`/`putDonut()` recorded a negative lifetime verbatim while the storage clamped what it stored, so the log both contradicted its own save events and violated the published `put_donut` schema (`minimum: 0`). The requested lifetime is clamped where it is recorded.

src/EntityTags.php

@@ -7,11 +7,12 @@
 use function preg_match;
 use function preg_match_all;
 use function str_starts_with;
+use function strpbrk;
 use function substr;
 use function trim;
 
 /**
- * The opaque-tags an `If-None-Match` field value contains
+ * The opaque-tags an `If-None-Match` field value contains, reduced to pool-key candidates
  *
  * Pool keys are bare opaque-tags, so quoted entity-tags (RFC 9110 §8.8.3), weak validators and
  * comma-separated lists are reduced to bare tokens. A comma inside a quoted opaque-tag is data,
@@ -20,13 +21,23 @@
  * unterminated quote or trailing garbage is rejected rather than salvaged, because half-reading a
  * validator is how a request gets answered about content nobody asked for.
  *
+ * A token this server could never have issued is dropped, not looked up. Every setter produces
+ * tags free of PSR-6 reserved characters (crc32 decimal, or the sanitized URI tag in dev), so a
+ * token carrying one can never match - and handing it to the pool would turn a request header
+ * into a thrown InvalidArgumentException. `*` is dropped for the same reason on top of not being
+ * an opaque tag at all: RFC 9110 §13.1.2 gives it existence semantics this package does not
+ * implement, so it must not be mistaken for a key.
+ *
  * Its own class because two lookups need it and the parsing is the complicated half of both.
  */
 final class EntityTags
 {
     /** Entity-tag: optional weak indicator, then a quoted opaque-tag or a bare legacy token */
     private const ENTITY_TAG_PATTERN = '(?:W\/)?"[^"]*"|[^,"]+';
 
+    /** PSR-6 reserves these in cache keys; no ETag setter emits them, so a token with one cannot match */
+    private const PSR6_RESERVED = '{}()/\\@:';
+
     /** @return list<string> */
     public static function of(string $fieldValue): array
     {
@@ -46,7 +57,7 @@ public static function of(string $fieldValue): array
             }
 
             $opaqueTag = trim($entityTag, '"');
-            if ($opaqueTag !== '') {
+            if ($opaqueTag !== '' && $opaqueTag !== '*' && strpbrk($opaqueTag, self::PSR6_RESERVED) === false) {
                 $opaqueTags[] = $opaqueTag;
             }
         }

tests/EntityTagsTest.php

@@ -0,0 +1,48 @@
+<?php
+
+declare(strict_types=1);
+
+namespace BEAR\QueryRepository;
+
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Which tokens are worth looking up
+ *
+ * The parser reduces an If-None-Match field value to pool-key candidates. A token this server
+ * could never have issued - PSR-6 reserved characters, or the RFC-special `*` - is dropped:
+ * it can never match, and handing it to the pool would turn a request header into a thrown
+ * InvalidArgumentException (a client-chosen 500).
+ */
+class EntityTagsTest extends TestCase
+{
+    public function testATokenWithPsr6ReservedCharactersIsDropped(): void
+    {
+        // Symfony throws InvalidArgumentException for keys containing {}()/\@: - a validator
+        // carrying one is unanswerable, so it is dropped before the pool sees it.
+        $this->assertSame([], EntityTags::of('"x:y"'));
+        $this->assertSame([], EntityTags::of('"a/b"'));
+        $this->assertSame([], EntityTags::of('W/"a@b"'));
+        $this->assertSame([], EntityTags::of('x{y'));
+    }
+
+    public function testAValidTokenBesideADroppedOneStillMatches(): void
+    {
+        // Dropping is per-token, not per-field: a list with one honest validator revalidates.
+        $this->assertSame(['927897379'], EntityTags::of('"927897379", "x:y"'));
+    }
+
+    public function testStarIsNotAnOpaqueTag(): void
+    {
+        // RFC 9110 §13.1.2 gives `*` existence semantics; it is not a validator to look up.
+        $this->assertSame([], EntityTags::of('*'));
+        $this->assertSame(['abc'], EntityTags::of('"abc", *'));
+    }
+
+    public function testServerIssuedGrammarsPass(): void
+    {
+        // crc32 decimal (production setters) and the sanitized URI tag (dev setter).
+        $this->assertSame(['927897379'], EntityTags::of('"927897379"'));
+        $this->assertSame(['_user_id=1'], EntityTags::of('"_user_id=1"'));
+    }
+}

tests/HttpCacheTest.php

@@ -196,4 +196,25 @@ public function testTheScopedScopeClosesWhenTheLookupThrows(): void
             $this->assertSame('app://self/user?id=1', $error->uri, $class);
         }
     }
+
+    public function testAClientChosenUnpoolableTokenIsAMissNotAnError(): void
+    {
+        // Before EntityTags dropped these, `If-None-Match: "x:y"` reached the pool as a key,
+        // Symfony threw InvalidArgumentException, and any client could turn a request into a 500
+        // that logged like a pool outage. Unanswerable tokens are a plain miss at both boundaries.
+        $storage = ResourceStorageTest::getResourceStorageInstance();
+        $uri = new Uri('app://self/user?id=1');
+
+        foreach ([HttpCache::class, CliHttpCache::class] as $class) {
+            $logger = new RecordingSemanticLogger();
+            $httpCache = new $class($storage, $logger);
+
+            $this->assertFalse($httpCache->isNotModified(['HTTP_IF_NONE_MATCH' => '"x:y"']), $class);
+            $this->assertFalse($httpCache->isNotModifiedFor($uri, ['HTTP_IF_NONE_MATCH' => '"a/b, c"']), $class);
+            $this->assertFalse($httpCache->isNotModified(['HTTP_IF_NONE_MATCH' => '*']), $class . ': * is existence semantics, not a key');
+
+            $this->assertCount(0, $logger->events, $class . ': no cache_error - the pool was never asked');
+            $this->assertCount(3, $logger->closes, $class . ': each decision closes as an ordinary miss');
+        }
+    }
 }