GitIdentifiers: order tree entries by UTF-8 bytes as Git does

[lenamonj]

DirectoryEntry.compareTo used String.compareTo, which orders UTF-16 code units. Git orders raw UTF-8 bytes; outside the Basic Multilingual Plane the two differ, so treeId disagreed with git write-tree for a tree holding U+FF21 and U+1F600.

The sort key is now the name's UTF-8 bytes, compared unsigned. Names below U+D800 keep their order, so the existing constants are unchanged. The new test pins the id git write-tree produces and fails on the old comparator.

Checklist: guidelines and the ASF generative-tooling guidance read. AI was used: Claude (Anthropic) found the defect and drafted the fix and test in an audit loop I run; I reviewed it, reproduced the id with git write-tree, and ran the default Maven goal, which passes. changes.xml updated.

[Copilot]

🟡 Changes recommended

The critical malformed-name handling issue remains unresolved, along with two documentation nits.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This pull request updates Git tree-entry ordering to compare unsigned UTF-8 bytes, matching Git’s behavior for Unicode names.

Changes:

• Replaces UTF-16 string comparison with unsigned UTF-8 comparison.
• Adds regression coverage for supplementary Unicode characters.
• Documents the fix in changes.xml.

File summaries

File	Summary	Review comments
src/main/java/org/apache/commons/codec/digest/GitIdentifiers.java	Implements UTF-8 byte-based ordering.	Critical (2 votes): Malformed UTF-16 can produce duplicate sort keys and cause TreeSet to drop entries; reject malformed names or enforce unique keys. Nit (3 votes): Rephrase malformed Javadoc.
src/test/java/org/apache/commons/codec/digest/GitIdentifiersTest.java	Adds Unicode ordering regression tests.	Nit (1 vote): Correct the description distinguishing BMP and supplementary-plane characters.
src/changes/changes.xml	Records the bug fix.	No final comments.

Review details

Suppressed comments (1)

src/test/java/org/apache/commons/codec/digest/GitIdentifiersTest.java:228

• U+FF21 is in the Basic Multilingual Plane, so this sentence incorrectly describes both examples as names outside the BMP. The ordering difference is between a BMP character and a supplementary-plane character; please reword the test documentation to make that distinction accurate.

     * Tree entry names are ordered by their UTF-8 bytes, which is not the order {@link String#compareTo(String)} gives for names outside the Basic Multilingual
     * Plane: U+FF21 encodes to {@code EF BC A1} and U+1F600 to {@code F0 9F 98 80}, so Git sorts U+FF21 first, while the UTF-16 code units place the surrogate
     * pair of U+1F600 first.

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[garydgregory]

@lenamonj
Please review copilot's comments.

[ppkarwasz]

@lenamonj,

Nice catch! 💯

I completely overlooked that both Git and SWHID (https://www.swhid.org/) treat file names as plain byte sequences.

That raises a question, though: did we miss a Charset parameter in the API? Encoding file names in a Git repo with anything other than UTF-8 is technically possible, but it's a recipe for collaboration headaches.

I'm okay with just assuming UTF-8 for file names, but we should spell that out somewhere:

Git and SWHID treat file names as opaque byte sequences with no defined encoding. The identifiers produced here coincide with Git's or SWHID's own identifiers only if the original file names were UTF-8 encoded.

[lenamonj]

The class Javadoc now carries that paragraph, with symbolic link targets added, since Git also stores a link target as bytes with no defined encoding. Every name and link target has been encoded with StandardCharsets.UTF_8 since #428, so the paragraph describes what the code already does and the API does not change.

[lenamonj]

getBytes(UTF_8) turns a lone surrogate into ?, so a name holding one shared its sort key with ? and the TreeSet kept only one of them; compareTo now falls back to the String order when the bytes tie, which keeps both as the String comparator did, and never fires for well-formed names, whose UTF-8 encoding is unique; testTreeIdKeepsNamesWithTheSameUtf8Bytes covers it. Both Javadoc sentences are reworded. Two such names still serialize to the same byte, and git hash-object -t tree -w refuses that tree with duplicateEntries, so rejecting names UTF-8 cannot encode would stop treeId from hashing a tree git cannot store; that changes what it accepts, so it is not in this change unless you want it.

[garydgregory]

Hi @lenamonj and @ppkarwasz
Which UTF-8 Normalization does this PR enforce? Google tells me git uses Form C (NFC).

[lenamonj]

None, and Git's object model enforces none either: a tree object stores the name bytes it was given, so the NFC and NFD spellings of one name are two entries with two different ids. On git 2.53.0 on Linux, git mktree accepts a tree holding both caf\303\251 and cafe\314\201, git fsck --strict passes it, and git write-tree over a directory holding both files gives 79bc87b55119be79977a4f03942118bf8f06fadc; treeId at 7ed2bae over the same directory gives the same id. The NFC that Google reports is core.precomposeUnicode, a macOS-only option: compat/precompose_utf8.c wraps readdir and converts every non-ASCII name to NFC through iconv before it reaches the index, and git init on macOS turns it on when the filesystem treats the two spellings as one file. So on a Mac, git write-tree hashes NFC names while treeId hashes the names as the JDK returns them, and the two agree there only for names that are already NFC. The Javadoc paragraph can carry both facts, that names are hashed with no normalization and that git's macOS build precomposes first, if you would like them spelled out.

[garydgregory]

@ppkarwasz any thoughts on merging this PR?

[ppkarwasz]

Let's merge it.

@lenamonj, thanks again! 💯