MySQL Bugs: #83136: for libmysql the [client] group may be read twice

Bug #83136	for libmysql the [client] group may be read twice
Submitted:	24 Sep 2016 15:47	Modified:	17 Nov 2016 10:13
Reporter:	John Fawcett	Email Updates:
Status:	No Feedback	Impact on me:	None
Category:	MySQL Server: C API (client library)	Severity:	S3 (Non-critical)
Version:	5.7.15	OS:	Any
Assigned to:	Assigned Account	CPU Architecture:	Any
Tags:	libmysql [client] group

Description:
in function mysql_read_default_options in mysql-5.7.15/sql-common/client.c the following line of code deals with reading options from the default files.

The [client] group is read followed by the specific group if present.

        groups[0]= (char*) "client"; groups[1]= (char*) group; groups[2]=0;

(The specific group is set by calling 
mysql_options(&mysql,MYSQL_READ_DEFAULT_GROUP,"your_prog_name");

I noticed that it is possible to use mysql_options(&mysql,MYSQL_READ_DEFAULT_GROUP,"client");
and that in this case it appears that the [client] group is being read twice. While that doesn't seem to cause any harm, it is unnecessary to read it twice.

By the way, the use case for being able to call mysql_options with "client" group is to ensure that the default config files get read, but without introducing a specific group for the program.

How to repeat:
By calling 

mysql_options(&mysql,MYSQL_READ_DEFAULT_GROUP,"client");

Suggested fix:
The suggested fix is to check if the group name supplied is client and not pass it.

Just by calling below two functions in an application will not make the 
config file to be read.
mysql_options(&mysql,MYSQL_READ_DEFAULT_FILE,"abc.cnf");
mysql_options(&mysql,MYSQL_READ_DEFAULT_GROUP,"client");
We need to call mysql_read_default_options ( this function is internal and
not exposed). Or my_load_defaults() which is not recommended. So i want to
know how is your application program which links to libmysql is written.

groups[0]= (char*) "client"; groups[1]= (char*) group; groups[2]=0;
Here we set "client" as default group only to ensure that for any client
application "client" group is read by default.

Here is a minimal program that demonstrates the issue. Before compiling it is necessary to change the credentials to a valid user_name, password and database.

// CODE START
#include <stdio.h>
#include <mysql.h>

int main(int argc, char **argv)
{
        char *host_name="localhost";
        char *user_name="testuser";
        char *password="password";
        char *db_name="test";
        MYSQL  *mysql;
        if ((mysql = mysql_init(NULL)) == NULL)
        printf("insufficient memory\n");

        if (mysql_options(mysql,MYSQL_READ_DEFAULT_GROUP,"client"))
        {
                printf("mysql_options() failed\n");
                mysql_close(mysql);
                exit(1);
        }

        if (mysql_real_connect (mysql, host_name, user_name, password,
                db_name, 3306, 0, CLIENT_MULTI_STATEMENTS) == NULL)
        {
                printf("mysql_real_connect() failed\n");
                mysql_close(mysql);
                exit(1);
        }

        mysql_close(mysql);
        return 0;
}
// CODE END

The following are the compile commands for the above program

cc -g -c test.c -o test.o -I/usr/include/mysql
cc test.o -o test -lmysqlclient -L/usr/lib64/mysql

Here is a run of the program under gdb debuuger using a breakpoint at the function my_load_defaults()

At that point the input arguments to the function include the groups parameter which has the following contents;

$1 = {0x7ffff787cab1 "client", 0x602720 "client", 0x0}

So client group is being passed in as the first and second parameter. That seems inefficient. It is sufficient to pass only the first parameter when the second parameter is also "client".

gdb ./test
GNU gdb (GDB) Red Hat Enterprise Linux 7.6.1-80.el7
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /root/test...done.
(gdb) b my_load_defaults
Function "my_load_defaults" not defined.
Make breakpoint pending on future shared library load? (y or [n]) y

Breakpoint 1 (my_load_defaults) pending.
(gdb) run
Starting program: /root/./test 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Breakpoint 1, my_load_defaults (conf_file=0x7ffff787cb16 "my", 
    groups=groups@entry=0x7fffffffd9a0, argc=argc@entry=0x7fffffffd960, 
    argv=argv@entry=0x7fffffffd980, 
    default_directories=default_directories@entry=0x0)
    at /export/home2/pb2/build/sb_1-20188648-1472134462.7/rpm/BUILD/mysql-5.7.15/mysql-5.7.15/mysys_ssl/my_default.cc:631
631	/export/home2/pb2/build/sb_1-20188648-1472134462.7/rpm/BUILD/mysql-5.7.15/mysql-5.7.15/mysys_ssl/my_default.cc: No such file or directory.
Missing separate debuginfos, use: debuginfo-install glibc-2.17-106.el7_2.8.x86_64 libgcc-4.8.5-4.el7.x86_64 libstdc++-4.8.5-4.el7.x86_64
(gdb) print *groups@3
$1 = {0x7ffff787cab1 "client", 0x602720 "client", 0x0}
(gdb) continue
Continuing.
[Inferior 1 (process 2182) exited normally]
(gdb)

$1 = {0x7ffff787cab1 "client", 0x602720 "client", 0x0}
Yes there is definitely a duplicate entry in the array, however this does not mean that the "client" group from the config file is read twice. This array is passed as an input in order to compare with the group read from the config file. If groups is present in the array then the contents of the group are read. 

In my opinion there is an issue of same entries in the array, but this is not going to affect any behaviour or performance, also this is more of an end user issue. Since "client" group is read by default in all client application why does end user still set "client" group using mysql_options().

Just to be sure I understood: are you saying that the group client is not read twice in the example I gave or are you saying it is read twice but as the result is functionally correct and the performance impact of reading it twice is negligible, it is not worth fixing?

You said that <<client" group is read by default in all
client application why does end user still set "client" group using
mysql_options()>>. Actually that statement does not correspond to how the code works and contrasts with the documentation too. 

As the code stands when using the C API the client group is only read if MYSQL_READ_DEFAULT_FILE or MYSQL_READ_DEFAULT_GROUP is set via mysql_options call prior to connecting. So if I only want to read the client group from the default file I have to call mysql_options with "client" group. But then it gets passed in twice.

If the code should work as you said, always reading the "client" group, then I would open a bug report to make the code and documentation compliant, as that is not the way it works now. See below that the client group is read only if there is cnf file or cnf group specified.

    MYSQL * STDCALL 
    CLI_MYSQL_REAL_CONNECT(MYSQL *mysql, ... other args...
    {
      /* use default options */
BUG->>if (mysql->options.my_cnf_file || mysql->options.my_cnf_group)<<-BUG
      {
        mysql_read_default_options(&mysql->options,
                                   (mysql->options.my_cnf_file ?
                                    mysql->options.my_cnf_file : "my"),
                                   mysql->options.my_cnf_group);

Just to claify in the 5.7 code base the potential bug is in function mysql_real_connect()
in file libmysqld/libmysqld.c

line 107
 if (mysql->options.my_cnf_file || mysql->options.my_cnf_group)

Hello John,
Iam saying that "client" group from the config file is not read twice even 
if the groups parameter which is passed to my_load_defaults() has two 
"client" entries. If you check this function handle_default_option() you can 
see that group parameter is used only to do a comparison to see if the group
is present in the config file. If present read the option and populate in
ctx.

  if (find_type((char *)group_name, ctx->group, FIND_TYPE_NO_PREFIX))
  {
    if (!(tmp= (char *) alloc_root(ctx->alloc, strlen(option) + 1)))
      return 1;
  ....
  }

Hi Satish

thanks for clarifying this. As there is no duplication of processing when passing in client group twice this bug should be closed.

Only doubt now is whether I should open a bug report so that clien group is read by default in all client application using the C API instead of as now where it is read only if MYSQL_READ_DEFAULT_FILE or MYSQL_READ_DEFAULT_GROUP is set via mysql_options call prior to connecting.

Thanks again.

Hello John,
Irrespective of calling mysql_options() with MYSQL_READ_DEFAULT_FILE or MYSQL_READ_DEFAULT_GROUP "client" group is always read.
Please do a grep of "load_default_groups" in client folder you will see that the array has "client" as an entry, which states that "client" is always read.

No feedback was provided for this bug for over a month, so it is
being suspended automatically. If you are able to provide the
information that was originally requested, please do so and change
the status of the bug back to "Open".