Hi
On 2026-11-03 12:37, Sjoerd Langkemper wrote:
I changed the RFC to reflect this. openssl_get_channel_binding can no
longer return null, but it can throw a (newly introduced)
UnsupportedChannelBindingException.
Thank you. I've given the RFC another read and have some more comments:
1. The naming of the `$channel_binding_type` parameter. It seems there
is no current policy and using underscores is consistent with
ext/openssl overall, but from what I see, new parameters are typically
using camelCase. No strong opinion either way from my side, but
potentially worth a discussion.
2. “Introducing a new Error hierarchy for ext/openssl is a larger API
decision that would affect more than this function, and none of the
existing ext/openssl APIs currently use an Error subclass”
Not really. The “look and feel” of `Openssl\OpensslError` is already
required by policy, so there's no API design to be resolved here. I
don't disagree with the choice of ValueError (and even suggested it as
one of two alternatives), but that argument in the RFC stood out and
should probably be rephrased or dropped.
3. Overall and tying a little into (2), I feel that the RFC
“overjustifies” some of its choices. This is also visible in the “why an
enum” paragraph where the RFC goes great lengths to explain why a union
type is not chosen and how there is prior art - when an enum is just
obvious and consistent with what we recently did. To me this made the
RFC a bit tedious to read and harder to extract the parts where an
actual design decision happened. Similarly the “Mapping a configured
name to the enum” example just shows how enums work (which can be
assumed to be general knowledge at this point). Not a blocker of course,
but potentially something to keep in mind for future RFCs.
Best regards
Tim Düsterhus
