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.

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

Fix malformed packet error in MySQL statement for connection configuration #41232

Merged
merged 1 commit into from Jan 26, 2021

Conversation

robinroestenburg
Copy link
Contributor

Summary

After upgrading our AWS RDS MySQL database from version 5.6.mysql_aurora.1.22.2 to version 5.7.mysql_aurora.2.09.1 I got the following ActiveRecord::StatementInvalid error while starting Rails:

ActiveRecord::StatementInvalid: Mysql2::Error::ConnectionError: received malformed packet: SET NAMES utf8mb4 COLLATE utf8mb4_unicode_ci,  @@SESSION.sql_mode = 'NO_ENGINE_SUBSTITUTION',  @@SESSION.character_set_connection = 'utf8mb4', @@SESSION.collation_connection = 'utf8mb4_unicode_ci', @@SESSION.group_concat_max_len = 1048576, @@SESSION.sql_auto_is_null = 0, @@SESSION.wait_timeout = 2147483

This error is specific to MySQL client that is used. When using the client from within our Alpine (v3.12) container it failed, client version:

mysql  Ver 15.1 Distrib 10.4.15-MariaDB, for Linux (x86_64) using readline 5.1

When using an older client the query worked, version 10.1.41-r0 (part of Alpine v3.7).

When using an older client outside of the container, the query worked as well:

mysql  Ver 15.1 Distrib 5.5.68-MariaDB, for Linux (x86_64) using readline 5.1

The error did not occur when I removed one or more of the session variables. For example, the following queries worked:

# without sql_auto_is_null
SET NAMES utf8mb4 COLLATE utf8mb4_unicode_ci,  @@SESSION.sql_mode = 'NO_ENGINE_SUBSTITUTION',  @@SESSION.character_set_connection = 'utf8mb4', @@SESSION.collation_connection = 'utf8mb4_unicode_ci', @@SESSION.group_concat_max_len = 1048576, @@SESSION.wait_timeout = 2147483;
# without wait_timeout
SET NAMES utf8mb4 COLLATE utf8mb4_unicode_ci,  @@SESSION.sql_mode = 'NO_ENGINE_SUBSTITUTION',  @@SESSION.character_set_connection = 'utf8mb4', @@SESSION.collation_connection = 'utf8mb4_unicode_ci', @@SESSION.group_concat_max_len = 1048576, @@SESSION.sql_auto_is_null = 0;

Digging further I found the following MySQL documentation re: the syntax for SET variables and SET NAMES statements:

According to this documentation, the SET NAMES ..., variable1 = x, variable2 = y syntax that is used by Rails is not correct.

To verify, I ran the SET variables statement on its own and added extra variables to rule out an issue with the length of the statement. This worked fine:

MySQL [core]> SET @@SESSION.sql_mode = 'NO_ENGINE_SUBSTITUTION',  @@SESSION.character_set_connection = 'utf8mb4', @@SESSION.collation_connection = 'utf8mb4_unicode_ci', @@SESSION.group_concat_max_len = 1048576, @@SESSION.interactive_timeout = 28800, @@SESSION.lock_wait_timeout = 31536000, @@SESSION.net_read_timeout = 30, @@SESSION.net_write_timeout = 60, @@SESSION.sql_auto_is_null = 0, @@SESSION.wait_timeout = 2147483;
Query OK, 0 rows affected (0.000 sec)

I suspected that the newer MariaDB client (v10.4.15) that is present in our Alpine container somehow could not process or send the query from Rails correctly. I adjusted the code in abstract_mysql_adapter.rb#configure_connection to use two separate calls instead of one.

Using that code, the error no longer happened when starting Rails.

@kamipo
Copy link
Member

kamipo commented Jan 26, 2021

Actually using SET NAMES in a program is common bad practice for MySQL, it does not change the client library's character set. It should be set by mysql_options C API, which is already done by mysql2 gem.

Can you just remove SET NAMES in configure_connection?

diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index e065721eef..defd66e63a 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -770,15 +770,6 @@ def configure_connection
           end
           sql_mode_assignment = "@@SESSION.sql_mode = #{sql_mode}, " if sql_mode
 
-          # NAMES does not have an equals sign, see
-          # https://dev.mysql.com/doc/refman/en/set-names.html
-          # (trailing comma because variable_assignments will always have content)
-          if @config[:encoding]
-            encoding = +"NAMES #{@config[:encoding]}"
-            encoding << " COLLATE #{@config[:collation]}" if @config[:collation]
-            encoding << ", "
-          end
-
           # Gather up all of the SET variables...
           variable_assignments = variables.map do |k, v|
             if defaults.include?(v)
@@ -789,8 +780,12 @@ def configure_connection
             # or else nil; compact to clear nils out
           end.compact.join(", ")
 
+          if @config[:collation]
+            variable_assignments << ", @@collation_connection = #{@config[:collation]}"
+          end
+
           # ...and send them all in one query
-          execute("SET #{encoding} #{sql_mode_assignment} #{variable_assignments}", "SCHEMA")
+          execute("SET #{sql_mode_assignment} #{variable_assignments}", "SCHEMA")
         end
 
         def column_definitions(table_name) # :nodoc:

See also #34347 (comment).

@robinroestenburg
Copy link
Contributor Author

@kamipo thanks!

I've added a commit that addresses your feedback. I am assigning the collation value (if present) to variables["collation_connection"], that way it will automatically be included into the variable_assignments string.

It should be set by mysql_options C API, which is already done by mysql2 gem.

For future reference, the character set is taken from the :encoding option here in the mysql2 gem: https://github.com/brianmario/mysql2/blob/master/lib/mysql2/client.rb#L47

@kamipo
Copy link
Member

kamipo commented Jan 26, 2021

Looks good to me. Can you squash commits into one?

https://guides.rubyonrails.org/contributing_to_ruby_on_rails.html#squashing-commits

Fixes malformed packet error that occurred with MariaDB client
connecting to RDS Aurora (MySQL 5.7) database.
@robinroestenburg
Copy link
Contributor Author

@kamipo done

@kamipo kamipo merged commit 8b3fc5c into rails:main Jan 26, 2021
kamipo added a commit that referenced this pull request Jan 26, 2021
Fix malformed packet error in MySQL statement for connection configuration
rafaelfranca added a commit that referenced this pull request Feb 16, 2021
@rafaelfranca
Copy link
Member

Looking this patch again I think it is not right. It is only setting collation_connection after the connection was already established and not setting character_set_client, character_set_results, character_set_connection. I'm also going to revert on main and I'll have to release a new 6.1 because this is breaking apps in production

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants