> Hello Internals,
>
> I'd like to formally propose a restart of the original object-oriented
> curl API RFC (https://wiki.php.net/rfc/curl-oop):
>
> https://wiki.php.net/rfc/curl_oop_v2
>
> The prior RFC seemed to get positive feedback, with a small consensus
> around wanting enum(s) for curl options. I've taken that feedback and
> incorporated it into a new RFC, as I am not the original author, but I
> am interested in making another potential improvement to the curl
> extension.
>
> In a nutshell, this version of the RFC:
>
> - uses enumerations for curl options and other curl constants
> - introduces a new Curl namespace
>
> I have not yet created an implementation PR. I realize that is
> somewhat discouraged, but I believe that this should be relatively
> straightforward to implement (there's also the previous RFC's PR to
> build on top of). The implementation of this RFC as it is now will
> likely be tedious, however, so I'd like to get feedback on the
> enumeration idea before committing to the work.
>
> I've outlined one open question in the RFC, which relates to the above:
>
> - Should we organize the curl option enumerations by value type? Or
> have a single enumeration for all curl_setopt options and another for
> all curl_multi_setopt options?
>
> If others (including the original RFC author) are interested in
> working with me on this, I'm more than open to that, so please let me
> know.
>
> Thanks,
> Eric
Thank you for taking the initiative to work on this. It's long
overdue, and the pattern we have now — that a CurlHandle object is
passed to functions — is not as intuitive as calling a method on that
object.
However, I softly oppose this RFC in its current state and the way it
seems to be going.
I have pushed Curl and libcurl to some uncommon cases such as HTTP/3,
DoH, the new debug callback (which I authored the PR for), IMAP, and a
few other obscure tweaks. For all of them, the current curl_setopt
worked, and the more I used it, the more I understood that having just
a few Curl functions and a sea of Curl options is the least
"presumptive way", regardless of the protocol and the options Curl
provides.
The extension is named Curl, because it's supposed to provide Curl
functionality into PHP. It provides low-level functionality, but we
should leave it to the PHP users to build the libraries that have
fluent APIs.
This way, we can have excellent purpose-built HTTP clients, IMAP
clients, Tor proxies, etc, all built using the low-level functionality
the extension offers.The HTTP client authors know the Curl options
really well to build a secure and fast HTTP client, and someone else
building an IMAP client can provide an intuitive API but still use the
same underlying Curl functionality. I understand that your RFC does
not propose a high-level API, and I agree with you that a high-level
API will need a lot of discussion.
There's a ton of Curl constants, and you are right that it could
really use some organizing. The upstream Curl project has not done it,
most likely because the options behave differently depending on the
protocol, and simply because it's a lot of work and BC trouble to do
so. I don’t think we can realistically have a meaningful discourse on
how to semantically group the constants into meaningful Enums. I'd
also argue that if libcurl is OK with having a sea of Curl options, we
should not be the one to group them.
Grouping them by the expected value type is one way to do it, but as
mentioned elsewhere in replies, now, the consumers of the API will
have to figure out whether that one Curl option they always used
belongs to IntOpt or a BoolOpt. It will help with
CURLOPT_SSL_VERIFYHOST (which should be set to 2, not true),
but
I don't think this level of type safety is all that useful for the
trouble.
To bring some numbers, we currently have:
- 271 CURLOPT_* consts (https://php.watch/codex?search=CURLOPT_)
- 78 CURLINFO_* consts (https://php.watch/codex?search=CURLINFO_)
- 71 CURLE_* consts (https://php.watch/codex?search=CURLE_) + I sent
a PR to add 41 missing consts
(https://github.com/php/php-src/pull/13340/files)
- 150+ constants for options such as protocols, HTTP versions, URL
follow rules, etc.
I think a more light-weight approach would be to:
- Move all of them to the \Curl namespace.
- Rename Curl options to \Curl\Option namespace, and rename them,
so that CURLOPT_SSL_VERIFYHOST becomes Curl\Option\SSL_VERIFYHOST
- Rename Curl error codes to similar \Curl\Error constants.
- Have the CurlHandle object accept options, e.g.
$ch->setOption(Curl\Option\SSL_VERIFYHOST, 2). libcurl Easy handlers
do not have a way to retrieve the option once it's set, so there will
be no getOption either.
- Make Curl throw exceptions, and never false on \Curl\execute(),
with the Exception's error code and message mapped to the Curl error
code and message. We will not need to bring over curl_error or
curl_errno functions.
Realistically, I don't think we can deprecate and remove the \curl_*
functions any time soon, so this will actually add more maintenance
work for php-src at the end too.
Thank you.
Ayesh.