3/5 🧹 Add psalm at level 1 - #20
Merged
Merged
Conversation
apache/thrift < 0.24.0 is affected by GHSA-8wv5-x4w7-5gww / CVE-2026-43871 — an infinite loop in the PHP, Python, Go and Java bindings, severity high — so the whole previous '>=0.11, <0.17' range is unusable and 0.24.0 is the only release that is not. 0.24.0 itself requires PHP ^8.1; this major goes to ^8.4 so the toolchain can land on current stable releases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three fixes, all enabled by the new baseline: * socket_create() returns a Socket object since PHP 8.0, so the is_resource($this->socket) guard in connect() was always false and a fresh socket was created on every flush(). Descriptors did not leak — the old socket was collected immediately — but each flush paid for a needless socket_create() plus socket_connect(), and close() was a no-op in practice. * Match the parameter types of the now strictly typed TTransport in 0.24: read(int $len): string and write(string $buf): void. * read() returned '' unconditionally, so the inherited readAll() spun forever in while (strlen($data) < $len). Unreachable for the emit-only agent client, but it is the same infinite-loop shape as CVE-2026-43871. It throws TTransportException now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tree was generated by compiler 0.15 while the runtime is now 0.24. Regenerating against jaeger-idl v0.12.0 aligns the two: * declare(strict_types=1) in every file (THRIFT-5986) * native property and constructor types (THRIFT-5991), so 'public $key' becomes 'public ?string $key' — which is what the IDL actually says * native return types on struct methods (THRIFT-5990) * PSR-12 layout (THRIFT-5959) bin/thrift-gen.sh cloned jaeger-idl from a moving 'main', so two runs a month apart could differ with no record of what changed. It now pins jaeger-idl v0.12.0, refuses to run unless the compiler is 0.24.x, generates into a mktemp directory cleaned up by a trap instead of leaving a clone in the working tree, and drops a dead 'rm -rf ../src/Jaeger/Thrift/' from an older layout. Running it twice is now a no-op. BREAKING: 24 classes are gone — AggregationValidator, BaggageRestrictionManager, Dependency and ThrottlingService with their DTOs. They came from IDL definitions Jaeger removed upstream long ago (already absent in jaeger-idl v0.11.4); they are server-side APIs this client never used. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
php-cs-fixer and rector, both configured to skip src/Thrift: that tree is generated by bin/thrift-gen.sh and has to stay byte-for-byte reproducible from the IDL, so reformatting it would be undone by the next regeneration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Break multi-argument constructors across lines and normalise the spacing between promoted parameter types and their names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Level 1 — the strictest — over src/, with src/Thrift excluded: its shape is the thrift compiler's contract and it calls thrift_protocol_* from the optional C extension. ClassMustBeFinal is suppressed because this is a library and consumers legitimately extend these classes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolve every issue psalm reports at level 1: * type the arrays that cross a public boundary (tags, logs, spans, baggage) so coercions are checked instead of inferred as mixed * handle the false branch of gethostname/getmypid/getmyuid/getmygid/json_encode instead of passing it straight into a string parameter * cast mixed/int and float operands explicitly * give the ArrayAccess and IteratorAggregate implementations their template parameters and real native signatures, dropping #[ReturnTypeWillChange] * align parameter names with the interfaces that declare them, so named arguments work One suppression remains, in Span\Batch\SpanBatch: SpanInterface cannot declare that it extends the generated Thrift\Span even though its only implementation does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third of five stacked PRs for the 4.0 major. Stacked on #19.
What
Psalm 6 at errorLevel 1 — the strictest — over
src/, and every issue it reported is fixed:mixedgethostname/getmypid/getmyuid/getmygid/json_encodeis handled instead of being passed straight into astringparameterArrayAccessandIteratorAggregateimplementations get their template parameters and real native signatures, dropping#[ReturnTypeWillChange]One suppression
InvalidPropertyAssignmentValueinSpan\Batch\SpanBatch:SpanInterfacecannot declare that it extends the generatedThrift\Span, even though its only implementation does.Because #18 regenerated the Thrift code with honest nullable types, the two suppressions that would otherwise be needed — papering over the old
@vardocblocks inTag\AbstractTagandSpan\Span— are not necessary at all.src/Thriftstays excluded: in full-project context psalm reports 221 issues there, including calls tothrift_protocol_*from the optional C extension.