I was trawling through Pull Requests and found #660 which I think is a
nice idea and deserves some attention. It involves minor BC however,
so I've updated the patch and presented it for your discussing
pleasure.
Specifically, I should say I'm eager to hear thoughts on the item
listed as an Open Issue.
That is: Currently hash_final()
destroys the existing HashContext
resource immediately. No hope of continuing or even just reissuing
the result.
I'm quite tempted atm, to go the "copy the context, finalize to get a
digest, then restoring to the saved context state". For most use
cases of this API this is a slight overhead add since the context is
being duplicated then shortly thereafter discarded. (Bear in mind
though, that the hash_init->hash_update->hash_final API is the edge
case already compared to the all-in-one hash()
and hash_hmac()
functions).
What doing this does offer, however, is a pathway to reasonable APIs
for streaming hash functions (see SHA3/Keccak Sponge Functions). As a
minor side benefit, we also get an idempotent hash_final()
. So even
though there's a VERY minor BC break involved, I think it's the right
thing to do. If others agree, I'll update the diff to reflect.
Still looking for comments/feedback on Open Issue (above). I've
implemented what it would look like as a stacked commit on the feature
branch: https://github.com/php/php-src/compare/master...sgolemon:pr-660
though I could easily squash before commit if it's voted in.
The downside of this is pretty obvious, a completely unnecessary
duplication in the majority of the use cases. But the upside is that
HashContext actually behaves how you'd expect a normal object to
behave. The idea of freezing an object on calling hash_final()
(or
$hash->final(), eventually) makes me twitch a bit.
If I get no feedback at all, I'll just make it a secondary vote: Do
it? "Yes", "No". And if Yes, then: "Keeping freeze
behavior", or
"Include context-preservation."
FWIW, I think I'm actually favoring the idea of keeping the freeze
behavior of just the original implementation, and not including this
latest update.
-Sara