MySQL++

MariaDB 10.4.x needs mysql_library_init() for escape_string_no_conn()
Login

MariaDB 10.4.x needs mysql_library_init() for escape_string_no_conn()

(1) By Luke Mewburn (lukemewburn) on 2020-07-07 02:02:14 [link] [source]

Summary

MariaDB 10.4.13 needs mysql_library_init() called before mysql_escape_string() or mysql_cset_escape_slashes() are called without a connection. See https://jira.mariadb.org/browse/MDEV-20116

Without calling mysql_library_init() first, various test programs that test without connection fault with SEGV.

Fix

I worked around this with the following patch. This method does not require specific changes to MySQL++ consumer apps besides a recompile. The maintainers may prefer a different solution.

    Ensure mysql_library_init() is called in escape_string_no_conn()
    before mysql_escape_string().  Fixes SEGV crashes in test applications.
    Refer to https://jira.mariadb.org/browse/MDEV-20116 for more information.

    diff -pu -r mysql++-3.2.5-orig/lib/dbdriver.cpp mysql++-3.2.5/lib/dbdriver.cpp
    --- mysql++-3.2.5-orig/lib/dbdriver.cpp 2020-02-28 07:31:27.000000000 +1100
    +++ mysql++-3.2.5/lib/dbdriver.cpp      2020-07-06 13:24:52.966530258 +1000
    @@ -196,6 +196,7 @@ size_t
     DBDriver::escape_string_no_conn(std::string* ps, const char* original,
                    size_t length)
     {
    +       mysql_library_init(0, 0, 0);
            if (ps == 0) {
                    // Can't do any real work!
                    return 0;
    diff -pu -r mysql++-3.2.5-orig/lib/dbdriver.h mysql++-3.2.5/lib/dbdriver.h
    --- mysql++-3.2.5-orig/lib/dbdriver.h   2020-02-28 07:31:27.000000000 +1100
    +++ mysql++-3.2.5/lib/dbdriver.h        2020-07-06 13:25:04.762282341 +1000
    @@ -266,6 +266,7 @@ public:
            static size_t escape_string_no_conn(char* to, const char* from,
                            size_t length)
            {
    +               mysql_library_init(0, 0, 0);
                    return mysql_escape_string(to, from,
                                    static_cast<unsigned long>(length));
            }

Details

(Note: this is on CentOS 7.8, MariaDB 10.4.13, compiling with devtoolset-7 from SCL using scl enable devtoolset-7 './configure && make'. The newer compiler should not be specifically triggering this issue).

When building MySQL++ 3.2.5 with MariaDB 10.4.13, various test applications SEGV:

./dtest
Running unit tests: test_manip FAILED (139)

./exrun: line 66: 29400 Segmentation fault      (core dumped) LD_LIBRARY_PATH=. $TOOL ./$PROG $*

Further analysis shows that the code is faulting in mysqlpp::DBDriver::escape_string_no_conn():

env LD_LIBRARY_PATH=. gdb ./test_manip
(gdb) run
Starting program: test_manip 
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7b9ec00 in mysql_cset_escape_slashes () from /lib64/libmariadb.so.3
Missing separate debuginfos, use: debuginfo-install MariaDB-shared-10.4.13-1.el7.centos.x86_64 keyutils-libs-1.5.8-3.el7.x86_64 krb5-libs-1.15.1-46.el7.x86_64 libcom_err-1.42.9-17.el7.x86_64 libgcc-4.8.5-39.el7.x86_64 libselinux-2.5-15.el7.x86_64 libstdc++-4.8.5-39.el7.x86_64 openssl-libs-1.0.2k-19.el7.x86_64 pcre-8.32-17.el7.x86_64 zlib-1.2.7-18.el7.x86_64
(gdb) bt
#0  0x00007ffff7b9ec00 in mysql_cset_escape_slashes () from /lib64/libmariadb.so.3
#1  0x00007ffff794872d in mysqlpp::DBDriver::escape_string_no_conn (length=28, 
    from=0x60a9c0 "Doodle me, James, doodle me!", to=0x60ac40 "") at ./lib/dbdriver.h:270
#2  mysqlpp::DBDriver::escape_string_no_conn (ps=ps@entry=0x7fffffffdd08, 
    original=0x60a9c0 "Doodle me, James, doodle me!", length=length@entry=28) at ./lib/dbdriver.cpp:217
#3  0x00007ffff79512a3 in mysqlpp::Query::escape_string (this=this@entry=0x7fffffffdee0, 
    ps=ps@entry=0x7fffffffdd08, original=<optimized out>, length=length@entry=28)
    at ./lib/query.cpp:120
#4  0x00007ffff794ab91 in mysqlpp::operator<< (o=..., o@entry=..., in=...) at ./lib/manip.cpp:73
#5  0x0000000000402dd8 in explicit_query_quote<char*> (
    test=test@entry=0x7fffffffe1e0 "Doodle me, James, doodle me!", len=len@entry=28)
    at ./test/manip.cpp:59
#6  0x0000000000403501 in test<char*> (test=0x7fffffffe1e0 "Doodle me, James, doodle me!", len=28)
    at ./test/manip.cpp:149
#7  0x0000000000401dc8 in main () at ./test/manip.cpp:162

When I apply the patch above and rebuild, the tests succeed. Some of the output of the examples fails, but at least there's no more coredumps:

./dtest -uTESTUSER -pTESTPASS
Running unit tests: 16 tests succeeded
Running examples:
   resetdb simple1 simple2 simple3 store_if for_each multiquery tquery1
   resetdb tquery2 tquery3 tquery4
   resetdb ssqls1 ssqls2 ssqls3 ssqls4 ssqls5 ssqls6 load_jpeg cgi_jpeg
Testing ssqlsxlat -i examples/*.ssqls -o...pass 1, pass 2, diffdiff: /tmp/dtest-ssxgv2-pass1-: No such file or directory
diff: /tmp/dtest-ssxgv2-pass2-: No such file or directory
[...]

(2) By Warren Young (tangent) on 2020-07-07 05:11:19 [link] [source] in reply to 1

I worked around this with the following patch

I don't like redundantly calling that function where you do. It's a fairly hot path, so we should not make calls that are 99.9% of the time pointless.

mysql_library_init() is automatically called in the normal use cases.

In this specific case, the reason your call path fails is that trying to escape a MySQL/MariaDB string without a connection is somewhat bogus from the start, because proper escaping depends on settings that can vary by DB server configuration and by per-database configuration on that server.

When you tell the library to escape your string without that info, it's giving you a kind of best-guess result, which may not be correct.

Bottom line, I'd say the proper solution is to either:

  1. Get a Connection first, and call through that. Then the underlying C API library will be initted, and the escaping will be done properly.

  2. Call mysql_library_init() once from your program's startup code.

(3) By Luke Mewburn (lukemewburn) on 2020-07-07 06:18:19 [link] [source] in reply to 2

The issue is in the mysql++ test applications dumping core, not in my code.

I instead propose the following fixes:

  1. Change the CommandLine::CommandLine() to call mysql_library_init(), since all the test apps appear to use that?
  2. Explicitly document that mysql_library_init() should be called at the start of an application before using:
    1. DBDriver::escape_string_no_conn()
    2. Query::escape_string() without a connection
    3. SQLStream::escape_string() with a connection

thoughts?

(4) By Warren Young (tangent) on 2020-07-07 11:49:22 [link] [source] in reply to 3

The issue is in the mysql++ test applications dumping core, not in my code.

Didn't notice that. This has to be a C API level regression of some kind, because the code as-is has been working for years and years.

Change the CommandLine::CommandLine() to call mysql_library_init()

Better.

Explicitly document that mysql_library_init() should be called at the start of an application before using:

No, because some other bit of code might have initted the library via another path.

Bottom line, this is supposed to be an internal hidden detail of the C API's implementation, not something normal code is supposed to care about. At least, that's what it was back when all of this was designed. Obviously these DBMSes have gone through multiple changes of hands, so now we're getting mismatches as new minds come along and decide that old decisions are no longer correct, breaking code like mine.

thoughts?

You're welcome to provide a patch for this.

(5) By Warren Young (tangent) on 2020-07-07 11:51:11 [link] [source] in reply to 4

The issue is in the mysql++ test applications dumping core, not in my code.

Also, this explains why we're calling these functions without a Connection: we don't want to have a MySQL DBMS up and running just to run the tests. We're happy with best-effort escaping and quoting in this case.

It isn't good practice normally, but in a test program...

(6) By Luke Mewburn (lukemewburn) on 2020-07-08 04:45:15 [link] [source] in reply to 4

Change the CommandLine::CommandLine() to call mysql_library_init()

Better.

Actually, that turns out not to work, because the 3 test apps that coredump don't use CommandLine. I've instead just added the call to mysql_library_init() to the start of main() in:

  • test/manip.cpp
  • test/qssqls.cpp
  • test/sqlstream.cpp

That fixes the coredumps; all ./dtest unit tests pass now, and if I supply DB creds all of the examples pass too.

Patch below.

Explicitly document that mysql_library_init() should be called at the start of an application before using:

No, because some other bit of code might have initted the library via another path.

The mysql_library_init()should be safe to call multiple times in an application. The MariaDB implementation uses pthread_once() to ensure that it's only invoked once. The method already gets called implicitly by mysql_init() anyway for each Connection constructed my mysql++ anyway.

Bottom line, this is supposed to be an internal hidden detail of the C API's implementation, not something normal code is supposed to care about. At least, that's what it was back when all of this was designed. Obviously these DBMSes have gone through multiple changes of hands, so now we're getting mismatches as new minds come along and decide that old decisions are no longer correct, breaking code like mine.

It is an unfortunate behaviour change made in the MariaDB connector that now effectively requires mysql_library_init() when previous code linked against the older MySQL connector "just worked" (even if it shouldn't have).

I still think it may be beneficial to document (even just a a FAQ) to the end users of your library who are also using the (newer) MariaDB connector implementation that in the edge case of "using escaping without at least constructing a Connection once" should call mysql_library_init() to avoid the run-time SEGV experienced in the testsuite here caused by the behaviour change in the upstream MariaDB database connector. YMMV.

Here's a diff that worked.

diff --git i/test/manip.cpp w/test/manip.cpp
index 13c6e0a..a0264b1 100644
--- i/test/manip.cpp
+++ w/test/manip.cpp
@@ -155,6 +155,8 @@ test(T test, size_t len)
 int
 main()
 {
+       mysql_library_init(0, 0, 0);
+
        char s[] = "Doodle me, James, doodle me!";
        const size_t len = strlen(s);
 
diff --git i/test/qssqls.cpp w/test/qssqls.cpp
index 14062b9..a87d941 100644
--- i/test/qssqls.cpp
+++ w/test/qssqls.cpp
@@ -59,6 +59,8 @@ sql_create_19(test,
 int
 main()
 {
+       mysql_library_init(0, 0, 0);
+
        Query q(0);             // don't pass 0 for conn parameter in real code
        test empty(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, false,
                        Date(), Time(), DateTime(), "", sql_blob());
diff --git i/test/sqlstream.cpp w/test/sqlstream.cpp
index e0c127a..d17c73f 100644
--- i/test/sqlstream.cpp
+++ w/test/sqlstream.cpp
@@ -65,6 +65,8 @@ sql_create_23(test,
 int
 main()
 {
+       mysql_library_init(0, 0, 0);
+
        SQLStream sqls(0);              // don't do this in real code
        test empty(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, false,
                        Date(), Time(), DateTime(),"","","","","","");

(7) By Warren Young (tangent) on 2020-07-10 20:36:33 [source] in reply to 6

Applied. Thanks!