Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unpack() supports f{4,8}{be,le} out of the box in PHP "~7.0.15 || >=7.1.1" #7

Open
generalmimon opened this issue Feb 11, 2022 · 0 comments

Comments

@generalmimon
Copy link
Member

generalmimon commented Feb 11, 2022

Currently the BE/LE floats are unpacked manually from an integer, see the code:

public function readF4be(): float {
$bits = $this->readU4be();
return $this->decodeSinglePrecisionFloat($bits);
}
/**
* Double precision floating-point number.
*/
public function readF8be(): float {
$bits = $this->readU8be();
return $this->decodeDoublePrecisionFloat($bits);
}

This no longer makes sense in PHP "~7.0.15 || >=7.1.1", because in these versions, unpack() can read them natively - see pack() docs:

--- https://web.archive.org/web/20170616103946/http://php.net/manual/en/function.pack.php
+++ https://web.archive.org/web/20170617094522/http://php.net/manual/en/function.pack.php
 
 Currently implemented formats are:
 
@@ -25,15 +25,20 @@ Q | unsigned long long (always 64 bit, machine byte order)
 J | unsigned long long (always 64 bit, big endian byte order)
 P | unsigned long long (always 64 bit, little endian byte order)
 f | float (machine dependent size and representation)
+g | float (machine dependent size, little endian byte order)
+G | float (machine dependent size, big endian byte order)
 d | double (machine dependent size and representation)
+e | double (machine dependent size, little endian byte order)
+E | double (machine dependent size, big endian byte order)
 x | NUL byte
 X | Back up one byte
 Z | NUL-padded string (new in PHP 5.5)
 @ | NUL-fill to absolute position
 
-### Changelog [¶](https://web.archive.org/web/20170616103946/http://php.net/manual/en/function.pack.php#refsect1-function.pack-changelog)
+### Changelog [¶](https://web.archive.org/web/20170617094522/http://php.net/manual/en/function.pack.php#refsect1-function.pack-changelog)
 
 Version | Description
 -- | --
+7.0.15,7.1.1 | The "e", "E", "g" and "G" codes were added to enable byte order support for float and double.
 5.6.3 | The "q", "Q", "J" and "P" codes were added to enable working with 64-bit numbers.
 5.5.0 | The "Z" code was added with equivalent functionality to "a" for Perl compatibility.

This agrees with my tests - see https://3v4l.org/nGQjc#veol (no idea how long this link will last, but here's my Gist with the code, so you can paste it to https://3v4l.org/ again). Summary of results - unpack("{e,E,g,G}", $bytes):

  • works in PHP 7.0.15 - 7.0.33, 7.1.1 - 7.1.33, 7.2.0 - 7.2.34, 7.3.0 - 7.3.33, 7.4.0 - 7.4.27, 8.0.0 - 8.0.15, 8.1.0 - 8.1.2

  • doesn't work in 7.0.0 - 7.0.14, 7.1.0; these versions raise a Warning: unpack(): Invalid format type g

    (older than 7.0.0 don't work too, but this is due to the use of type hints).

discrepancy in the latest php.net documentation

There is some discrepancy in the documentation, though - the latest unpack() page claims that BE and LE float/double types are only supported since 7.2.0 (contrary to my tests above):

Changelog

Version Description
7.2.0 float and double types supports both Big Endian and Little Endian.
7.1.0 The optional offset has been added.

Apparently, the same nonsense has been added to the documentation of pack() too (here the absurdity is evident, because the rows 7.2.0 and 7.0.15,7.1.1 are right next to each other and they have the same meaning):

Changelog

Version Description
8.0.0 This function no longer returns false on failure.
7.2.0 float and double types supports both Big Endian and Little Endian.
7.0.15,7.1.1 The "e", "E", "g" and "G" codes were added to enable byte order support for float and double.

This isn't right - unpack("{e,E,g,G}", $data) apparently works normally in PHP 7.0.15 and 7.1.1.


I'm sure it's better to decode f{4,8}{be,le} natively using unpack() than with our own decode{Single,Double}PrecisionFloat() methods. I don't think our decoding methods are too reliable (they are only tested on a few values like 0.5, 0.25 and -pi; no coverage of 0.0, -0.0, NaN, +inf, -inf, etc.) and there's no way they can be fast while using this for loop. But most importantly, we lose code that we have to take care of, which is good.

By switching to unpack() in readF{4,8}{Be,Le}() implementations, support for PHP 7.0.0 - 7.0.14 and 7.1.0 would be lost, but I think it's totally worth the benefits. Whoever still relies on EOL versions 7.0 and 7.1 in production should be at least using the latest patch versions 7.0.33 and 7.1.33 (they're always backward compatible with older patches, so no code migration should be needed), otherwise they're exposed to a security risk, since patch versions usually fix security vulnerabilities.

Along with dropping the support, this composer.json line should be changed:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant