Fwd: [RFC] HashContext as Object

On Thu, Dec 29, 2016 at 6:32 PM, Sara Golemon <pollita@php.net> wrote: > On Mon, Dec 26, 2016 at 7:35 PM, Sara Golemon wrote: >> 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