Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:97655
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: softfail (pb1.pair.com: domain golemon.com does not designate 209.85.218.42 as permitted sender)
MIME-Version: 1.0
Sender: php@golemon.com
In-Reply-To: <CAESVnVovq2rLu5tLj0GOv_vCd21hDosOhxEF17jG_fPwaNMVNQ@mail.gmail.com>
References: <CAESVnVrsf7bdtuAT5+6vGKNmXLr2K3Mnpr=CCnd+o4z9vKMhwQ@mail.gmail.com>
 <CAESVnVpOtMgk0JvWPxwZwoQFBw31cyUvLpRgpgWsZyV_BVHQzg@mail.gmail.com>
 <CAESVnVoc6j2c7MO-_N=1JEe4Z3d3Q-S+UHZ0m-v3TAy+QJfKOg@mail.gmail.com>
 <CAESVnVo5HeOuCwqu=ntgAeTRujOXR=uyKV9nPMtYZ-3z_3_Sxw@mail.gmail.com>
 <CAESVnVoKF7xQKJMjSAO4p0GcYgJy7h=_n04q-VARLM0c9beAwg@mail.gmail.com>
 <CAESVnVq7q3_-fJtZwTs0FRVeONnUnqcaDw+PPgbFGYg_5tF1iA@mail.gmail.com> <CAESVnVovq2rLu5tLj0GOv_vCd21hDosOhxEF17jG_fPwaNMVNQ@mail.gmail.com>
Date: Tue, 10 Jan 2017 00:23:42 -0800
Message-ID: <CAESVnVpO0CStJC1Gcu4k7VtQiFLPGZ_2=cOfHN1T8cAfAtM0DQ@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Fwd: [RFC] HashContext as Object
From: pollita@php.net (Sara Golemon)

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