Skip to content

Mysqli: fix missing error for invalid mysqli_options() option #20971

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

Open
arshidkv12 wants to merge 19 commits into
base: master
Choose a base branch
from
+47 −6
Open

Mysqli: fix missing error for invalid mysqli_options() option #20971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a401699
Fix bug gh20968
arshidkv12 Jan 19, 2026
217b19b
mysqli: raise ValueError for invalid option in mysqli_options() respe…
arshidkv12 Jan 19, 2026
66fbc0a
mysqli: raise ValueError for invalid option in mysqli_options() respe…
arshidkv12 Jan 19, 2026
54a8e06
mysqli: raise ValueError for invalid option in mysqli_options() respe…
arshidkv12 Jan 19, 2026
0716f97
mysqli: raise ValueError for invalid option in mysqli_options() respe…
arshidkv12 Jan 19, 2026
d7dd938
mysqli: raise ValueError for invalid option in mysqli_options() respe…
arshidkv12 Jan 19, 2026
1aa360a
mysqli: raise ValueError for invalid option in mysqli_options() respe…
arshidkv12 Jan 19, 2026
04f8006
Update ext/mysqli/mysqli_api.c
arshidkv12 Jan 19, 2026
9ba4bf2
Update ext/mysqli/tests/gh20968.phpt
arshidkv12 Jan 19, 2026
2db6e98
mysqli: raise ValueError for invalid option in mysqli_options() respe…
arshidkv12 Jan 19, 2026
29ed278
mysqli: raise ValueError for invalid option in mysqli_options() respe…
arshidkv12 Jan 20, 2026
1647ae8
mysqli: raise ValueError for invalid option in mysqli_options() respe…
arshidkv12 Jan 20, 2026
0a98c9c
mysqli: raise ValueError for invalid option in mysqli_options() respe…
arshidkv12 Jan 20, 2026
8193542
Update ext/mysqli/mysqli_api.c
arshidkv12 Jan 20, 2026
3a633b3
Update ext/mysqli/tests/mysqli_options.phpt
arshidkv12 Jan 20, 2026
072d1a4
mysqli: raise ValueError for invalid option in mysqli_options() respe…
arshidkv12 Jan 20, 2026
a188868
Update ext/mysqli/tests/mysqli_set_opt.phpt
arshidkv12 Jan 20, 2026
e102967
mysqli: raise ValueError for invalid option in mysqli_options() respe…
arshidkv12 Jan 20, 2026
6a71d4f
mysqli: raise ValueError for invalid option in mysqli_options() respe…
arshidkv12 Jan 20, 2026
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
5 changes: 5 additions & 0 deletions ext/mysqli/mysqli_api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1198,6 +1198,11 @@ PHP_FUNCTION(mysqli_options)
MYSQLI_FETCH_RESOURCE_CONN(mysql, mysql_link, MYSQLI_STATUS_INITIALIZED);

expected_type = mysqli_options_get_option_zval_type(mysql_option);
if (expected_type == IS_NULL) {
zend_argument_value_error(ERROR_ARG_POS(2), "must be one of predefined options");
RETURN_THROWS();
}
Comment on lines 1200 to +1204
Copy link
Member

@Girgias Girgias Jan 20, 2026

Choose a reason for hiding this comment

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

We normally list the available options, and if there are too many we at least provide a prefix for the error eg. MYSQLI_BLAH_X

Copy link
Member

@kamil-tekiela kamil-tekiela Jan 20, 2026

Choose a reason for hiding this comment

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

I thought about that too, but there isn't any common pattern between them. One of the values doesn't even have a corresponding constants.

Copy link
Member

@Girgias Girgias Jan 20, 2026

Choose a reason for hiding this comment

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

That seems kinda bad that one of the options doesn't have a constant?

As future scope, would it make sense to have an Enum of MySQLi options? As that would make the type checking and value validation easy.

Copy link
Member

@kamil-tekiela kamil-tekiela Jan 20, 2026

Choose a reason for hiding this comment

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

Yes, it is. Maybe we should add the constant. Enums would definitely be better but it would be a breaking change difficult for cross-version compatibilty.

Copy link
Member

@Girgias Girgias Jan 20, 2026

Choose a reason for hiding this comment

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

Yes, it is. Maybe we should add the constant. Enums would definitely be better but it would be a breaking change difficult for cross-version compatibilty.

It would be possible to assign the enum case as the constant value, or support int|Enum. But I think already adding the missing constant is the big thing.


if (expected_type != Z_TYPE_P(mysql_value)) {
switch (expected_type) {
case IS_STRING:
Expand Down
27 changes: 27 additions & 0 deletions ext/mysqli/tests/gh20968.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
--TEST--
Bug #20968 mysqli_options() with invalid option triggers ValueError
Copy link
Member

@Girgias Girgias Jan 20, 2026

Choose a reason for hiding this comment

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

Suggested change
Bug #20968 mysqli_options() with invalid option triggers ValueError
GH-20968 mysqli_options() with invalid option triggers ValueError

We use Bug #xxxxx notation to refer to old bugsnet bugs.

Copy link
Member

@Girgias Girgias Jan 20, 2026

Choose a reason for hiding this comment

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

Suggested change
Bug #20968 mysqli_options() with invalid option triggers ValueError
Bug #20968 mysqli_options() with invalid option should triggers ValueError

The phrasing of the bug title seems to imply that an invalid option MUST NOT thow a ValueError, which is the complete opposite of what is being done.

--EXTENSIONS--
mysqli
--CONFLICTS--
mysqli
--SKIPIF--
<?php
require_once 'skipifconnectfailure.inc';
?>
--FILE--
<?php
require_once 'connect.inc';

$mysqli = new mysqli($host, $user, $passwd, $db, $port, $socket);

$value = $mysqli->options(10, 'invalid_option');
Copy link
Member

@Girgias Girgias Jan 20, 2026

Choose a reason for hiding this comment

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

Add a try catch here and check the exception message, we don't care about the stack trace.


var_dump($value);

?>
--EXPECTF--
Fatal error: Uncaught ValueError: mysqli::options(): Argument #1 ($option) must be one of predefined options in %s:%d
Stack trace:
#0 %s(%d): mysqli->options(%d, 'invalid_option')
#1 {main}
thrown in %s on line %d
12 changes: 8 additions & 4 deletions ext/mysqli/tests/mysqli_options.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,13 @@ var_dump("MYSQLI_OPT_LOCAL_INFILE", mysqli_options($link, MYSQLI_OPT_LOCAL_INFIL
var_dump("MYSQLI_INIT_COMMAND", mysqli_options($link, MYSQLI_INIT_COMMAND, 'SET AUTOCOMMIT=0'));

/* mysqli_real_connect() */
var_dump("MYSQLI_CLIENT_SSL", mysqli_options($link, MYSQLI_CLIENT_SSL, 'not a mysqli_option'));
var_dump("MYSQLI_CLIENT_SSL");

try {
var_dump(mysqli_options($link, MYSQLI_CLIENT_SSL, 'not a mysqli_option'));
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}

mysqli_close($link);

Expand All @@ -81,8 +87,6 @@ try {
echo $e->getMessage() . "\n";
}

// invalid options do not generate errors
mysqli_options($link, -1, "Invalid option");

print "done!";
?>
Expand Down Expand Up @@ -110,7 +114,7 @@ bool(true)
%s(19) "MYSQLI_INIT_COMMAND"
bool(true)
%s(17) "MYSQLI_CLIENT_SSL"
bool(false)
mysqli_options(): Argument #2 ($option) must be one of predefined options
Link closed
mysqli object is already closed
Unknown character set
Expand Down
9 changes: 7 additions & 2 deletions ext/mysqli/tests/mysqli_set_opt.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ require_once 'skipifconnectfailure.inc';
var_dump(mysqli_set_opt($link, MYSQLI_OPT_CONNECT_TIMEOUT, 10));
var_dump(mysqli_set_opt($link, MYSQLI_OPT_LOCAL_INFILE, 1));
var_dump(mysqli_set_opt($link, MYSQLI_INIT_COMMAND, 'SET AUTOCOMMIT=0'));
var_dump(mysqli_set_opt($link, MYSQLI_CLIENT_SSL, 'not an mysqli_option'));

try {
var_dump(mysqli_set_opt($link, MYSQLI_CLIENT_SSL, 'not an mysqli_option'));
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}

mysqli_close($link);

Expand All @@ -48,6 +53,6 @@ bool(true)
bool(true)
bool(true)
bool(true)
bool(false)
mysqli_set_opt(): Argument #2 ($option) must be one of predefined options
mysqli object is already closed
done!
Loading