Skip to content

fix: tolerate NUL suffixes in symbolic ref files#2658

Merged
Sebastian Thiel (Byron) merged 1 commit into
mainfrom
compliant-ref-parser
Jun 20, 2026
Merged

fix: tolerate NUL suffixes in symbolic ref files#2658
Sebastian Thiel (Byron) merged 1 commit into
mainfrom
compliant-ref-parser

Conversation

@Byron

@Byron Sebastian Thiel (Byron) commented Jun 20, 2026

Copy link
Copy Markdown
Member

Tasks

This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.

  • refackiew

Everything below this line was generated by Codex GPT-5.

Created by Codex on behalf of Byron. Byron will review before this is ready to merge.

Summary

Tolerate NUL suffixes in loose symbolic ref files by parsing the symbolic target only up to the first NUL byte, matching Git's C-string behavior for symbolic referents.

Reported issue

When refs (like HEAD) contains null bytes, Git will just ignore everything past it, but we currently fail. Instead, we should be as permissive as Git. Current behaviour leads to errors like this:

error: The reference at "HEAD" could not be instantiated: The path "refs/heads/main\0codex-hidden-head-metadata: status-clean-demo" to a symbolic reference within a ref file is invalid: Reference name contains invalid byte: "\0"

Git baseline

Local Git checkout 7760f83b59 shows parse_loose_ref_contents() in refs/files-backend.c stores symbolic referents through C-string semantics. A command-level check with a HEAD file containing ref: refs/heads/main\0codex-hidden-head-metadata: status-clean-demo returned refs/heads/main from git symbolic-ref HEAD.

Validation

  • cargo test -p gix-ref --test refs file::reference::parse::valid::symbolic_ignores_nul_suffix_like_git -- --exact
  • cargo test -p gix-ref --test refs file::reference::parse --
  • cargo clippy -p gix-ref --test refs -- -D warnings
  • cargo test -p gix-ref --test refs
  • codex review --commit 2e2c8a7f52f648369c054b6d060f5cde24f51a18

Loose symbolic ref files can contain bytes after a NUL terminator. Git treats
those bytes as invisible when reading the referent, so a HEAD file like
'ref: refs/heads/main\0...' still resolves to refs/heads/main.

gitoxide read the entire line until CR/LF before validating the symbolic target,
which made the embedded NUL fail refname validation.

Match Git by stopping symbolic target parsing at the first NUL byte, while
leaving direct object ref parsing unchanged.

Git baseline: local Git checkout 7760f83b59, refs/files-backend.c
parse_loose_ref_contents() uses C-string semantics for symbolic referents;
'git symbolic-ref HEAD' returns refs/heads/main for a HEAD containing
'ref: refs/heads/main\0codex-hidden-head-metadata: status-clean-demo'.

Co-authored-by: Sebastian Thiel <[email protected]>
@Byron
Sebastian Thiel (Byron) marked this pull request as ready for review June 20, 2026 12:33
Copilot AI review requested due to automatic review settings June 20, 2026 12:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates loose symbolic reference parsing in gix-ref to match Git’s C-string behavior by ignoring any bytes after the first NUL (\0) in ref: … files (e.g., HEAD). This makes the implementation more permissive and avoids errors when symbolic ref files contain NUL-suffixed metadata.

Changes:

  • Adjust symbolic ref parsing to stop the target path at the first NUL byte (in addition to CR/LF or end-of-input).
  • Update rustdoc to document the new parsing behavior.
  • Add a regression test asserting that NUL-suffixed symbolic targets parse and resolve to the prefix before the NUL.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
gix-ref/src/store/file/loose/reference/decode.rs Extend symbolic ref path termination logic to include \0, aligning parsing with Git behavior.
gix-ref/tests/refs/file/reference.rs Add a test ensuring NUL-suffixed symbolic ref contents parse as symbolic and use only the prefix before the NUL.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Byron
Sebastian Thiel (Byron) merged commit 1514b0f into main Jun 20, 2026
33 checks passed
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.

3 participants