fix: allow string as parameter to CURLRequest version

[tangix]

Description
The docs (as of 4.5.3) specifies "To set the HTTP protocol to use, you can pass a string or float with the version number". However, the code in the library does a strict comparison with === 2.0 so a string value will not work.

Adding a strict comparison with '1.0', '1.1' and '2.0' since this seems to be the coding style used in the library. (A == comparison would work, but casting could introduce some issues?)

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[ddevsr]

@tangix Adding test on https://github.com/codeigniter4/CodeIgniter4/blob/develop/tests/system/HTTP/CURLRequestTest.php#L1147

[tangix]

@tangix Adding test on https://github.com/codeigniter4/CodeIgniter4/blob/develop/tests/system/HTTP/CURLRequestTest.php#L1147

Will do.

[michalsn]

I would simplify this a bit.

[michalsn]

Can't we just cast it to string or float and do only one check?

[tangix]

Glad for the question - that was my first thought, but checking the rest of the code in the Library having all strict comparisons made me reluctant. Would (string)$config['version'] === '1.0' have any side-effects? is float-to-string cast always locale-independant?

[michalsn]

I don't believe it will cause any problems, we use this config value to set this one option and nothing else.
Casting to string should not have any side effects in this case.

[tangix]

But.... seems like sprintf() should be used - https://www.php.net/manual/en/function.strval.php

"This function performs no formatting on the returned value. If you are looking for a way to format a numeric value as a string, please see sprintf() or number_format()."

Edge-cases.... brrrr.....

[tangix]

I was quite surprised by the original code actually. Is float 1.0 always exactly 1.0?

[michalsn]

Yes, you're right. I guess we should use something like:
$version = sprintf('%.1f', $config['version']) at the beginning. That should be safe enough.

I was quite surprised by the original code actually. Is float 1.0 always exactly 1.0?

Yes, it's safe in our case. The problem may occur when we start doing operations on float value.

[tangix]

Pushed a new commit with that conversion

[tangix]

Seems to be ok...

<?php

$config['version'] = '1.0';
$version = sprintf('%.1f', $config['version']);
var_dump($version);

$config['version'] = 1.0;
$version = sprintf('%.1f', $config['version']);
var_dump($version);


/* returns:

/opt/project/cast.php:5:
string(3) "1.0"
/opt/project/cast.php:9:
string(3) "1.0"

*/

[kenjis]

As far as I know, (string) is locale-independent. But var_dump((string) 0.00001); returns "1.0E-5".

[michalsn]

Looks good. We just need a changelog update in the docs.

[tangix]

Looks good. We just need a changelog update in the docs.

In what file should I do that or do you do it?

[michalsn]

It is preferable to include this in the PR: https://github.com/codeigniter4/CodeIgniter4/blob/develop/user_guide_src/source/changelogs/v4.5.4.rst
Please add an entry in the bug fixes section.

[kenjis]

LGTM!

I will reconsider.

[kenjis]

The format is wrong. f is locale aware. See https://www.php.net/manual/en/function.sprintf.php#refsect1-function.sprintf-parameters

Suggested change

	$version = sprintf('%.1f', $config['version']);
	$version = sprintf('%.1F, $config['version']);

[tangix]

You are correct! Fixed and pushed a new update.

[kenjis]

LGTM!