BUG30416704: Binary columns returned as strings

This patch fixes the result of binary columns that were returning strings
instead, for both pure Python and C extension.

Existing tests were changed accordingly, and added new ones for regression.

Author: nmariz

CHANGES.txt

@@ -18,6 +18,7 @@ v8.0.24
 - BUG#32435181: Add support for Django 3.2
 - BUG#32029891: Add context manager support for pooled connections
 - BUG#31490101: Fix wrong cast of Python unicode to std::string
+- BUG#30416704: Binary columns returned as strings
 
 v8.0.23
 =======

lib/mysql/connector/conversion.py

@@ -1,4 +1,4 @@
-# Copyright (c) 2009, 2020, Oracle and/or its affiliates.
+# Copyright (c) 2009, 2021, Oracle and/or its affiliates.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License, version 2.0, as
@@ -582,21 +582,15 @@ def _STRING_to_python(self, value, dsc=None):  # pylint: disable=C0103
 
         Returns string typed columns as string type.
         """
+        if self.charset == "binary":
+            return value
         if dsc is not None:
-            # Check if we deal with a SET
+            if dsc[1] == FieldType.JSON and self.use_unicode:
+                return value.decode(self.charset)
             if dsc[7] & FieldFlag.SET:
                 return self._SET_to_python(value, dsc)
-            if dsc[7] & FieldFlag.BINARY:
-                if self.charset != 'binary' and not isinstance(value, str):
-                    try:
-                        return value.decode(self.charset)
-                    except (LookupError, UnicodeDecodeError):
-                        return value
-                else:
-                    return value
-
-        if self.charset == 'binary':
-            return value
+            if dsc[8] == 63:  # 'binary' charset
+                return value
         if isinstance(value, (bytes, bytearray)) and self.use_unicode:
             return value.decode(self.charset)
 

lib/mysql/connector/protocol.py

@@ -1,4 +1,4 @@
-# Copyright (c) 2009, 2020, Oracle and/or its affiliates.
+# Copyright (c) 2009, 2021, Oracle and/or its affiliates.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License, version 2.0, as
@@ -266,7 +266,7 @@ def parse_column_count(self, packet):
         except (struct.error, ValueError):
             raise errors.InterfaceError("Failed parsing column count")
 
-    def parse_column(self, packet, charset='utf-8'):
+    def parse_column(self, packet, encoding='utf-8'):
         """Parse a MySQL column-packet"""
         (packet, _) = utils.read_lc_string(packet[4:])  # catalog
         (packet, _) = utils.read_lc_string(packet)  # db
@@ -276,20 +276,26 @@ def parse_column(self, packet, charset='utf-8'):
         (packet, _) = utils.read_lc_string(packet)  # org_name
 
         try:
-            (_, _, field_type,
-             flags, _) = struct.unpack('<xHIBHBxx', packet)
+            (
+                charset,
+                _,
+                column_type,
+                flags,
+                _,
+            ) = struct.unpack('<xHIBHBxx', packet)
         except struct.error:
             raise errors.InterfaceError("Failed parsing column information")
 
         return (
-            name.decode(charset),
-            field_type,
+            name.decode(encoding),
+            column_type,
             None,  # display_size
             None,  # internal_size
             None,  # precision
             None,  # scale
             ~flags & FieldFlag.NOT_NULL,  # null_ok
             flags,  # MySQL specific
+            charset,
         )
 
     def parse_eof(self, packet):

src/include/mysql_capi_conversion.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2021, Oracle and/or its affiliates. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License, version 2.0, as
@@ -70,8 +70,11 @@ PyObject*
 mytopy_bit(const char *data, const unsigned long length);
 
 PyObject*
-mytopy_string(const char *data, const unsigned long length,
-              const unsigned long flags, const char *charset,
+mytopy_string(const char *data,
+              enum_field_types field_type,
+              const unsigned int field_charsetnr,
+              const unsigned long field_length,
+              const char *charset,
               unsigned int use_unicode);
 
 #endif /* MYCONNPY_MYSQL_CAPI_CONVERSION_H */

src/mysql_capi.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2020, Oracle and/or its affiliates.
+ * Copyright (c) 2014, 2021, Oracle and/or its affiliates.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License, version 2.0, as
@@ -213,33 +213,57 @@ fetch_fields(MYSQL_RES *result, unsigned int num_fields, MY_CHARSET_INFO *cs,
     {
         field = PyTuple_New(11);
 
-        decoded= mytopy_string(myfs[i].catalog, myfs[i].catalog_length,
-                               myfs[i].flags, charset, use_unicode);
+        decoded= mytopy_string(myfs[i].catalog,
+                               myfs[i].type,
+                               45,
+                               myfs[i].catalog_length,
+                               charset,
+                               use_unicode);
         if (NULL == decoded) return NULL; // decode error
         PyTuple_SET_ITEM(field, 0, decoded);
 
-        decoded= mytopy_string(myfs[i].db, myfs[i].db_length,
-                               myfs[i].flags, charset, use_unicode);
+        decoded= mytopy_string(myfs[i].db,
+                               myfs[i].type,
+                               45,
+                               myfs[i].db_length,
+                               charset,
+                               use_unicode);
         if (NULL == decoded) return NULL; // decode error
         PyTuple_SET_ITEM(field, 1, decoded);
 
-        decoded= mytopy_string(myfs[i].table, myfs[i].table_length,
-                               myfs[i].flags, charset, use_unicode);
+        decoded= mytopy_string(myfs[i].table,
+                               myfs[i].type,
+                               45,
+                               myfs[i].table_length,
+                               charset,
+                               use_unicode);
         if (NULL == decoded) return NULL; // decode error
         PyTuple_SET_ITEM(field, 2, decoded);
 
-        decoded= mytopy_string(myfs[i].org_table, myfs[i].org_table_length,
-                               myfs[i].flags, charset, use_unicode);
+        decoded= mytopy_string(myfs[i].org_table,
+                               myfs[i].type,
+                               45,
+                               myfs[i].org_table_length,
+                               charset,
+                               use_unicode);
         if (NULL == decoded) return NULL; // decode error
         PyTuple_SET_ITEM(field, 3, decoded);
 
-        decoded= mytopy_string(myfs[i].name, myfs[i].name_length,
-                               myfs[i].flags, charset, use_unicode);
+        decoded= mytopy_string(myfs[i].name,
+                               myfs[i].type,
+                               45,
+                               myfs[i].name_length,
+                               charset,
+                               use_unicode);
         if (NULL == decoded) return NULL; // decode error
         PyTuple_SET_ITEM(field, 4, decoded);
 
-        decoded= mytopy_string(myfs[i].org_name, myfs[i].org_name_length,
-                               myfs[i].flags, charset, use_unicode);
+        decoded= mytopy_string(myfs[i].org_name,
+                               myfs[i].type,
+                               45,
+                               myfs[i].org_name_length,
+                               charset,
+                               use_unicode);
         if (NULL == decoded) return NULL; // decode error
         PyTuple_SET_ITEM(field, 5, decoded);
 
@@ -383,13 +407,9 @@ MySQL_init(MySQL *self, PyObject *args, PyObject *kwds)
         self->raw= self->raw_at_connect;
     }
 
-    self->use_unicode= 0;
-    if (use_unicode)
+    if (use_unicode && use_unicode == Py_False)
     {
-        if (use_unicode == Py_True)
-        {
-            self->use_unicode= 1;
-        }
+        self->use_unicode= 0;
     }
 
     if (charset_name) {
@@ -2383,7 +2403,7 @@ MySQL_fetch_row(MySQL *self)
 	unsigned long *field_lengths;
 	unsigned int num_fields;
 	unsigned int i;
-	unsigned long field_type, field_flags;
+	unsigned long field_charsetnr, field_type, field_flags;
 	const char *charset= NULL;
 
     CHECK_SESSION(self);
@@ -2458,6 +2478,7 @@ MySQL_fetch_row(MySQL *self)
             Py_RETURN_NONE;
         }
 
+        field_charsetnr= PyLong_AsUnsignedLong(PyTuple_GetItem(field_info, 6));
         field_type= PyLong_AsUnsignedLong(PyTuple_GetItem(field_info, 8));
         field_flags= PyLong_AsUnsignedLong(PyTuple_GetItem(field_info, 9));
 
@@ -2491,8 +2512,12 @@ MySQL_fetch_row(MySQL *self)
                  field_type == MYSQL_TYPE_ENUM ||
                  field_type == MYSQL_TYPE_VAR_STRING)
         {
-            value= mytopy_string(row[i], field_lengths[i], field_flags,
-                                  charset, self->use_unicode);
+            value= mytopy_string(row[i],
+                                 field_type,
+                                 field_charsetnr,
+                                 field_lengths[i],
+                                 charset,
+                                 self->use_unicode);
             if (!value)
             {
                 goto error;
@@ -2563,8 +2588,12 @@ MySQL_fetch_row(MySQL *self)
             }
             else
             {
-                value= mytopy_string(row[i], field_lengths[i], field_flags,
-                                     charset, self->use_unicode);
+                value= mytopy_string(row[i],
+                                     field_type,
+                                     field_charsetnr,
+                                     field_lengths[i],
+                                     charset,
+                                     self->use_unicode);
             }
             PyTuple_SET_ITEM(result_row, i, value);
         }
@@ -2577,8 +2606,12 @@ MySQL_fetch_row(MySQL *self)
     	else
     	{
     	    // Do our best to convert whatever we got from MySQL to a str/bytes
-            value = mytopy_string(row[i], field_lengths[i], field_flags,
-                                  charset, self->use_unicode);
+            value = mytopy_string(row[i],
+                                  field_type,
+                                  field_charsetnr,
+                                  field_lengths[i],
+                                  charset,
+                                  self->use_unicode);
     		PyTuple_SET_ITEM(result_row, i, value);
     	}
     }
@@ -3553,6 +3586,10 @@ MySQLPrepStmt_fetch_row(MySQLPrepStmt *self)
                         PyTuple_SET_ITEM(row, i,
                             mytopy_bit(PyBytes_AsString(obj), self->cols[i].length));
                     }
+                    else if (field->charsetnr == 63) /* 'binary' charset */
+                    {
+                        PyTuple_SET_ITEM(row, i, PyByteArray_FromObject(obj));
+                    }
                     else
                     {
                         PyTuple_SET_ITEM(row, i, PyUnicode_FromString(PyBytes_AsString(obj)));

src/mysql_capi_conversion.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2020, Oracle and/or its affiliates.
+ * Copyright (c) 2014, 2021, Oracle and/or its affiliates.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License, version 2.0, as
@@ -711,32 +711,40 @@ pytomy_decimal(PyObject *obj)
   Convert, and decode if needed, a string MySQL value to
   Python str or bytes.
 
-  @param    data        string to be converted
-  @param    length      length of data
-  @param    flags       field flags
-  @param    charset     character used for decoding
-  @param    use_unicode return Unicode
+  @param    data             string to be converted
+  @param    field_type       field type
+  @param    field_charsetnr  charset number
+  @param    field_length     length of data
+  @param    charset          character used for decoding
+  @param    use_unicode      use unicode
 
   @return   Converted string
-    @retval PyUnicode   if not BINARY_FLAG
-    @retval PyBytes     Python v3 if not use_unicode
-    @retval NULL    Exception
+    @retval PyUnicode   if use unicode
+    @retval PyBytes     if not use_unicode or charset is 'binary'
+    @retval NULL        Exception
  */
 PyObject*
-mytopy_string(const char *data, const unsigned long length,
-              const unsigned long flags, const char *charset,
+mytopy_string(const char *data,
+              enum_field_types field_type,
+              const unsigned int field_charsetnr,
+              const unsigned long field_length,
+              const char *charset,
               unsigned int use_unicode)
 {
     if (!charset || !data) {
         return NULL;
     }
 
-    if (!((flags != 0) & flags & BINARY_FLAG) && use_unicode && strcmp(charset, "binary") != 0)
+    if (strcmp(charset, "binary") == 0)
     {
-        return PyUnicode_Decode(data, length, charset, NULL);
+        return PyByteArray_FromStringAndSize(data, field_length);
     }
-    else
+
+    /* 'binary' charset = 63 */
+    if (use_unicode && (field_type == MYSQL_TYPE_JSON || field_charsetnr != 63))
     {
-        return PyBytes_FromStringAndSize(data, length);
+        return PyUnicode_Decode(data, field_length, charset, NULL);
     }
+
+    return PyByteArray_FromStringAndSize(data, field_length);
 }

tests/cext/test_cext_api.py

@@ -1,6 +1,6 @@
 # -*- coding: utf-8 -*-
 
-# Copyright (c) 2014, 2020, Oracle and/or its affiliates.
+# Copyright (c) 2014, 2021, Oracle and/or its affiliates.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License, version 2.0, as
@@ -337,12 +337,12 @@ def test_select_db(self):
 
         cmy.select_db('mysql')
         cmy.query("SELECT DATABASE()")
-        self.assertEqual(b'mysql', cmy.fetch_row()[0])
+        self.assertEqual('mysql', cmy.fetch_row()[0])
         cmy.free_result()
 
         cmy.select_db('myconnpy')
         cmy.query("SELECT DATABASE()")
-        self.assertEqual(b'myconnpy', cmy.fetch_row()[0])
+        self.assertEqual('myconnpy', cmy.fetch_row()[0])
         cmy.free_result()
 
     def test_affected_rows(self):
@@ -612,12 +612,12 @@ def test_set_character_set(self):
                           cmy1.set_character_set, 'ham_spam')
 
         variables = ('character_set_connection',)
-        exp = b'utf8'
+        exp = 'utf8'
         self.assertIn(
             exp,
-            get_variables(cmy1, variables=variables)[b'character_set_connection'])
+            get_variables(cmy1, variables=variables)['character_set_connection'])
 
-        exp = {b'character_set_connection': b'big5',}
+        exp = {'character_set_connection': 'big5'}
         cmy1.set_character_set('big5')
         self.assertEqual(exp, get_variables(cmy1, variables=variables))
 
@@ -681,7 +681,7 @@ def create_table(charset):
                     fetched = fetch_rows(cmy)[0][0]
                 except UnicodeEncodeError:
                     self.fail("Could not encode {0}".format(encoding))
-                self.assertEqual(case, fetched.decode(encoding),
+                self.assertEqual(case, fetched,
                                  "Failed with case {0}/{1}".format(i, encoding))
 
         cmy.query("DROP TABLE IF EXISTS {0}".format(table))
@@ -838,9 +838,9 @@ def test_next_result(self):
             "SELECT 'SPAM'",
         )
         exp = [
-            [(b'HAM',)],
+            [('HAM',)],
             {'insert_id': 1, 'affected': 1},
-            [(b'SPAM',)]
+            [('SPAM',)]
         ]
 
         result = []