KarpelesLab / goro / 24634655988

13%

Build Type

push

github

Committed by MagicalTux

Commit Message

phpv: introduce ZVal.SetName to protect cached ZVals from Name pollution

Small integers (-1..256) and bools return shared cached ZVal wrappers from
ZInt.ZVal()/ZBool.ZVal() (see commits d5439b22 / be64b90a). Mutation
through Set() and MakeRef() is already guarded with panics, but several
call sites were writing to the public Name field directly on whatever
ZVal came back from ZVal() — pollution that never cleared because
subsequent readers of ZInt(1).ZVal() got the same instance back.

This surfaced as arg_unpack/*.phpt failing with diffs like:

  array(5) {
  -  [0]=>
  +  ["count"]=>
     int(1)
  ...

A prior call somewhere in the test suite set `.Name = "count"` on the
cached ZVal for the integer 1; later positional arguments that happened
to evaluate to 1 picked up the stale name, and the variadic packer used
it as the array key instead of a numeric index.

Fix by introducing ZVal.SetName(*ZString) which Dups first when the
receiver is cached, and swap the unsafe `.Name =` assignments for it at
all sites that feed from a *.ZVal() call whose result may be cached:

  core/phpctx/global-call.go:314 (named-arg-to-variadic tagging)
  core/params.go:407 (by-ref undefined-var binding)
  ext/openssl/random.go (openssl_random_pseudo_bytes crypto_strong)
  ext/openssl/pkey.go (openssl_pkey_export output arg)
  ext/openssl/encrypt.go (GCM tag by-ref)
  ext/standard/network_socket.go (stream_socket_* error/peer args)

The other .Name writes in the tree were audited: they either follow an
explicit Dup() (so the target is already fresh) or operate on hash-table
entries (which hashtable.go copies cached instances into on insert).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Coverage Stats

0 of 23 new or added lines in 7 files covered. (0.0%)

308 existing lines in 7 files now uncovered.

2188 of 111917 relevant lines covered (1.96%)

70.38 hits per line