[matkomat]

Summary

This hardens Object Store metadata addressing and read integrity.

• switch metadata subjects to reversible base64url name encoding, with legacy subject fallback for reads
• migrate legacy metadata subjects forward when an existing object is updated
• make updateMeta() renames remove the old logical name and reject collisions with active targets
• fail Object Store reads explicitly when chunks disappear before the expected object is fully delivered
• validate streamed reads against expected byte count, chunk count, and digest
• add regression coverage for name collisions, rename semantics, incomplete reads, and ObjectResult validation

Verification

• vendor/bin/php-cs-fixer fix --config=.php-cs-fixer.dist.php --dry-run --diff src/ObjectStore/ObjectStore.php src/ObjectStore/ObjectResult.php tests/Unit/ObjectStore/ObjectResultTest.php tests/Integration/ObjectStore/ObjectStoreTest.php
• vendor/bin/phpstan analyse --no-progress
• vendor/bin/phpunit --testsuite=unit
• env -u NATS_URL vendor/bin/phpunit --testsuite=integration --stop-on-error

Closes #61
Closes #62
Closes #63
Closes #65

[matkomat-agent]

Clean fix for four related integrity issues. The base64url encoding is reversible, legacy fallback reads preserve backward compat, and the migration-on-write strategy is pragmatic. Read validation (bytes, chunks, digest) is thorough. Left a few inline notes, none blocking.

[matkomat-agent]

Review — fix: harden object store metadata and reads

Solid PR covering several serious integrity and collision issues. Here are the findings:

---

1. Base64url padding — potential incompatibility with Go client

encodeName() uses strtr(base64_encode($name), '+/', '-_') which retains = padding.

The Go NATS client uses base64.RawURLEncoding (no padding). For the name "ab":

• PHP: subject $O.bucket.M.YWI=
• Go: subject $O.bucket.M.YWI

This means objects created by the Go client won't be visible to the PHP client (and vice versa) for names whose base64 output contains padding.

Fix:

private function encodeName(string $name): string
{
    return rtrim(strtr(base64_encode($name), '+/', '-_'), '=');
}

---

2. updateMeta() rename doesn't leave a delete marker for the old name

When an object is renamed from A to B, updateMeta() purges the old subject without leaving a tombstone. Watchers monitoring the store won't receive a delete event for the old name — the object simply vanishes. If ObjectWatcher instances are in use, this can cause stale data on the client side.

The Go client handles this by writing a delete marker for the old name before purging.

---

3. putStream() drift with putRaw() in legacy cleanup logic

putRaw() uses purgeLegacyMetaSubjectIfNeeded(), but putStream() has an inline if-check that does the same thing. The TODO comment exists (L89), but it's worth fixing in this PR while the logic is fresh — otherwise it's easy to forget and introduce a subtle bug when one path changes but the other doesn't.

// putStream, L158-166 — replace with:
$this->purgeLegacyMetaSubjectIfNeeded($name, $existingMetaSubject);

---

4. assertCompleteRead() — hash_final cannot be called twice

hash_final($this->hashContext, true) consumes the context. If assertCompleteRead() is ever called twice, we'll get a fatal error. Currently protected by the sourceExhausted guard in finishSource(), but hashContext should be nullable after finalization for defensiveness:

private ?\HashContext $hashContext;
// ...
$actualDigest = 'SHA-256=' . base64_encode(hash_final($this->hashContext, true));
$this->hashContext = null; // prevent double-finalize

This requires removing readonly from the property, which is a minimal change.

---

5. Minor: import ordering in Stream.php

The JetStreamException import was added between OrderedConsumerConfig and PushConsumerInterface, breaking the alphabetical order that is consistently used throughout the rest of the file.

---

Positives

• tryGetLastMessageForSubject() with errCode 10037 check — clean and correct approach
• findObjectInfoRecord with name verification after legacy lookup — properly handles lossy legacy encoding
• ObjectResult integrity validation (bytes, chunks, digest) — well-designed and in the right place (finishSource)
• Test coverage is thorough — collision test, rename semantics, mid-read chunk disappearance, digest mismatch

Points 1 and 3 are the most important to address before merge. Point 2 depends on how important watchers are in practice — if nobody uses them, it can be deferred.

[matkomat]

Follow-up pushed in 83444bd.

Addressed in code:

• Point 3: putStream() now reuses purgeLegacyMetaSubjectIfNeeded() instead of keeping an inline cleanup branch.
• Point 4: ObjectResult now nulls the hash context after hash_final(), guards against double-finalize, and still runs cleanup exactly once on validation failure.
• Point 5: reordered the Stream.php imports.

Left unchanged intentionally:

• Point 1: current nats.go still uses padded URL-safe base64 for object-name subjects (encodeName() calls base64.URLEncoding.EncodeToString(...)), so keeping padding here is the compatibility-preserving choice.
• Point 2: current nats.go UpdateMeta() publishes the new meta and then purges the old subject directly; it does not emit a tombstone for the old name first. If we want watcher-specific rename/delete signaling beyond that, I think that should be a separate behavior change rather than part of this parity fix.

Re-verified after the follow-up:

• vendor/bin/phpunit tests/Unit/ObjectStore/ObjectResultTest.php
• env -u NATS_URL vendor/bin/phpunit tests/Integration/JetStream/StreamTest.php tests/Integration/ObjectStore/ObjectStoreTest.php
• vendor/bin/phpstan analyse --no-progress

[matkomat-agent]

Follow-up review — 83444bd

The follow-up commit cleanly addresses points 3, 4, and 5 from the initial review.

What was fixed:

• Import ordering — JetStreamException is now in the correct alphabetical position.
• putStream() drift — inline legacy cleanup replaced with purgeLegacyMetaSubjectIfNeeded(), eliminating the divergence with putRaw().
• hashContext double-finalize — property is now nullable, nulled after hash_final(), and throws an explicit exception if accessed after finalization.
• finishSource() cleanup guarantee — assertCompleteRead() is now wrapped in try/catch so that runCleanup() always runs, even when validation fails. The exception is re-thrown after cleanup. This is a better approach than what I suggested — it ensures the internal consumer is deleted even when the read fails validation.
• Tests — added verification that the cleanup callback is called exactly once in both the incomplete read and digest mismatch scenarios.

Intentionally left unchanged (with sound reasoning):

• Padding (point 1) — nats.go actually uses base64.URLEncoding (with padding), not RawURLEncoding. My mistake in the initial review — keeping padding is the correct choice for Go client parity.
• Rename tombstone (point 2) — the Go client also does not emit a tombstone on rename, so the current behavior maintains parity. Agreed that this should be a separate PR if it's ever needed.

Everything looks good to merge.