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

Fix 8.4 incompatibilities #22

Merged
merged 4 commits into from
Jan 8, 2025
Merged

Fix 8.4 incompatibilities #22

merged 4 commits into from
Jan 8, 2025

Conversation

Firehed
Copy link
Contributor

@Firehed Firehed commented Jan 6, 2025

8.4 starts issuing warnings about implicit nullable parameters, e.g. int $foo = null should be ?int $foo = null. This change corrects the signatures for compatibility.

This also relatedly blocks versions of PHP with a regression in GMP, demonstrated at https://3v4l.org/lnhXp

@sop sop merged commit 74b4267 into sop:master Jan 8, 2025
@sop
Copy link
Owner

sop commented Jan 8, 2025

Thanks for the PR!
While it's true that those PHP versions you added as conflicting seem to cause errors, i think it's fairly safe to allow distributing regardless. So i removed the restrictions from composer.json.

@Firehed
Copy link
Contributor Author

Firehed commented Jan 8, 2025

Glad to help!

FWIW, the following patch avoids that regression:

diff --git a/lib/ASN1/Util/BigInt.php b/lib/ASN1/Util/BigInt.php
index 28e5ccd..7d25660 100644
--- a/lib/ASN1/Util/BigInt.php
+++ b/lib/ASN1/Util/BigInt.php
@@ -183,7 +183,10 @@ class BigInt
             } while ($tmp > 128);
         }
         // compute two's complement 2^n - x
-        $num = gmp_pow('2', 8 * $width) - $num;
+        $base = gmp_init('2');
+        $exponent = 8 * $width;
+        $num = $base ** $exponent - $num;
         $bin = gmp_export($num, 1, GMP_MSW_FIRST | GMP_BIG_ENDIAN);
         // if first bit is 0, prepend full inverted byte to represent negative two's complement
         if (!(ord($bin[0]) & 0x80)) {

I believe this will work all the way back to 7.2 (GMP operator overloads were added back in 5.6), but I didn't see CI results so I left it out for now in case other areas are similarly affected. Firehed/webauthn-php#89 and php/php-src#16870 have more context if you're interested.

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

Successfully merging this pull request may close these issues.

2 participants