Skip to content

feat(#176): read PHP's native phar containers - #458

Merged
helly25 merged 3 commits into
mainfrom
feat/176-phar-reader
Aug 11, 2026
Merged

feat(#176): read PHP's native phar containers#458
helly25 merged 3 commits into
mainfrom
feat/176-phar-reader

Conversation

@helly25

@helly25 helly25 commented Aug 11, 2026

Copy link
Copy Markdown
Owner

libarchive does not read the native phar format at all, so the archive extra gains its own reader behind the same Member shape the libarchive reader presents. A phar is a PHP script (the "stub") ending in __HALT_COMPILER();, followed by a binary manifest and then the member data - a layout no generic archive library recognises. The tar- and zip-based phar variants (.phar.tar, .phar.zip) need none of this: they are ordinary tars and zips that the libarchive reader already handles.

Entry points mirror the libarchive reader, in-memory and file-based: ListPharMembers, ListPharMembersOfFile, ReadPharMember, ReadPharMemberOfFile.

Details worth stating, each test-pinned:

  • Neither entry point reads a whole container. Listing reads the stub scan window plus exactly the manifest length the header declares; a content read then seeks to that one member's byte range. The stub scan is bounded (1 MiB) so handing the walk a large non-phar file cannot turn into a full read, and the declared manifest length is capped so a corrupt or hostile length field cannot become a huge allocation.
  • Every declared length is honoured even when the value is unused (the API version, the container alias, the per-member metadata). Skipping one does not fail - it shifts every following field, which is why a non-empty alias and a second member's data offset each have their own case.
  • A directory member is the trailing-slash spelling of a name; phar has no flag bit for it. The reported path drops the slash, and a read of it reports FailedPrecondition ("no content"), not NotFound.
  • A per-member compressed entry is refused with Unimplemented rather than returning the stored bytes (phar may deflate or bzip2 individual members). Returning them would make -grep silently miss a pattern the member does contain. Decompression is a follow-up slice; listing such members already works, since the manifest carries their metadata regardless.
  • Errors keep the libarchive reader's split: InvalidArgument for "not a phar" (the walk treats it as an ordinary file) versus DataLoss for a phar-shaped container whose manifest is truncated, lies about its member count, or points member data past the end.

Member-name normalization (./x and dir/ spellings) moves into xff_extras_api's member-path library as NormalizeMemberName, since both readers need exactly the same rule and that library owns member-path spelling. #457 added the same rule locally in archive_reader.cc; once #457 is in, that copy is replaced by this shared one (this branch does the swap when it syncs, so no duplication lands on main).

There is no phar writer to lean on, so the test builds containers byte by byte: the fixture is the format specification.

Not wired to --archive yet - that is the engine-mount slice, and AUTO-separator / per-format scheme questions (#177) stay open and untouched here.

Side note for a later slice: compile_commands-update.sh uses the extractor's refresh_all, which covers //... only, so no extras source is in the compile DB and clang-tidy cannot see this directory (it reports 'xff/archive/*.h' file not found for the existing reader too). Worth fixing separately.

Part of #176. Not armed for auto-merge; queued behind #455 / #456 / #457.

libarchive does not read the native phar format at all, so the archive extra
gains its own reader behind the same `Member` shape the libarchive reader
presents. A phar is a PHP script (the "stub") ending in `__HALT_COMPILER();`,
followed by a binary manifest and then the member data - a layout no generic
archive library recognises. The tar- and zip-based phar variants
(`.phar.tar`, `.phar.zip`) need none of this: they are ordinary tars and zips
that the libarchive reader already handles.

Entry points mirror the libarchive reader, in-memory and file-based:
`ListPharMembers`, `ListPharMembersOfFile`, `ReadPharMember`,
`ReadPharMemberOfFile`.

Details worth stating, each test-pinned:

- Neither entry point reads a whole container. Listing reads the stub scan
  window plus exactly the manifest length the header declares; a content read
  then seeks to that one member's byte range. The stub scan is bounded (1 MiB)
  so handing the walk a large non-phar file cannot turn into a full read, and
  the declared manifest length is capped so a corrupt or hostile length field
  cannot become a huge allocation.
- Every declared length is honoured even when the value is unused (the API
  version, the container alias, the per-member metadata). Skipping one does not
  fail - it shifts every following field, which is why a non-empty alias and a
  second member's data offset each have their own case.
- A DIRECTORY member is the trailing-slash spelling of a name; phar has no flag
  bit for it. The reported path drops the slash, and a read of it reports
  FailedPrecondition ("no content"), not NotFound.
- A per-member COMPRESSED entry (phar may deflate or bzip2 individual members)
  is refused with Unimplemented rather than returning the stored bytes.
  Returning them would make `-grep` silently miss a pattern the member does
  contain. Decompression is a follow-up slice; listing such members already
  works, since the manifest carries their metadata regardless.
- Errors keep the libarchive reader's split: InvalidArgument for "not a phar"
  (the walk treats it as an ordinary file) versus DataLoss for a phar-shaped
  container whose manifest is truncated, lies about its member count, or points
  member data past the end.

Member-name normalization (`./x` and `dir/` spellings) moves into
`xff_extras_api`'s member-path library as `NormalizeMemberName`, since both
readers need exactly the same rule and that library owns member-path spelling.

There is no phar writer to lean on, so the test builds containers byte by byte:
the fixture is the format specification.
Conflict: #455 (archive VFS backend) and this branch each appended targets to
extra_modules/archive/BUILD.bazel. Both sets are kept.

Also does what this PR promised once #457 landed: `archive_reader.cc` drops its
local copy of the member-name normalization and uses the shared
`NormalizeMemberName` from the member-path library, so the rule exists once for
both readers rather than being duplicated on main.
@helly25
helly25 enabled auto-merge (squash) August 11, 2026 08:05
The merge commit resolving this branch's BUILD conflict slipped past the local
trunk hook, which skips on a merge, so a dep inserted out of order only surfaced
in CI.
@helly25
helly25 merged commit dc691e6 into main Aug 11, 2026
11 checks passed
@helly25
helly25 deleted the feat/176-phar-reader branch August 11, 2026 08:23
helly25 added a commit that referenced this pull request Aug 11, 2026
…xtures

Retargeted onto main now that the phar reader landed. The three phar files
conflicted add/add because the reader was squash-merged; this branch's copies are
that content plus the detection fix, so they are kept whole.

Two real problems surfaced while syncing:

- The `end-of-file-fixer` and `trailing-whitespace` hooks APPENDED a newline to
  four of the containers. They begin with PHP source, so pre-commit's content
  sniff calls them text. The corruption was silent: a byte at EOF moves neither
  the manifest nor any member's data, so every functional test still passed - but
  PHP rejects the result, because the signature covers the whole file and its
  `GBMB` magic must be the last four bytes. The fixtures are regenerated (and
  verified with PHP: all nine open, 3 members each, signature intact).
- The hooks now exclude archive EXTENSIONS rather than that one directory: the
  hazard is the file type, not where it lives, so it holds for any container
  added anywhere. The repo-policy pygrep hooks get the same exclusion, since they
  also scan whatever is sniffed as text.

And a guard so this cannot recur quietly: phar_fixture_test asserts the `GBMB`
trailer is still the last four bytes of each uncompressed native fixture. Only
those - in the whole-file compressed and tar / zip variants the trailer is inside
the compressed stream or behind the format's own end record, and those files are
binary from byte 0, which is exactly why no text tool mistook them for editable.
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