`Hash` portability between compiler versions is potentially misleading

[hoxxep]

Location

https://doc.rust-lang.org/std/hash/trait.Hash.html#portability

Summary

Whether Hash and Hasher is portable between compiler versions is ambiguously documented, and potentially misleading.

Serialization formats intended to be portable between platforms or compiler versions should either avoid encoding hashes or only rely on Hash and Hasher implementations that provide additional guarantees.

Changes to the default behaviour of Hasher are frequently[1] discussed[2] without considering portability between rust versions. I am not aware of any breaking changes on stable rust yet, but implying that Hasher output can be stable between compiler versions seems misleading.

A rust-users discussion over whether rapidhash can offer a Hasher-compatible persistent hasher has generated conflicting opinions.

I think it would be restrictive to future work on the Hash trait to claim that Hash and Hasher are portable between compiler versions, and suggest that non-portability between compiler versions is made explicit in both the Hash and Hasher documentation.

Very happy to be convinced otherwise, as it would let me build a persistent RapidHasher if portability is indeed guaranteed. Thanks!

[hanna-kruppe]

cc @the8472 (introduced this disclaimer in #90995)

Note that the disclaimer talks about both Hash and Hasher impls. A Hasher alone can’t guarantee stable results, the types being hashed also have to promise that their interaction with the Hasher are stable.

Together with the lack of guarantees by all(?) standard library types and the possibility of Hasher’s default impls changing, this boils down to: you only get stable results if you only hash types that only calls methods explicitly overriden by the Hasher. This is very difficult to ensure in practice because you basically have to write all your Hash impls by hand and remember to never call into other Hash impls that don’t promise stability (which includes all primitive types and everything else in std).

But if someone manages to do that, the addition of new Hasher methods and changes in their default implementations aren’t breaking, because the new methods / default impls won’t be magically called.

[hoxxep]

@hanna-kruppe I fully agree with what you've said.

The current docs don't make it clear that users have to:

• Explicitly not use derive(Hash) and implement Hash themselves.
• Take care to avoid Hash::hash (even on primitives/str) and only use Hasher::write_* methods.
• Avoid Hasher::write_* methods with default implementations (particularly the upcoming write_str), which requires reading the Hasher implementation source code to check.
• Iterate manually over any tuples or collections.
• Be informed on how to construct a hash to avoid length-extension attacks etc, if required for their use case.
• Ensure their chosen Hasher is portable, aware of the above, and promises to be stable between rust and crate versions.

At which point I don't believe the std library traits can be considered "safe" for this purpose. It seems fraught with footguns (or at the very least confusion) to hint that the standard library traits are suitable for portable hashing.

I suspect these traits are currently used for portable hashing by many people unaware of the caveats, I certainly was when writing rapidhash. There are many other hashers that describe themselves as portable without pointing out the above caveats, and I'm worried that the first breaking change to the default rust Hasher behaviour will cause some surprises for end users.

My preferences in order are:

1. The docs should suggest Hash and Hasher traits are not portable between compiler versions, including on primitive types, and advanced users can then avoid that advice if they choose.
2. The portability caveats should be better documented on both the Hash and Hasher traits, for end users and hash authors.

Tangentially, I'm tempted to write a crate with PortableHash and PortableHasher traits that rapidhash can implement, which seems like a much safer alternative for users. A poorly documented one that's missing a few things already exists: https://github.com/graphprotocol/stable-hash

[the8472]

Can you explain how the current wording is ambiguous or misleading?

[hoxxep]

I believe the final sentence in the Hash docs implies that Hash and Hasher can be suitable for portable hashing, which is misleading.

Serialization formats intended to be portable between platforms or compiler versions should either avoid encoding hashes or only rely on Hash and Hasher implementations that provide additional guarantees.

There is no documentation of what the guarantees are. I can't find a good list in any of the portable hashing discussions I've read, and to get it right requires an intricate understanding of the Hasher trait and its development (eg. including length prefixes for collections or users can only use write_* methods where the Hasher has manually implemented them).

My suggestion would be to update the Hash docs with the below, and also include the same section in the Hasher docs:

Portability

Due to differences in endianness and type sizes, data fed by Hash to a Hasher should not be considered portable across platforms. Additionally the default behaviour and data passed by most standard library types should not be considered stable between compiler versions.

This means tests shouldn’t probe hard-coded hash values or data fed to a Hasher and instead should check consistency with Eq.

Serialization formats intended to be portable between platforms or compiler versions should avoid relying on the Hash and Hasher traits.

[the8472]

or only rely on Hash and Hasher implementations that provide additional guarantees.

There is no documentation of what the guarantees are.

That talks about additional guarantees that some particular implementations (including 3rd-party ones) might make. Such extra guarantees would be documented on those particular implementations. std can't list them, you'd find them in crates that have custom impls, if they exist.

The guarantees for the standard library types already are documented, and they are practically none:

Due to differences in endianness and type sizes, data fed by Hash to a Hasher should not be considered portable across platforms. Additionally the data passed by most standard library types should not be considered stable between compiler versions.

[hoxxep]

Many rust hashers describe themselves as portable. When writing rapidhash, I also naively thought a well written Hasher implementation was mostly sufficient.

Rapidhash, wyhash, seahash, highwayhash, rustc-stable-hash, and many others promise portability without documenting how to use Hash portably. If the default Hasher::write_str behaviour changes there will be many users that are caught out. Day 1 of documenting RapidHasher as no longer portable and I had a user raise a GitHub issue doing tuple.hash(...) and expecting a portable result.

Such extra guarantees would be documented on those particular implementations.

Can you show me somewhere (a crate, discussion thread, blog post) that outlines the full list of gotchas for portably using Hash::hash? I can't find one?

I believe the documentation would be safer and clearer if it simply said "we don't recommend using these traits for portable hashing". Advanced users can still proceed at their own risk.

[the8472]

Rapidhash, wyhash, seahash, highwayhash, rustc-stable-hash, and many others promise portability

portability is not the same as stability. portability only means that compiling the same versions for two platforms and feeding it the same input would produce the same output. Of course you still need to feed them the same input, but that's the impl Hash's responsibility, not the Hasher's

The documentation says

only rely on Hash and Hasher implementations that provide additional guarantees.

If only one were sufficient then the word "or" would have been used here, but that's not the case.

I'll also note that this sentence only comes after the explicit statement that data passed by std is not stable. Followed by the recommendation not to encode hashes. Only then it mentions the hard way of implementing both Hash and Hasher with extra guarantees.
It may be a case of two feldpars, but it seems obvious to me that if we lead with the standard library types (which includes primitives) not being stable or portable then it should follow that this infects a lot of things using those impls.

So please explain to me how people are actually reading this text and then come to the conclusion that the default expectation would be stable and portable hashes. What's the thought process here?

Can you show me somewhere (a crate, discussion thread, blog post) that outlines the full list of gotchas for portably using Hash::hash?

I'm not aware of any such discussion, in #74215 we mostly discussed the various ways in which the default impls provided by std are unstable, which motivated the current wording.
But the basic approach matches roughly what you mention in #144540 (comment) though I'd say that avoiding Hash::hash isn't necessary in cases where you only apply it to type-trees where all used types have a custom Hash impl.