CEP XXXX: Fix directory-hash collision via length-prefixed fields (v2)

[pb01ka]

Checklist for submitter

• I am submitting a new CEP: CEP XXXX - Computing the hash of the contents in a directory (v2).
  • I am using the CEP template by creating a copy cep-0000.md named cep-XXXX.md in the root level.
• I am submitting modifications to CEP XX.
  • The PR title reflects the CEP I'm modifying: CEP XX: Amend XYZ.
  • I updated the "Updated" date.
  • I added the link of this PR to the Discussions row.
  • I added or extended the ## Changelog section right above the final "Copyright" section with an item that uses syntax YYYY-MM-DD: Brief explanation of changes.
• Something else: (add your description here).

Checklist for CEP approvals

• The vote period has ended and the vote has passed the necessary quorum and approval thresholds.
• A new CEP number has been minted. Usually, this is ${greatest-number-in-main} + 1.
• The cep-XXXX.md file has been renamed accordingly.
• The # CEP XXXX - header has been edited accordingly.
• The CEP status in the table has been changed to approved.
• The last modification date in the table has been updated accordingly.
• The table in the README has been updated with the new CEP entry.
• The pre-commit checks are passing.

Summary

• Introduces CEP XXXX, which supersedes CEP 19 by fixing a hash-collision vulnerability in the directory-contents hashing algorithm.
• Adds a length prefix (<len>:) to every variable-length field (relative path and symlink target) fed into the hasher, making all field boundaries unambiguous and provably collision-free.
• Deprecates the existing content_sha256 / content_sha384 / content_sha512 recipe keys in favour of new _v2 variants validated with the fixed algorithm; the legacy keys continue to work (with a PendingDeprecationWarning) for backwards compatibility.

TODO - changes required in cep-XXXX.md based on review thread

Review thread - #174 (review), #174 (comment)

Sorting / ordering

• Change the sort definition from "ascending lexicographical comparison using numerical Unicode code points" to lexicographic comparison of raw UTF-8 bytes after NFC normalization. Remove the footnote referencing Python's < operator (it becomes unnecessary).

Path normalization rules

• Add an explicit rule: a leading ./ MUST be stripped (e.g. ./README.txt -> README.txt).
• Add an explicit rule: the scanned directory itself (.) MUST NOT be included as an entry (rename "root directory" to "scanned directory" to avoid confusion with /).
• Add an explicit rule: paths MUST be relative; a leading / MUST be rejected for entry paths.
• Add an explicit rule: drive letters (e.g. C:) and UNC prefixes (e.g. //server/share) MUST be stripped from entry paths before processing - Went as per @jaimergp's thoughts on this.
• Add an explicit rule: null bytes (\0) in any path component MUST cause the implementation to error out.
• Add a note on \0, /, \\, : in filenames referencing their history as sources of archive-extraction vulnerabilities (or at minimum recommend that implementations reject them for consistency).
• Add a short example to the Examples section showing a dotfile (e.g. .gitignore) in a directory tree, to demonstrate the sort position of names beginning with ..

Unicode normalization

• Mandate NFC normalization of all paths before encoding to UTF-8. Add: "Before encoding a path to UTF-8 for sorting or hashing, implementations MUST normalize it to NFC."
• Update the sort description to say: "Paths MUST be NFC-normalized and then UTF-8-encoded before sorting; the sort MUST be a lexicographic comparison of the resulting UTF-8 bytes."
• Add: "If NFC normalization produces two entries with the same path, implementations MUST error out rather than silently feeding duplicate bytes into the hasher."

Symlink target normalization

• Split the normalization rules into two explicit lists:
  • Entry paths: relative only; leading /, drive letters, and UNC prefixes MUST be rejected.
  • Symlink targets: MAY be relative or absolute. A leading / is legitimate (valid absolute Unix path) and MUST be preserved. Windows drive letters (e.g. C:) and UNC prefixes (e.g. //server/share) MUST cause the implementation to error out, because there is no meaningful cross-platform way to normalize them into the hash stream.
  • Backslash -> forward-slash and redundant ./.. removal still apply.
• Clarify "normalized as above" in the symlink-target section to point only to the separator/component rules that actually apply to targets.

File content hashing (text vs. binary)

• Remove the text/binary distinction entirely: drop the UTF-8 decode attempt and the \r\n -> \n line-ending normalization - Went as per @jaimergp's thoughts on this.
• Always treat file contents as raw bytes and feed them directly into the hasher.
• Update the reference implementation accordingly (remove the try/except UnicodeDecodeError block and always open files in binary mode "rb").
• Add a note explaining the rationale: cross-platform line-ending consistency is the responsibility of the tooling that produces the directory (e.g. git .gitattributes), not of the hashing algorithm.

Python surrogateescape / non-UTF-8 filenames

• Add a note that the spec requires UTF-8-encodable filenames and that Python's surrogateescape error handler (PEP 383) may silently accept non-UTF-8 filenames on Unix.
  The reference implementation MUST encode paths such that non-UTF-8 filenames cause an explicit error rather than producing surrogates in the hash stream.

Add @hunger as co-author

• Firstly, many thanks to @hunger for his insightful reviews and discussions. Since his reviews will change the CEP and its implementation significantly, I intend to add him as co-author to this CEP (and in all the commit messages of this PR).

[jaimergp]

Let's move this example to the Examples section, merging it in Example 3.

[jaimergp]

I'll note that this section is supposed to explain decisions made in the specification. As such, rules with MUST or SHOULD do not belong here. Python PEPs usually refrain from dictating UX for tools, though, so I don't think we should use RFC language here. Let's lowercase those suggestions because they are not part of the specification.

[pb01ka]

@jaimergp I have addressed your reviews. Please let me know if there is any other thing we can change here. Thanks.

[jaimergp]

@hunger, as the author of #150, do you want to take a look here? Thanks!

[hunger]

@jaimergp: I am honored to take a look :-)

[hunger]

I think this is a step up from CEP-19, thank you for doing the work to get this fixed. I very much appreciate it.

One tiny nit-pick: I found the "utf8 encoded bytes" phrasing a bit confusing for ASCII letters, which are all 1 byte.

Some of my comments are over the top for a hasher. I registered them anyway as I assumed you are going to calculate a hash for something you want to package up or process in some other way and I expect you want to make sure the hasher and the tool using the hashed contents will agree on the files they accept or reject.

[hunger]

Any ordering will do here. We can not use the raw bytes straight from the file system though, as that would not be cross platform. You mandate utf-8 encoded file names later.

The cheapest would IMHO be comparing the raw utf8 bytes. This avoids extracting the unicode code point for a single comparison.

[pb01ka]

You are right that we cannot use raw filesystem ordering - os.listdir() / Path.rglob() return entries in OS-dependent (and sometimes non-deterministic) order, so the hash would differ across platforms without an explicit sort. The sort is mandatory.

On the UTF-8 bytes point: agreed. UTF-8 was specifically designed so that lexicographic order on raw UTF-8 bytes is identical to lexicographic order on Unicode code points. Since we already mandate UTF-8 for all path names, comparing the raw rel.encode("utf-8") bytes and comparing the decoded Unicode strings produce exactly the same ordering - there is no semantic difference. Sorting on bytes is therefore a valid and marginally cheaper alternative (no decoding step needed for the comparison key).

I will update the spec to say "sort by raw UTF-8 bytes" instead of "sort by numerical Unicode code points". The footnote referencing Python's < operator will be removed since the byte-comparison framing is self-contained and does not need a language-specific reference.

[hunger]

I assume this is all text-based without looking up real files in the file system?

You could add "." into the example, so implementers won't forget to handle that.

What about leading '/'? Is that allowed? I think it should not be, as you are hashing a subtree.

What about the Windows drive letters, UNC paths?

[pb01ka]

I assume this is all text-based without looking up real files in the file system?

Yes. Specifically, the examples you are looking at are constructed by hand to illustrate the byte stream; The byte streams shown are exactly what the algorithm would produce if run against the described directory structure.

---

You could add "." into the example, so implementers won't forget to handle that.

I think the two things the spec should clarify here are,

1. A leading ./ in a relative path MUST be stripped (e.g. ./README.txt -> README.txt). The current normalization bullet only mentions ..-removal; I will add an explicit rule for .-stripping.
2. The root directory itself (.) MUST NOT be included as an entry. AFAIK, Python's rglob("*") never yields the root, but implementations in other languages (if done) that walk directories manually (e.g. using readdir) may encounter . and .. entries and must skip them. I will add a sentence to the spec making this explicit, and add a short example showing a dotfile (e.g. .gitignore) in a tree so the sort position of names starting with . is demonstrated.

---

What about leading '/'? Is that allowed? I think it should not be, as you are hashing a subtree.

Agreed - a leading / would make the path absolute, which contradicts the relative-path semantics of the algorithm. In the reference implementation path.relative_to(directory) guarantees this can never happen for entry paths, but other implementations could accidentally produce it. I will add an explicit normalization rule: "Paths MUST be relative; a leading / MUST be rejected."
Also, symlink targets are hashed as literal strings (not resolved), so an absolute symlink target like /usr/lib/foo is stored as-is with its / intact - that is intentional and I will add a note to distinguish the two cases.

---

What about the Windows drive letters, UNC paths?

AFAIK, for entry paths this cannot arise. path.relative_to(directory) strips any drive letter or UNC prefix, and after backslash-to-forward-slash normalization the result is a plain relative path. I will add a normalization rule making this explicit for non-Python implementations: "Drive letters (e.g. C:) and UNC prefixes (e.g. //server/share) MUST be stripped from entry paths before processing."

For symlink targets an absolute Windows path or UNC target could appear in principle. The spec should state that implementations MUST error out if a symlink target contains a drive letter or UNC prefix, because there is no meaningful cross-platform way to normalize those into the hash stream.

[hunger]

I misunderstood your point 2) at first. A better wording might be:

The scanned directory itself (.) MUST NOT be included as an entry.

root directory made me think of '/'.

Good catch, I totally missed this one.

[jaimergp]

"Drive letters (e.g. C:) and UNC prefixes (e.g. //server/share) MUST be stripped from entry paths before processing."

I'd argue those conditions should result in an error. No stripping.

[mgorny]

Actually, why are we doing this? Since the input is from a scanning function, I suppose such entries can only be symlink targets. Why would we want to consider foo/../bar the same as bar? Presumably if you use two different targets, there is a reason for the difference, right?

[hunger]

This is over-specifying things a bit for a hash, I agree. But a hash will be used together with a archive in our use case, and we need to make sure that the hash and the archive generated will agree on the files they work on. It makes no sense to hash one set of files and then package up a (slightly) different set.

[hunger]

This seems very wrong to me: You are changing file contents for the hashing. That is a new way to introduce hash collisions!

Just treat all files as binary and do not try to make any sense of the contents whatsoever.

[pb01ka]

You are right that this creates an intentional hash collision: a file containing hello\r\nworld and a file containing hello\nworld feed identical bytes into the hasher and therefore produce the same digest, even though their raw byte content differs.

The line-ending normalization was inherited from CEP 19 with the goal of cross-platform reproducibility: a conda recipe checked out on Windows (where git may materialise \r\n) should hash the same as when checked out on Linux (where git materialises \n).

However, your concern is well-founded for two reasons,

1. It really is a collision. Regardless of whether the equivalence is intentional, two files with different raw bytes silently hash to the same value. For a hash used in security contexts (e.g. verifying a package has not been tampered with) that is a meaningful weakness: an attacker who can substitute a CRLF version of a file for an LF version bypasses the check.

2. The text/binary heuristic is fragile. Any file whose bytes happen to be valid UTF-8 is treated as text and has its line endings silently transformed. AFAIK, many binary formats (e.g. certain compiled outputs; please correct me if I'm wrong) are legal UTF-8 and would be misclassified. The Python reference implementation compounds this by opening files in text mode first, which on Windows already performs platform-level CRLF translation before the explicit \r\n -> \n replacement.

The cleaner fix is to drop the text/binary distinction entirely and treat all file contents as raw bytes, as you suggest. Cross-platform consistency should be the responsibility of the tooling that produces the directory (e.g. git's .gitattributes), not of the hashing algorithm. The
hashing algorithm should be a pure function of the bytes on disk and nothing more.

I will update the spec and the reference implementation accordingly: remove the UTF-8 decode
attempt, remove the \r\n -> \n replacement, and always feed the raw file bytes into the hasher.

@jaimergp - does this direction make sense to you, or do you think the cross-platform line-ending consistency is important enough to keep in the algorithm?

[hunger]

I fully agree with @pb01ka

[jaimergp]

This was added to prevent unix vs win checkouts producing different hashes. .gitattributes may not help here if there are data files which are meant to be newline-normalized for the target platform. Maybe we can offer it as an optional step so implementers can pick whether to add it as an additional method, while mentioning the caveats for potential collision?

[hunger]

Makes a lot of sense!

[hunger]

Is it OK to have a target reaching out of the tree we are hashing?

Is it OK for a target to be a absolute path or start with drive letters, etc.? I would think that is OK here, but it makes for a different normalization as above.

[pb01ka]

Is it OK to have a target reaching out of the tree we are hashing?

Yes, and it is already handled correctly by the current spec.

The algorithm never follows symlinks (line: "without following symlinks") - it hashes the target string as literal bytes, not the content of whatever the target points to. So a symlink with target ../../outside/file contributes those exact bytes to the hash stream. If the content at the target changes, the hash is unaffected, but that is true of all symlinks regardless of whether the target is inside or outside the tree.
The hash captures the structure and metadata of the tree being scanned; content outside that tree is explicitly out of scope. I think no change should be needed here.

Please let me know if you think there is a case where this behavior is not what you want, and we can discuss.

---

Is it OK for a target to be a absolute path or start with drive letters, etc.? I would think that is OK here, but it makes for a different normalization as above.

Agreed - absolute symlink targets are valid and should be allowed. A symlink to /usr/lib/libfoo.so or C:/Windows/System32/foo.dll is perfectly legal and meaningful on its respective platform, and the hash should faithfully record that target string.

You are right that the normalization rules differ from those for entry paths:

• Entry paths must be relative; a leading / or a drive letter is a bug in the implementation and MUST be rejected.
• Symlink targets may be relative or absolute; a leading / or drive letter is legitimate data and MUST be preserved.

The only normalization that applies to both is the mechanical one: backslashes -> forward slashes, and removal of redundant . / .. components. The spec currently says "normalized as above" for symlink targets, which is ambiguous because "as above" also implies the relative-path constraints.

I will clarify this by splitting the normalisation rules into two explicit lists: one for entry paths (relative-only) and one for symlink targets (absolute or relative, separators and redundant components only).

[hunger]

I suggest not doing line ending normalization nor touching the contents of any file in any way.

[pb01ka]

I have discussed this in my reply above.

[hunger]

This can in theory lead to more hash collisions as filenames using different encodings might end up with the same utf-8 file name. Just pointing that out here: I think you can not avoid that while having file names that are valid cross-platform.

You might want to mandate a unicode normalization form to be used? You might end up with the same file name having different hashes if you leave that out.

Should all characters be allowed? /, \\ and \0 and : might be problematic in a file name... "a:\foo" should probably not be a valid file name.

[pb01ka]

This can in theory lead to more hash collisions as filenames using different encodings might end up with the same utf-8 file name.

Agreed, and the root cause might be Unicode equivalence rather than encoding per se. The spec already mandates UTF-8 encoding of paths, so the problematic case is not two different encodings for the same byte sequence, but rather two different Unicode representations of the same logical character.

For example, the filename é can be stored as:

• NFC: U+00E9 (2 UTF-8 bytes: 0xC3 0xA9)
• NFD: U+0065 + U+0301 (3 UTF-8 bytes: 0x65 0xCC 0x81)

macOS stores filenames in NFD; Linux stores whatever bytes the application writes (?). A recipe hashed on macOS and the same recipe hashed on Linux could therefore differ on any filename containing a composed character. So we might end up in a permutation of characters leading to the same byte stream.

I think you are right that this cannot be fully eliminated while supporting all cross-platform filenames, but mandating a normalization form (see below) should close the practical gap.

---

You might want to mandate a unicode normalization form to be used? You might end up with the same file name having different hashes if you leave that out.

Good suggestion - I will add NFC as the required normalization form. I think NFC is de-facto standard in Python's standard library (it lets you use NFD), and in the vast majority of web and network protocols. Please correct me if I'm wrong.

Before encoding a path to UTF-8 for hashing or sorting, implementations MUST normalize it to NFC using their platform's Unicode library (e.g. unicodedata.normalize('NFC', path) in Python).

This makes the hash independent of whether the underlying filesystem stores filenames in NFC or NFD.

[pb01ka]

Should all characters be allowed? /, \\ and \0 and : might be problematic in a file name... "a:\foo" should probably not be a valid file name.

Each one explained in the following pointers,

• / and \\: These are already handled. Backslashes are normalized to forward slashes, and forward slashes are path separators - they delimit components, they cannot appear inside a component after normalization. Any path that still contains a bare / inside a component after normalization is malformed and MUST be rejected.

• \0 (null byte): AFAIK, Null bytes are not valid in filenames on any mainstream OS. If a path component contains \0 the implementation MUST error out. I will add this to the normalization rules.

• :: On Linux, : is a legal filename character. On Windows, it is forbidden in ordinary filenames (because it is used for drive letters?). The a:\foo form is a Windows drive path and is already covered by the "drive letters MUST be rejected for entry paths" rule from the earlier discussion. A bare : inside a filename component (e.g. foo:bar) is unusual but does not conflict with any part of the hash stream format (: is used as a length-prefix delimiter, but only at the position immediately after the decimal length, which is unambiguous). I think I will leave : inside components as implementation-defined for now.

[hunger]

/, \\, \0 and : in malicious tar archive have been a constant source of problems, so they should not be allowed anywhere close to a tarball (or zip file ;-). Sure, you can store those safely, but you need to expect some tools to fail to extract that data safely. So there is a case to be made for conda to reject anything that looks fishy when people try to put it into or get it out of packages.

The hasher is a pretty harmless component in itself, but it might make sense to apply the same rules to the hasher as to the packager. Otherwise you might end up with the hasher considering a different set of files from a packaging tool, which would be unexpected.

[hunger]

I assume you mean utf8 string here? Technically are several encodings for unicode strings.

You might want to add that these strings must be normalized as specified below and deduplicated. Might be overdoing it, but somebody might try to throw a directory and a file with the same name into the hasher.

[pb01ka]

I assume you mean utf8 string here? Technically are several encodings for unicode strings.

Yes, UTF-8 is the intended encoding. The current wording "sort by their full path as a Unicode string" is ambiguous.

Following my previous replies about mandating NFC normalization, I will tighten this to:

Paths MUST be NFC-normalized and then UTF-8-encoded before sorting, and the sort MUST be a lexicographic comparison of the resulting UTF-8 bytes.

This removes any ambiguity about which Unicode encoding or normalization form governs the ordering.

---

You might want to add that these strings must be normalized as specified below and deduplicated. Might be overdoing it, but somebody might try to throw a directory and a file with the same name into the hasher.

On normalization order

The current spec normalizes paths before hashing them (lines below the sort) but does not say whether the sort is performed on the raw or the normalized form.

I will make this explicit: normalization MUST be applied first, and the sort MUST be performed on the normalized paths.

On deduplication

The deduplication check is on the full normalized path, not the basename alone. A directory foo/ and a file foo/foo have different full paths and are never duplicates; neither are two entries named bar in different subdirectories.

On a real filesystem, two entries with the exact same full path genuinely cannot exist: the OS enforces name uniqueness within each directory, so the full path is globally unique within the tree.

The only practical edge case arises after NFC normalization on a case-sensitive filesystem such as Linux: two entries whose raw byte names differ (e.g. one stored in NFD, one in NFC) but collapse to the same NFC-normalized path would be indistinguishable to the algorithm. This might be rare, but the spec should be precise:

If normalization produces two entries with the same path, implementations MUST error out rather than silently feeding duplicate bytes into the hasher.

[hunger]

Sounds good to me.

[pb01ka]

@hunger I responded to your review comments. If I misinterpreted your reviews then please let me know in the respective thread. I tried to be verbose in my replies.

I will make the changes to the CEP tomorrow and subsequently push the commits.

Thank you for your insightful review.

[hunger]

Another random thought: My python is severely limited, but I remembered in the back of my head there was something funny happening there wrt. filenames and utf8 in python.

Does the "surrogateescape" mechanismn effect your filesystem encoding, considering that your reference implementation is in python (a quick serch turned up pep-383, but this blog has a good summary of the discussions that happened in python)? As I read it, this standardizes encoding file names in a not fully utf8-compliant way into utf8 strings.

Or do we need something like the surrogateescape mechanismn? IIRC it allows round-tripping legacy utf-16 windows filenames and deal with legacy "what encoding is this?!" filenames on unix systems.

The CEP rules this out explicitly by requiring utf8 encoded strings as I read the proposal. But considering your reference implementation uses python, it might be worthwhile to mention this behavior explicitly.

[pb01ka]

Thank you @hunger for in-depth discussion on this CEP. Based on our discussion I have added a TODO list in the description of the PR. Please go through it and let me know if you think I misunderstood anything from your comments.

Please take a special look at Symlink target normalization. I tried to find a middle ground for erroring out. Please let me know if you have different thoughts than mine.

Thank you again. ❤️

cc: @jaimergp

[hunger]

@pb01ka: I think you covered everything I threw at you :-)

[pb01ka]

Just a heads up folks - I will start working on my TODO list by tomorrow or Sunday. :)).

[pb01ka]

@hunger @jaimergp I have addressed all the TODOs in the description of this PR. Please review the latest changes to the CEP and let me know if any further improvements can be made. TYSM. <3.

[mgorny]

I could be wrong but I think "lexicographic" is ambiguous, as that could depend on collation used. Is this referring to byte values?

[mgorny]

Also, perhaps worth making it clear that we're dealing with relative paths? Especially "full path" makes one think of absolute paths.

[mgorny]

This is a bit ambiguous whether "encoded as UTF-8" refers to "the decimal representation" or "the normalized relative path" (I suppose both).

[mgorny]

Suggested change

	- The UTF-8 encoded bytes of `F`.
	- The UTF-8 encoded byte of `F`.

We know it's always one byte.

[mgorny]

This is confusing. Are we feeding raw unmodified bytes or newline-substituted bytes now?

[mgorny]

Unless I'm missing something, this is contradictory:

If the file can't be opened, it MUST be handled as if it were empty

If the file can't be read, implementations MUST error out.

[mgorny]

Perhaps worth mentioning (if intentional) that this doesn't cover all error paths (i.e. disallowed paths per the rules).

[mgorny]

File contents can introduce hash collision here. Consider the first example:

10:README.txtFHello\n-3:srcD-

A directory containing only README.txt with Hello\n-3:srcD produces the same hash.

[jaimergp]

Do we need the number of items in the directory tree too?

[jaimergp]

Or the number of bytes in the file?

[mgorny]

I suppose this is outdated since you forbid null bytes in filenames. They could occur in data, though.

[pb01ka]

@mgorny @jaimergp @hunger I have addressed all of your reviews received so far. Can you please take a look at the latest changes I have made and let me know if there is anything else which can improved. TYSM.

[jaimergp]

This goes before content bytes, right? Also should it be the length of the content bytes, or the length of the normalized content bytes. I think the latter, otherwise we get different hashes per platform.

[mgorny]

Perhaps we should move "content bytes normalization" into a separate algorithm, and then refer to it here; both for length (i.e. "length of normalized content, per algorithm …" and for content bytes ("normalized content, per algorithm …").

[jaimergp]

This is dictating implementation choices for tools; the specification shouldn't concern itself with that detail, IMO. Good to note, sure, but not in this section. Perhaps just leave it in the Reference Implementation section.

[hunger]

I would encourage you to remove this text file handling. It is error prone and out of scope for a hash function IMHO. That should be able to detect differences between files, not normalize files.

[pb01ka]

As per, #174 (comment) we decided to have the line ending normalisations. However as per the above comment we shouldn't have it.

@jaimergp What do you say? Should we have the line ending normalisation?

IMO, we should have it because otherwise the hash values will become platform dependent. Anyone who is not aware of this specific detail might run into a problem. Two alternatives I can think of,

• Include line ending normalisation in the specification by default. Allow an option to not have it.
• Exclude line ending normalisation from this specification. However, recommend to give warnings on platforms (like Windows) while hashing files with \r\n line endings. Through this someone who is unaware of this minor detail, will get to know that the hash value they are computing may not work uniformly on other platforms. If they intentionally want \r\n, then warnings are just minor noise which they can simply ignore.

Upon reading - #174 (comment) - it appears me that not having it is cleaner path - no confusion, all clean.

@mgorny I would like to have your opinion on this.

[mgorny]

That's a hard question. FWICS the hash is currently used to verify downloaded sources. IIRC git is capable of converting newlines upon checkout, so a need for newline normalization exists here. However, git generally carries indication as to binary/text files, so perhaps it would be good enough to limit the normalization to files where we know that the conversion happened.

That said, we probably shouldn't be using it for archives since it makes us vulnerable to zipbombs and compressor vulnerabilities.

[jaimergp]

I sympathize with both points of view. "A hash function should only concern itself with bytes" is a very valid point, but I also want to get back to the intent of the original CEP: provide a(n) (extracted) contents-focused hash method for source verification. Independent of compression schemes, tools used or their versions. So it shouldn't matter if one clones the repository or downloads the tarball, same contents = same hash.

Line normalization is mentioned mostly because of git clone behavior across platforms. So maybe the thing to do here is to decouple line ending normalization rules from the hashing algorithm itself, and then mention in Rationale why this may be relevant on certain cases.

And worst case, folks would have to add a different hash on Windows.

However, git generally carries indication as to binary/text files, so perhaps it would be good enough to limit the normalization to files where we know that the conversion happened.

This is a good point, we only need this on git repos, so we can query git (e.g. via git ls-files --eol) to see what we need to do about line endings.

[pb01ka]

After reading your comments, here's my proposal for how to handle the line ending normalization issue in the new hash algorithm.

1. Take line ending normalization out of the algorithm

The hash will cover raw bytes only: no UTF-8 decode attempt, no text/binary check, and no \r\n -> \n rewrite. Each file's contents are hashed exactly as they are on disk (length-prefixed).

This makes the hash a pure function of the directory contents.

2. Explain in the Rationale why line endings are left alone

The key point is that the file's bytes do not contain the answer. Take a Windows checkout with two files:

File	index	working tree	bytes on disk
lf.txt	i/lf	w/crlf	a\r\nb\r\n
crlf_committed.txt	i/crlf	w/crlf	a\r\nb\r\n

Both files have identical bytes on disk, but they need opposite treatment. lf.txt was converted by git on checkout, so it has to be converted back to match the tarball. crlf_committed.txt was committed with CRLF, so its CRLFs are real content and must be kept.

AFAICT, nothing inside the files can tell these two apart. Only the git index can. So the hash should stay a plain hash over bytes, and recovering the original form is a job for whoever has the index, not for the hashing algorithm.

This also shows a problem with the current rule: it normalizes both files and erases a difference that really exists.

3. Recommend how to avoid CRLF mismatches (in the Rationale, not normative)

Both of the following options apply to git checkouts. A downloaded tarball needs neither - extracting it does not rewrite line endings, so it already hashes the same on every platform. This is why the fix belongs at checkout time and not in the hash.

1. Fix it at checkout. Use * text=auto eol=lf in .gitattributes, or set core.autocrlf=input. This needs no tooling and solves the problem for everyone cloning the repository.

2. If you cannot control the checkout, undo the conversion before hashing. Run git ls-files --eol and look at each file's index and working tree line endings. For files reported as i/lf w/crlf, replace \r\n with \n in memory before feeding the bytes to the hasher.

4. Use a separate hash only as a last resort

If two trees really differ in bytes, they are different contents and get different hashes. That is the algorithm doing its job and not a bug.

Where this is unavoidable - for example files marked eol=crlf or in sources that do not come from git, or files with mixed line endings - a recipe can carry a platform specific hash. This is the last fallback and Step 3 should make it less probable.

---

Please let me know your thoughts - if any step needs to be removed, added or modified. TY.

[mgorny]

I suppose "or file contents" belongs here now.

[mgorny]

This is still potentially ambiguous: you're talking of "sorting them" by relative paths, but you're not saying that relative paths are actually used until much later.

Also worth explicitly mentioning here that directories are collected as well (and whether that includes empty directories).

[mgorny]

I'd combine these two points, given the second one is specifically talking about the result of the first one; while the points below are talking of separate steps.

[mgorny]

Actually, why are we doing this? Since the input is from a scanning function, I suppose such entries can only be symlink targets. Why would we want to consider foo/../bar the same as bar? Presumably if you use two different targets, there is a reason for the difference, right?

[mgorny]

Perhaps this point should be moved earlier on, especially that you're talking about UTF-8 encoding in the first two points, and making an explicit "before encoding" statement there.

[mgorny]

It's weird to be using "raw" when it's actually Unicode string. "Raw" makes me think of raw bytes.

[mgorny]

Any reason not to use os.walk() here? I know it's just an example but I don't think you'd ever not want to use os.walk().

[mgorny]

This will never match anything, given how you obtain the input.

[mgorny]

Not a big deal but you could avoid duplicating the data by making entries a dict.

[mgorny]

Well, strictly speaking, some data files may combine platform-specific line endings with raw data that doesn't decode as valid UTF-8 :-).

[pb01ka]

Heads up - I will address your reviews this week. TY.

cc: @jaimergp @hunger @mgorny

[pb01ka]

I have addressed the reviews locally. However, before pushing I would like to get an answer to my question in this comment. @hunger's opinion is already clear. Waiting for other participants. Once it's clear I will push the revised version.

TYSM for your reviews.

@jaimergp @mgorny