Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:127777
Precedence: bulk
MIME-Version: 1.0
References: <CA+NMNO0K85acCismdBL=8vr5=nKg1_-i0BjXyiqnjoqGAnH3Xw@mail.gmail.com>
In-Reply-To: <CA+NMNO0K85acCismdBL=8vr5=nKg1_-i0BjXyiqnjoqGAnH3Xw@mail.gmail.com>
Date: Sat, 28 Jun 2025 03:28:00 +0530
Message-ID: <CANLbj-ppn+QtYij2uHXmNvbXckKzz84fkj6f-6iw_7wioFN0dA@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC][DISCUSSION] Object-oriented curl API v2
To: erictnorris@gmail.com
Cc: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
From: ayesh@php.watch (Ayesh Karunaratne)

> 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 =E2=80=94 that a `CurlHandle` object i=
s
passed to functions =E2=80=94 is not as intuitive as calling a method on th=
at
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=E2=80=99t 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=3DCURLOPT_)
 - 78 CURLINFO_* consts (https://php.watch/codex?search=3DCURLINFO_)
 - 71 CURLE_* consts (https://php.watch/codex?search=3DCURLE_) + 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.