Remove $age parameter of curl_version()

Hi,

curl_version()[1] (of ext/curl) makes curl_version_info()[2] (of
libcurl) available to PHP userland. The latter requires to pass an age
argument which usually is CURLVERSION_NOW, so that the information
returned by the runtime matches the declarations used during compile
time. For C programs it is simply necessary to pass this information,
and it might make sense to pass something else than CURLVERSION_NOW, but
the PHP wrapper assumes that the return value of curl_version_info()
matches the compile time declarations anyway, so passing anything else
might give bad results.

Therefore I suggest to remove this parameter in the long run altogether.
For PHP 7.4 I suggest to deprecate using the parameter, and also to
ignore anything that is not CURLVERSION_NOW, and to raise a warning in
this case. I.e. something like the following behavior:

// okay
curl_version();

// E_DEPRECATED: passing an argument is deprecated
curl_version(CURLVERSION_NOW);

// E_WARNING: argument ignored
curl_version($not_curlversion_now);

Thoughts? Do I overlook something important?

[1] https://www.php.net/manual/en/function.curl-version.php
[2] https://curl.haxx.se/libcurl/c/curl_version_info.html

--
Christoph M. Becker

On Wed, May 1, 2019 at 1:18 PM Christoph M. Becker cmbecker69@gmx.de
wrote:

curl_version()1 (of ext/curl) makes curl_version_info()[2] (of
libcurl) available to PHP userland. The latter requires to pass an age
argument which usually is CURLVERSION_NOW, so that the information
returned by the runtime matches the declarations used during compile
time. For C programs it is simply necessary to pass this information,
and it might make sense to pass something else than CURLVERSION_NOW, but
the PHP wrapper assumes that the return value of curl_version_info()
matches the compile time declarations anyway, so passing anything else
might give bad results.

It looks like the ext/curl binding 1 handles run-time age values that
differ from compile-time, as it makes checks for if (d->age). Am I missing
something?

Therefore I suggest to remove this parameter in the long run altogether.
For PHP 7.4 I suggest to deprecate using the parameter, and also to
ignore anything that is not CURLVERSION_NOW, and to raise a warning in
this case. I.e. something like the following behavior:

// okay
curl_version();

// E_DEPRECATED: passing an argument is deprecated
curl_version(CURLVERSION_NOW);

// E_WARNING: argument ignored
curl_version($not_curlversion_now);

Thoughts? Do I overlook something important?

Well, what about:

if (false === curl_version(3)) {
throw new \Exception('Please rebuild PHP with curl at least
version 7.16.1');
}

Here the developer has used age as a proxy to a major release point of
curl, as opposed to say:

if (version_compare(curl_version()['version'], '7.16.1', '<') { throw new
Exception...; }

On Wed, May 1, 2019 at 7:19 PM Christoph M. Becker cmbecker69@gmx.de
wrote:

Hi,

curl_version()[1] (of ext/curl) makes curl_version_info()[2] (of
libcurl) available to PHP userland. The latter requires to pass an age
argument which usually is CURLVERSION_NOW, so that the information
returned by the runtime matches the declarations used during compile
time. For C programs it is simply necessary to pass this information,
and it might make sense to pass something else than CURLVERSION_NOW, but
the PHP wrapper assumes that the return value of curl_version_info()
matches the compile time declarations anyway, so passing anything else
might give bad results.

Therefore I suggest to remove this parameter in the long run altogether.
For PHP 7.4 I suggest to deprecate using the parameter, and also to
ignore anything that is not CURLVERSION_NOW, and to raise a warning in
this case. I.e. something like the following behavior:

// okay
curl_version();

// E_DEPRECATED: passing an argument is deprecated
curl_version(CURLVERSION_NOW);

// E_WARNING: argument ignored
curl_version($not_curlversion_now);

Thoughts? Do I overlook something important?

Sounds good to me.

Nikita

On Wed, May 1, 2019 at 12:18 PM Christoph M. Becker cmbecker69@gmx.de
wrote:

curl_version()[1] (of ext/curl) makes curl_version_info()[2] (of
libcurl) available to PHP userland. The latter requires to pass an age
argument which usually is CURLVERSION_NOW, so that the information
returned by the runtime matches the declarations used during compile
time. For C programs it is simply necessary to pass this information,
and it might make sense to pass something else than CURLVERSION_NOW, but
the PHP wrapper assumes that the return value of curl_version_info()
matches the compile time declarations anyway, so passing anything else
might give bad results.

Wow... yeah. That's an example of being far too idiomatic with the
bindings. I didn't even know we accepted that arg. Kill it with fire.

-Sara

On Wed, May 1, 2019 at 1:18 PM Christoph M. Becker cmbecker69@gmx.de
wrote:

curl_version()1 (of ext/curl) makes curl_version_info()[2] (of
libcurl) available to PHP userland. The latter requires to pass an age
argument which usually is CURLVERSION_NOW, so that the information
returned by the runtime matches the declarations used during compile
time. For C programs it is simply necessary to pass this information,
and it might make sense to pass something else than CURLVERSION_NOW, but
the PHP wrapper assumes that the return value of curl_version_info()
matches the compile time declarations anyway, so passing anything else
might give bad results.

It looks like the ext/curl binding 1 handles run-time age values that
differ from compile-time, as it makes checks for if (d->age). Am I missing
something?

Indeed, as the binding is written, it's unlikely that bad results are
returned. Still, using a newer "age" than available at compile time
will neither provide the PHP program more information, nor would using
an older "age" have real benefits.

Thoughts? Do I overlook something important?

Well, what about:

if (false === curl_version(3)) {
throw new \Exception('Please rebuild PHP with curl at least
version 7.16.1');
}

It seems to me that wouldn't work anyway, since curl_version() only
returns false if curl_version_info() returned NULL, but that won't
happen[2].

[2]
https://github.com/curl/curl/blob/d1b5cf830bfe169745721b21245d2217d2c2453e/lib/version.c#L390-L463

--
Christoph M. Becker

On Thu, May 2, 2019 at 3:58 AM Christoph M. Becker cmbecker69@gmx.de
wrote:

On Wed, May 1, 2019 at 1:18 PM Christoph M. Becker cmbecker69@gmx.de

Thoughts? Do I overlook something important?

Well, what about:

if (false === curl_version(3)) {
throw new \Exception('Please rebuild PHP with curl at least
version 7.16.1');
}

It seems to me that wouldn't work anyway, since curl_version() only
returns false if curl_version_info() returned NULL, but that won't
happen[2].

[2]
<
https://github.com/curl/curl/blob/d1b5cf830bfe169745721b21245d2217d2c2453e/lib/version.c#L390-L463

Indeed. In that case, I cannot see any value from the $age parameter. It
seems to be only an overly-literal functionality translation. I'm in favor
of removal.

curl_version()[1] (of ext/curl) makes curl_version_info()[2] (of
libcurl) available to PHP userland. The latter requires to pass an age
argument which usually is CURLVERSION_NOW, so that the information
returned by the runtime matches the declarations used during compile
time. For C programs it is simply necessary to pass this information,
and it might make sense to pass something else than CURLVERSION_NOW, but
the PHP wrapper assumes that the return value of curl_version_info()
matches the compile time declarations anyway, so passing anything else
might give bad results.

Therefore I suggest to remove this parameter in the long run altogether.
For PHP 7.4 I suggest to deprecate using the parameter, and also to
ignore anything that is not CURLVERSION_NOW, and to raise a warning in
this case. I.e. something like the following behavior:

// okay
curl_version();

// E_DEPRECATED: passing an argument is deprecated
curl_version(CURLVERSION_NOW);

// E_WARNING: argument ignored
curl_version($not_curlversion_now);

Thoughts? Do I overlook something important?

[1] https://www.php.net/manual/en/function.curl-version.php
[2] https://curl.haxx.se/libcurl/c/curl_version_info.html

I have submitted PR #4106[3] to address this issue. Of course, ignoring
anything else than CURLVERSION_NOW would be a BC break, and is as such
debatable. Also, the deprecation should be followed by an actual
removal sometime in the future, so likely there should be a respective
RFC -- or can we do without in this case?

[3] https://github.com/php/php-src/pull/4106

--
Christoph M. Becker