Skip to content

1/5 🔒 Upgrade to apache/thrift 0.24 and regenerate src/Thrift - #18

Open
dozer111 wants to merge 3 commits into
code-tool:3.0.xfrom
dozer111:v4/thrift-0.24
Open

dozer111 wants to merge 3 commits into
code-tool:3.0.xfrom
dozer111:v4/thrift-0.24

Conversation

@dozer111

Copy link
Copy Markdown

Stacked series, 1 of 5. The branches live in my fork, so every PR in the series targets 3.0.x directly and its diff includes the preceding stages. Merging them in order collapses each diff to its own commits.

  1. this PR — apache/thrift 0.24 + regenerated src/Thrift (3 commits)
  2. php-cs-fixer + rector (4 commits)
  3. psalm at level 1 (2 commits)
  4. PHPUnit 13 suite + CI (3 commits)
  5. five defect fixes (5 commits)

First of five stacked PRs for the 4.0 major.

Why

apache/thrift < 0.24.0 is affected by CVE-2026-43871 (GHSA-8wv5-x4w7-5gww) — an infinite loop in the PHP, Python, Go and Java bindings, severity high. The whole previous >=0.11, <0.17 range is unusable and 0.24.0 is the only release that is not.

What

  • apache/thrift: ^0.24.0, php: ^8.4 (0.24 itself needs ^8.1; the major targets 8.4 so the toolchain lands on current stable releases)
  • TUDPTransport modernised — see below
  • src/Thrift regenerated with compiler 0.24 against jaeger-idl v0.12.0

TUDPTransport

  • socket_create() returns a Socket object since PHP 8.0, so is_resource($this->socket) was always false and a fresh socket was created on every flush(). Descriptors did not leak, but each flush paid for a needless socket_create() + socket_connect(), and close() was a no-op in practice.
  • Parameter types now match the strictly typed TTransport in 0.24.
  • read() returned '' unconditionally, so the inherited readAll() spun forever — the same infinite-loop shape as the CVE. It throws TTransportException now.

Regeneration

The tree was generated by compiler 0.15 while the runtime is 0.24. Regenerating brings them in line: declare(strict_types=1) (THRIFT-5986), native property/constructor types (THRIFT-5991) so public $key becomes public ?string $key, native return types (THRIFT-5990), PSR-12 (THRIFT-5959).

bin/thrift-gen.sh cloned the IDL from a moving main. It now pins v0.12.0, requires compiler 0.24.x, and generates into a mktemp dir. Running it twice is a no-op — verified.

⚠️ Breaking

24 classes removed: AggregationValidator, BaggageRestrictionManager, Dependency, ThrottlingService and their DTOs. They came from IDL definitions Jaeger removed upstream long ago (already absent in jaeger-idl v0.11.4) — server-side APIs this client never used. Nothing in src/ or tests/ referenced them.

Verified

Struct round-trips over binary and compact protocols, AgentClient::emitBatch over a real UDP socket, and the reflection contract against TTransport.

dozer and others added 3 commits September 16, 2026 16:18
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant