Conversation
|
@kelunik This PR fixes a deprecation warning when using amphp in the Symfony testsuite |
| if (4 !== \PHP_INT_SIZE) { | ||
| if ($value < 0) { | ||
| throw new \UnderflowException('Long integer value must be in the range 0 - 4294967296'); | ||
| } else if ($value > 0xffffffff) { | ||
| throw new \OverflowException('Long integer value must be in the range 0 - 4294967296'); | ||
| } | ||
| } |
There was a problem hiding this comment.
If we're on 32 bit, values that should be positive (unsigned) will be negative, no?
There was a problem hiding this comment.
On 32-bit, we won't enter this branch because of the 4 !== \PHP_INT_SIZE check above.
There was a problem hiding this comment.
I know, but values decoding to large values will be negative on 32 bit, no? Shouldn't we reject those?
There was a problem hiding this comment.
Nevermind, I guess it's fine that way.
| class DomainName extends Type | ||
| { | ||
| const FLAG_NO_COMPRESSION = 0x80000000; | ||
| const FLAG_NO_COMPRESSION = 0x8000; |
There was a problem hiding this comment.
Technically, this is a BC break. Should we just fix the usage and leave this as is?
There was a problem hiding this comment.
This line is the culprit, php triggers the deprecation on this very line. It's a too big number on 32bit.
|
@stof Thanks for the reminder, seems like we've missed this! |
|
Closing in favor of #28 |
3 participants
const FLAG_NO_COMPRESSION = 0x80000000;is a float on 32 bit archs.Since PHP 8.1, the following expression causes a deprecation:
Types::DOMAIN_NAME | DomainName::FLAG_NO_COMPRESSIONThis PR fixes the issue by shifting
FLAG_NO_COMPRESSIONby 16 bits.It also skips a comparison in
Long::setValue()that doesn't need to happen on 32b archs, where numbers are always 32b and can be negative even if they're meant to be understood as unsigned longs. Encoding/decoding doesn't care about the sign since they use pack/unpack().