[RFC] OOP API for cURL extension

I love it!

When is CurlMultiException and CurlShareException thrown? I feel like
this part in general is not very clear in the RFC.

Good afternoon folks, I'd like to open discussion on adding OOP APIs
to the cURL extension.
https://wiki.php.net/rfc/curl-oop

Thanks for making this. It's certainly an improvement, but it's
disappointing to see such a conservative change when the extension
interface is so weird and awkward. I'm sure you know what I mean.

It's easy to forget how bad it is until you have to do something
without a framework, or explain it to a new developer.

I guess the proper fix would be to add a new extension with new names
for everything, with Request and Response objects, along the lines of
PSR-18. But if I can think of a few minor backwards-compatible changes
which would improve things a bit. For example, getInfo() could be
split out to separate read-only properties. And setOpt() could be
split out to separate chainable mutator methods. That would improve
type inference in surrounding code.

I see CurlHandle::exec() does not have the same return value as
curl_exec() in your proposal, since errors are promoted to exceptions.
But it still has a union return type. I would take it one step
further, and have it unconditionally return either a string or void.

Having it return a string is defensible if you consider it to be
returning the final value of the body buffer. Enable
CURLOPT_RETURNTRANSFER by default. If CURLOPT_WRITEFUNCTION or
CURLOPT_FILE is set and thus the buffer doesn't get appended to,
naturally curl_exec() will return an empty string.

-- Tim Starling

Am 15-Feb-2024 03:30:44 +0100 schrieb pollita@php.net:

Good afternoon folks, I'd like to open discussion on adding OOP APIs to the cURL extension.
https://wiki.php.net/rfc/curl-oop

This has been a long standing bug-bear of mine, and I think its time has come.

try {
  (new \CurlHandle)->setOpt(YOUR_VOTE, true)->exec();
} catch (\CurlHandleException $ex) {
  assert(false); // Why not?!
}

-Sara

Hi Sara,

I like your proposal generally. But I'd suggest to rethink the names of the methods. I understand that you want to reuse the same names of the procedural version and the underlying lib. But this is the chance to optimize the naming and reduce inconsistence. In the future this will become harder.

For example there is the getter "getInfo" and some getters without the "get" prefix, like "errno", "error" and "strerror". I'd suggest to unify them. Either use the "get" prefix for all of those or for none. Personally, I have no particular preference for one of the two options.

The second thing I propose to change is the usage of abbreviations. I'd suggest the following mapping for methods of CurlHandle:

• errno => errorNumber or getErrorNumber
• error => errorMessage or getErrorMessage
• strerror => errorMessageByNumber or getErrorMessageByNumber
• exec => execute
• setOpt => setOption
• setOptArray => setOptionArray

There may be better proposals from others.

What do you think?

Best regards,
Christian

Good afternoon folks, I'd like to open discussion on adding OOP APIs to
the cURL extension.
https://wiki.php.net/rfc/curl-oop

This has been a long standing bug-bear of mine, and I think its time has
come.

try {
(new \CurlHandle)->setOpt(YOUR_VOTE, true)->exec();
} catch (\CurlHandleException $ex) {
assert(false); // Why not?!
}

-Sara

Good morning from this side of the globe Sara! :)

"Hell... It's about damn time" (c) Tychus, StarCraft 2.

The proposed API looks good to me and covers it all as far as I can tell.

--

Arvīds Godjuks
+371 26 851 664
arvids.godjuks@gmail.com
Telegram: @psihius https://t.me/psihius

Working with remote servers is a little bit harder than just catching the
exception.

Just implement OOP stuff gives no benefit except "do not read the docs, but
use IDE". Usually count of curl options is so big that it wont help.

And also, there's multicurl too.
There's batch calling with limit too.
There's errors of different types. I mean curl errors on create, curl
errors on execute, http errors (network), response errors (invalid data,
misformatted data), and api errors, that could be sent even with 200 code.

How try/catch will solve? Easy solution, too many discussion points.

Good afternoon folks, I'd like to open discussion on adding OOP APIs to
the cURL extension.
https://wiki.php.net/rfc/curl-oop

This has been a long standing bug-bear of mine, and I think its time has
come.

try {
(new \CurlHandle)->setOpt(YOUR_VOTE, true)->exec();
} catch (\CurlHandleException $ex) {
assert(false); // Why not?!
}

-Sara

Although I do not have voting karma, that'd be a +1 from me!

--
Atenciosamente,

Flávio Heleno

Sara Golemon pollita@php.net hat am 14.02.2024 19:47 CET geschrieben:

Good afternoon folks, I'd like to open discussion on adding OOP APIs to the cURL extension.
https://wiki.php.net/rfc/curl-oop

This has been a long standing bug-bear of mine, and I think its time has come.

try {
(new \CurlHandle)->setOpt(YOUR_VOTE, true)->exec();
} catch (\CurlHandleException $ex) {
assert(false); // Why not?!
}

-Sara

Thanks a lot for the rfc.

Regarding secure default options, I'd like to mention this article:
https://php.watch/articles/php-curl-security-hardening

Since CurlHandle::setOpt() no longer returns true on success, it would be good to have an example in the RFC how error handling should be implemented in user land regarding setting invalid, deprecated or unknown options (e.g. SSLVERSION received some changes over the years: https://curl.se/libcurl/c/CURLOPT_SSLVERSION.html).

Also I'd prefer to have class constants for the options, e.g. CURLOPT::TIMEOUT instead of CURLOPT_TIMEOUT.

Static code analysis for curl_getinfo() is currently difficult to implement (see https://github.com/phpstan/phpstan-src/blob/1.11.x/src/Type/Php/CurlGetinfoFunctionDynamicReturnTypeExtension.php). So I'd suggest adding sth. like curl_getinfo_all():CurlInfo-Class if possible.

Maybe Ondrej also has some suggestions regarding a more object oriented version or curl_setopt(CurlOption-Class)? (see https://github.com/phpstan/phpstan-src/blob/1.11.x/src/Reflection/ParametersAcceptorSelector.php#L562)

Regards
Thomas

Good afternoon folks, I'd like to open discussion on adding OOP APIs to the
cURL extension.
https://wiki.php.net/rfc/curl-oop

This has been a long standing bug-bear of mine, and I think its time has
come.

try {
(new \CurlHandle)->setOpt(YOUR_VOTE, true)->exec();
} catch (\CurlHandleException $ex) {
assert(false); // Why not?!
}

-Sara

I love the idea of an OOP API. Personally I use Python's Requests
library a lot, it could offer some inspiration. I would really like to
simply write:

 Curl::get('https://...', params: ['key' => 'value'])->json()
 Curl::post(...)
 ...

Some more specific exceptions may be nice too, to easily differentiate
between errors you might want to retry (network errors) and programming
errors.

Regards,
Dik

Summarizing replies so far. Won't be able to update the RFC immediately as
my day job needs me, but some great discussions already, gang. Thanks!

• Define the conditions under which exceptions will be thrown (and which
  exceptions) - I'll add these to the RFC, but in short:

  • CurlException - Never, it's an interface type to group the other
    exceptions.
  • CurlHandleException - Whenever a CurlHandle::method() fails (in lieu of
    returning false)
  • CurlMultiException - Same, but for the CurlMultiHandle class.
  • CurlShareException - Same, but for the CurlShareHandle class.

• Naming styles, e.g getErrorNumber(): int vs errno()
  Comment: Agreed with your points. We don't have to stick to a hard line
  mapping of the function names. My initial translation did because my
  bugbear is with the lack of fluency and all the rest is just window
  dressing on that.
  Proposal: I'll rename all the new APIs according to a get*/set* scheme
  without abbreviations, e.g.:

  • CurlHandle::getErrorNumber(): int
  • CurlHandle::getError(): ?string
  • static CurlHandle::getErrorTextFromCode(int $code): ?string
  • CurlHandle::execute(): ?string // See late note about exec return
    values
  • CurlHandle::setOptionInt(int $option, int $value): CurlHandle

• Better typing for setOpt() methods.
  Comment: Yep, this is a good and fair point. It's going to take a little
  more dicing up of the current implementation, but hopefully not too rough.
  Proposal: Keep untyped setOption() method (1/ Easier migration of
  existing code, 2/ Some options may prove difficult to type), but add:

  • CurlHandle::setOptionBool(int $option, bool $value): CurlHandle
  • CurlHandle::setOptionInt(int $option, int $value): CurlHandle
  • CurlHandle::setOptionString(int $option, string $value): CurlHandle
  • etc... as needed, will this get weird when we get to Array since there
    IS a setOptArray? Maybe we just don't mirror that one, or we call it
    setOptionMany(). Yeah, I like Many for the multiple option set, and Array
    for the single option of array type.
    Internally, the setopt handler will be split by type so that each typed
    setting can call explicitly, and the untyped one can call all.

• CurlHandle::exec() mixed typing of return values.
  Comment: Agreed. The true return value becomes meaningless in the
  RETURNTRANSFER==false case.
  Proposal: Update the RFC for CurlHandle::execute() to return ?string.

• https://php.watch/articles/php-curl-security-hardening
  Comment: I'll read this later when I'm updating the RFC.

• Prefer class constants to global constants.
  Comment: I'm less compelled by this. The global scope is polluted with
  the constants whether we add copies or not. Adding a second way to spell
  the constant name only adds a second way to spell the constant name.
  Proposal: Change my mind?

• Request and Response objects, along the lines of PSR-18
  Comment: I'd be in favor of that, but it's not the mountain I'm
  personally trying to climb today. No offense taken if you vote no because
  this isn't enough, but I don't have the cycles to go in that hard.
  Proposal: Write an RFC (and get it passed) and I can probably help you
  implement it?

-Sara

Hi Sara,
i like this proposal.

Silvio

From: Sara Golemon pollita@php.net
Sent: Wednesday, February 14, 2024 7:47 PM
To: PHP internals internals@lists.php.net
Subject: [RFC] OOP API for cURL extension

Good afternoon folks, I'd like to open discussion on adding OOP APIs to the cURL extension.
https://wiki.php.net/rfc/curl-oop

This has been a long standing bug-bear of mine, and I think its time has come.

try {
(new \CurlHandle)->setOpt(YOUR_VOTE, true)->exec();
} catch (\CurlHandleException $ex) {
assert(false); // Why not?!
}

-Sara