Skip to content

Add test about invalid comparison with GMP objects #6553

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Add test about invalid comparison with GMP objects #6553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
347ad41
Add test about invalid comparison with GMP objects
Girgias Dec 29, 2020
a508349
Check for exception
Girgias Jan 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ext/gmp/gmp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,9 +429,13 @@ static int gmp_compare(zval *op1, zval *op2) /* {{{ */
zval result;

gmp_cmp(&result, op1, op2);
if (Z_TYPE(result) == IS_FALSE) {

/* An error/exception occurs if one of the operands is not a numeric string
* or an object which is different from GMP */
if (EG(exception)) {
return 1;
}
/* result can only be a zend_long if gmp_cmp hasn't thrown an Error */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this for an EG(exception) check then? Z_LVAL(result) may not be set if an exception has been thrown.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should, but I initially dropped this as I was expecting some kind of memory read error or segfault but that didn't happen, will update this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a ZEND_ASSERT(Z_TYPE(result) == IS_LONG); here.

return Z_LVAL(result);
}
/* }}} */
Expand Down
23 changes: 23 additions & 0 deletions ext/gmp/tests/comparison_invalid.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
--TEST--
Invalid comparison with a GMP object
--SKIPIF--
<?php if (!extension_loaded("gmp")) print "skip"; ?>
--FILE--
<?php

try {
var_dump("hapfegfbu" > gmp_init(0));
} catch (\Error $e) {
echo $e::class, ': ', $e->getMessage(), \PHP_EOL;
}

try {
var_dump((new DateTime()) > gmp_init(0));
} catch (\Error $e) {
echo $e::class, ': ', $e->getMessage(), \PHP_EOL;
}

?>
--EXPECT--
TypeError: main(): Argument #2 is not an integer string
TypeError: main(): Argument #2 must be of type GMP|string|int, DateTime given