Everything

php

[RFC] Under Discussion: Default User-Agent for cURL

php.internals

Michael Maroszek

5 years ago

Dear internals,

I would like to propose an RFC to add a new php.ini option called
curl.user_agent which allows setting a default User-Agent for all cURL
requests. This option is the equivalent to user_agent for stream based
functions (like file_get_contents). The proposal is quite simple and has no
backward incompatible changes.

Please have a look at the full RFC: https://wiki.php.net/rfc/curl_user_agent

You can also find an initial discussion here:
https://www.mail-archive.com/internals@lists.php.net/msg106186.html

There is one open issue which Tyson raised to find the correct mode for the
ini option. Either PHP_INI_ALL or PHP_INI_SYSTEM is currently being
discussed. See here for all details:
https://github.com/php/php-src/pull/6834/files#r628774432

The PR: https://github.com/php/php-src/pull/6834

I'd love to hear more feedback and hope to land this to 8.1, if everything
goes well.

Regards,
Michael

A.L.E.C

5 years ago

On 25.06.2021 16:33, Michael Maroszek wrote:

>
> Please have a look at the full RFC: https://wiki.php.net/rfc/curl_user_agent
>
> There is one open issue which Tyson raised to find the correct mode for the
> ini option. Either PHP_INI_ALL or PHP_INI_SYSTEM is currently being
> discussed. See here for all details:
> https://github.com/php/php-src/pull/6834/files#r628774432

The existing user_agent setting is PHP_INI_ALL. Please, don't make it
different.

Imo, the fact that curl is not using user_agent setting is a bug and
should be fixed, even with BC break. We really don't need another setting.

--
Aleksander Machniak
Kolab Groupware Developer        [https://kolab.org]
Roundcube Webmail Developer  [https://roundcube.net]
----------------------------------------------------
PGP: 19359DC1 # Blog: https://kolabian.wordpress.com

Michael Maroszek

5 years ago

Hi Aleksander,

That's what I also thought when making the PR and therefore I initially
went with PHP_INI_ALL.

But Tyson made a good point that the curl.cainfo is PHP_INI_SYSTEM and we
might want to be consistent about modes inside an extension.

I'd love to hear some more opinions on which direction we should go.

> Imo, the fact that curl is not using user_agent setting is a bug and
> should be fixed, even with BC break. We really don't need another setting.

That was my option B, but in my opinion this feature is too minor and
therefore not worth the BC break.

Regards
Michael


Am Sa., 26. Juni 2021 um 09:20 Uhr schrieb Aleksander Machniak <alec@alec.pl

A.L.E.C

5 years ago

On 27.06.2021 08:48, Michael Maroszek wrote:

> That's what I also thought when making the PR and therefore I initially
> went with PHP_INI_ALL.
>
> But Tyson made a good point that the curl.cainfo is PHP_INI_SYSTEM and we
> might want to be consistent about modes inside an extension.

Another option might be PHP_INI_PERDIR (for both). Why? Because that's
what's used for (similar) openssl extension configuration.

ps. anyway, right now I'm on -1 for the new config option.

--
Aleksander Machniak
Kolab Groupware Developer        [https://kolab.org]
Roundcube Webmail Developer  [https://roundcube.net]
----------------------------------------------------
PGP: 19359DC1 # Blog: https://kolabian.wordpress.com

Michael Maroszek

5 years ago

Hi!

Would anyone else be in favor of reusing the user_agent setting for cURL
despite the BC break?

At the moment all possibilities (user_agent, curl.user_agent (PHP_INI_ALL),
curl.user_agent (PHP_INI_SYSTEM)) seem to have negative votes attached.

I am unsure if it makes sense to go forward with the RFC and if I should
bring the RFC to a YES/NO vote for the feature itself and the three
mentioned possibilities to choose from as a secondary vote option.

I'd love to get some help on how to proceed even if the answer might be:
don't proceed.


Am So., 27. Juni 2021 um 09:25 Uhr schrieb Aleksander Machniak <alec@alec.pl

Michael Maroszek

5 years ago

Dear internals,

I didn't receive any more feedback on this RFC, therefore I would like to
start voting next week on the RFC as is.

Rationale:
- I would like to stick with a distinct curl.user_agent ini option to avoid
the BC break.
- Also using PHP_INI_ALL as the options visibility is matching the core
user_agent options behaviour. That is being able to modify the user_agent
via ini_set during runtime.

Thank you for all the contributions!


Am Di., 29. Juni 2021 um 10:35 Uhr schrieb Michael Maroszek <
paresy@gmail.com>:

Dan Ackroyd

5 years ago

On Tue, 6 Jul 2021 at 15:58, Michael Maroszek <paresy@gmail.com> wrote:

>
> Dear internals,
>
> I didn't receive any more feedback on this RFC, therefore I would like to
> start voting next week on the RFC as is.

fyi, I think you'll need to update the 'Proposed PHP Version' to 8.2.

Ben Ramsey wrote on 22 Jun 2021 - https://news-web.php.net/php.internals/115001

> This means FEATURE FREEZE for PHP 8.1 is on 20 July!

From the RFC 'rules' - https://wiki.php.net/rfc/howto

> Set a deadline for voting; the minimum period is two weeks.

20 - 14 = yesterday was the last day for opening votes targeting 8.1

As there are quite a few RFCs in flight, and this one hasn't been
discussed much, I'd recommend holding off opening the vote until after
the 20th, as that people might have a bit more spare bandwidth to
think about it then.

cheers
Dan
Ack

Michael Maroszek

5 years ago

Hi Dan,

that sounds reasonable. I have also updated the target to 8.2 and will wait
a big more to give everyone extra time.

Am Mi., 7. Juli 2021 um 18:13 Uhr schrieb Dan Ackroyd <
Danack@basereality.com>:

Hans Henrik Bergan

5 years ago

fwiw i think no self-respecting codebase is depending on an ini-setting
being correct for the ua in cases where the ua is actually important, so
the breakage should be minimal.

On Sun, 27 Jun 2021 at 09:25, Aleksander Machniak <alec@alec.pl> wrote:

Sara Golemon

5 years ago

On Tue, Aug 3, 2021 at 4:45 AM Hans Henrik Bergan <divinity76@gmail.com>
wrote:

> fwiw i think no self-respecting codebase is depending on an ini-setting
> being correct for the ua in cases where the ua is actually important, so
> the breakage should be minimal.
>
>

In my mind, this is either an argument in favor of either NOT adding a new
INI setting (rather applying the existing user_agent setting to curl), or
doing nothing at all.  Code using curl already knows how to call the
precisely ONE line required to set a user agent, or more likely uses a
wrapper which simplifies cURL's archaic interface, in which case it's that
proxy which should be setting any default UA, not PHP.

And on the subject of PHP_INI_ALL/PERDIR/SYSTEM.  The answer is ALL.
PERDIR and SYSTEM exist either for technical limits (something gets
allocated and has to live across requests), or security limiters.  The
former certainly doesn't apply, and the latter wouldn't achieve anything
since the UA can always be set explicitly on any curl handle.   PHP_INI_ALL
is the only setting which makes sense if we add an additional configuration
(which I don't think we should).

-Sara

Hans Henrik Bergan

5 years ago

>
> or doing nothing at all.
>

Sounds good to me.

On Tue, 3 Aug 2021 at 16:47, Sara Golemon <pollita@php.net> wrote: