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.

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

Jump to bottom

Support get_server_public_key option #1377

Merged
merged 2 commits into from
Sep 6, 2024
Merged

Support get_server_public_key option #1377

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
ca32f89
caching_sha2_password requests secure connection if cache is not read…
samitani Aug 31, 2024
8f2bbe3
return error if get_server_public_key option is not supported
samitani Sep 5, 2024
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
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ Mysql2::Client.new(
:reconnect = true/false,
:local_infile = true/false,
:secure_auth = true/false,
:get_server_public_key = true/false,
:default_file = '/path/to/my.cfg',
:default_group = 'my.cfg section',
:default_auth = 'authentication_windows_client'
Expand Down
16 changes: 16 additions & 0 deletions ext/mysql2/client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,13 @@ static VALUE _mysql_client_options(VALUE self, int opt, VALUE value) {
retval = charval;
break;

#ifdef HAVE_CONST_MYSQL_OPT_GET_SERVER_PUBLIC_KEY
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#ifdef HAVE_CONST_MYSQL_OPT_GET_SERVER_PUBLIC_KEY
#ifdef MYSQL_OPT_GET_SERVER_PUBLIC_KEY

case MYSQL_OPT_GET_SERVER_PUBLIC_KEY:
boolval = (value == Qfalse ? 0 : 1);
retval = &boolval;
break;
#endif

#ifdef HAVE_MYSQL_DEFAULT_AUTH
case MYSQL_DEFAULT_AUTH:
charval = (const char *)StringValueCStr(value);
Expand Down Expand Up @@ -1485,6 +1492,14 @@ static VALUE set_init_command(VALUE self, VALUE value) {
return _mysql_client_options(self, MYSQL_INIT_COMMAND, value);
}

static VALUE set_get_server_public_key(VALUE self, VALUE value) {
#ifdef HAVE_CONST_MYSQL_OPT_GET_SERVER_PUBLIC_KEY
sodabrew marked this conversation as resolved.
Show resolved Hide resolved
return _mysql_client_options(self, MYSQL_OPT_GET_SERVER_PUBLIC_KEY, value);
#else
rb_raise(cMysql2Error, "get-server-public-key is not available, you may need a newer MySQL client library");
#endif
}

static VALUE set_default_auth(VALUE self, VALUE value) {
#ifdef HAVE_MYSQL_DEFAULT_AUTH
return _mysql_client_options(self, MYSQL_DEFAULT_AUTH, value);
Expand Down Expand Up @@ -1596,6 +1611,7 @@ void init_mysql2_client() {
rb_define_private_method(cMysql2Client, "default_file=", set_read_default_file, 1);
rb_define_private_method(cMysql2Client, "default_group=", set_read_default_group, 1);
rb_define_private_method(cMysql2Client, "init_command=", set_init_command, 1);
rb_define_private_method(cMysql2Client, "get_server_public_key=", set_get_server_public_key, 1);
Copy link
Collaborator

@sodabrew sodabrew Sep 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The method name set_get_... seems extraneous? Can it be plain get_server_public_key? nm, this is a setter. Weird. Reading more.

rb_define_private_method(cMysql2Client, "default_auth=", set_default_auth, 1);
rb_define_private_method(cMysql2Client, "ssl_set", set_ssl_options, 5);
rb_define_private_method(cMysql2Client, "ssl_mode=", rb_set_ssl_mode_option, 1);
Expand Down
1 change: 1 addition & 0 deletions ext/mysql2/extconf.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ def add_ssl_defines(header)
have_const('SERVER_QUERY_WAS_SLOW', mysql_h)
have_const('MYSQL_OPTION_MULTI_STATEMENTS_ON', mysql_h)
have_const('MYSQL_OPTION_MULTI_STATEMENTS_OFF', mysql_h)
have_const('MYSQL_OPT_GET_SERVER_PUBLIC_KEY', mysql_h)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not necessary to test for this constant into its own HAVE_ constant, because we can directly test for the constant at time of use (see suggested change)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sodabrew Hello, Thank you for reviewing my PR.
MYSQL_OPT_GET_SERVER_PUBLIC_KEY is defined as enum. If my understand correctly, ifdef does not work with enum.
https://github.com/mysql/mysql-server/blob/8.0/include/mysql.h.pp#L411-L448

Copy link
Collaborator

Choose a reason for hiding this comment

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

You’re absolutely right!


# my_bool is replaced by C99 bool in MySQL 8.0, but we want
# to retain compatibility with the typedef in earlier MySQLs.
Expand Down
4 changes: 2 additions & 2 deletions lib/mysql2/client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ def initialize(opts = {})
opts[:connect_timeout] = 120 unless opts.key?(:connect_timeout)

# TODO: stricter validation rather than silent massaging
%i[reconnect connect_timeout local_infile read_timeout write_timeout default_file default_group secure_auth init_command automatic_close enable_cleartext_plugin default_auth].each do |key|
%i[reconnect connect_timeout local_infile read_timeout write_timeout default_file default_group secure_auth init_command automatic_close enable_cleartext_plugin default_auth get_server_public_key].each do |key|
next unless opts.key?(key)

case key
when :reconnect, :local_infile, :secure_auth, :automatic_close, :enable_cleartext_plugin
when :reconnect, :local_infile, :secure_auth, :automatic_close, :enable_cleartext_plugin, :get_server_public_key
send(:"#{key}=", !!opts[key]) # rubocop:disable Style/DoubleNegation
when :connect_timeout, :read_timeout, :write_timeout
send(:"#{key}=", Integer(opts[key])) unless opts[key].nil?
Expand Down