Skip to content

fix(container): use scoped binding for parser - #12

Merged
danielebarbaro merged 2 commits into
mainfrom
fix/octane-safe-container-binding
Sep 8, 2026
Merged

danielebarbaro merged 2 commits into
mainfrom
fix/octane-safe-container-binding

Conversation

@danielebarbaro

@danielebarbaro danielebarbaro commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Problem

KmlParserServiceProvider registers KmlParser as a singleton, but the parser holds the loaded document as mutable instance state (protected ?SimpleXMLElement $xml).

Under Octane, or inside a long running queue worker, the container is reused across requests/jobs. That means:

  1. Request A calls KmlParser::loadFromFile('customer-a.kml').
  2. Request B resolves the same instance and calls getPlacemarks() without loading anything.
  3. Request B silently receives request A's data instead of the expected No KML data loaded exception.

Cross-request data leakage, and it fails open (returns wrong data) rather than erroring. Note that phpstan.neon.dist already sets checkOctaneCompatibility: true; static analysis does not catch this class of stateful-singleton issue.

Fix

Swap singleton() for scoped(). A scoped binding behaves exactly like a singleton for the duration of a request or job, so the chained facade usage documented in the README keeps working:

KmlParser::loadFromFile('path/to/file.kml')->getPlacemarks();

but the container flushes it between lifecycles, so nothing carries over.

Tests

Three tests in tests/ServiceProviderTest.php, the third one being the actual regression test (it fails on main):

  • the instance is reused within a single scope
  • a fresh instance is resolved after forgetScopedInstances()
  • a document loaded before the flush is no longer readable after it

Notes

  • No breaking change for the usage patterns shown in the README.
  • vendor/bin/pint --test currently reports src/KmlParser.php as unformatted on main already. Untouched here to keep the diff focused.

KmlParser holds the loaded SimpleXMLElement as instance state, so the
singleton binding kept a parsed document alive across Octane requests
and across jobs in a long running queue worker. Any consumer resolving
the parser without loading first could read another request's document.

A scoped binding keeps the instance stable for the whole request or job
(so the documented facade chaining still works) while the container
flushes it between lifecycles.
@danielebarbaro
danielebarbaro force-pushed the fix/octane-safe-container-binding branch from dacfcff to c92785b Compare September 8, 2026 12:33
@danielebarbaro
danielebarbaro merged commit 49388a1 into main Sep 8, 2026
@danielebarbaro
danielebarbaro deleted the fix/octane-safe-container-binding branch September 8, 2026 12:37
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