Merge pull request brianmario#482 from brianmario/error-encoding

Ensure error messages are always valid UTF-8

Author: brianmario

.travis.yml

@@ -15,3 +15,6 @@ script:
 before_install:
   - gem update --system 2.1.11
   - gem --version
+matrix:
+  allow_failures:
+    - rvm: rbx

ext/mysql2/client.c

@@ -12,7 +12,7 @@
 VALUE cMysql2Client;
 extern VALUE mMysql2, cMysql2Error;
 static VALUE sym_id, sym_version, sym_async, sym_symbolize_keys, sym_as, sym_array, sym_stream;
-static ID intern_merge, intern_merge_bang, intern_error_number_eql, intern_sql_state_eql;
+static ID intern_merge, intern_merge_bang, intern_error_number_eql, intern_sql_state_eql, intern_server_version;
 
 #ifndef HAVE_RB_HASH_DUP
 static VALUE rb_hash_dup(VALUE other) {
@@ -125,26 +125,13 @@ static VALUE rb_raise_mysql2_error(mysql_client_wrapper *wrapper) {
   VALUE rb_error_msg = rb_str_new2(mysql_error(wrapper->client));
   VALUE rb_sql_state = rb_tainted_str_new2(mysql_sqlstate(wrapper->client));
   VALUE e;
+
 #ifdef HAVE_RUBY_ENCODING_H
-  if (wrapper->server_version < 50500) {
-    /* MySQL < 5.5 uses mixed encoding, just call it binary. */
-    int err_enc = rb_ascii8bit_encindex();
-    rb_enc_associate_index(rb_error_msg, err_enc);
-    rb_enc_associate_index(rb_sql_state, err_enc);
-  } else {
-    /* MySQL >= 5.5 uses UTF-8 errors internally and converts them to the connection encoding. */
-    rb_encoding *default_internal_enc = rb_default_internal_encoding();
-    rb_encoding *conn_enc = rb_to_encoding(wrapper->encoding);
-    rb_enc_associate(rb_error_msg, conn_enc);
-    rb_enc_associate(rb_sql_state, conn_enc);
-    if (default_internal_enc) {
-      rb_error_msg = rb_str_export_to_enc(rb_error_msg, default_internal_enc);
-      rb_sql_state = rb_str_export_to_enc(rb_sql_state, default_internal_enc);
-    }
-  }
+  rb_enc_associate(rb_error_msg, rb_utf8_encoding());
+  rb_enc_associate(rb_sql_state, rb_usascii_encoding());
 #endif
 
-  e = rb_exc_new3(cMysql2Error, rb_error_msg);
+  e = rb_funcall(cMysql2Error, rb_intern("new"), 2, rb_error_msg, LONG2FIX(wrapper->server_version));
   rb_funcall(e, intern_error_number_eql, 1, UINT2NUM(mysql_errno(wrapper->client)));
   rb_funcall(e, intern_sql_state_eql, 1, rb_sql_state);
   rb_exc_raise(e);
@@ -1221,6 +1208,7 @@ void init_mysql2_client() {
   intern_merge_bang = rb_intern("merge!");
   intern_error_number_eql = rb_intern("error_number=");
   intern_sql_state_eql = rb_intern("sql_state=");
+  intern_server_version = rb_intern("server_version=");
 
 #ifdef CLIENT_LONG_PASSWORD
   rb_const_set(cMysql2Client, rb_intern("LONG_PASSWORD"),

lib/mysql2/error.rb

@@ -1,15 +1,81 @@
+# encoding: UTF-8
+
 module Mysql2
   class Error < StandardError
-    attr_accessor :error_number, :sql_state
+    REPLACEMENT_CHAR = '?'
+    ENCODE_OPTS      = {:undef => :replace, :invalid => :replace, :replace => REPLACEMENT_CHAR}
 
-    def initialize msg
-      super
-      @error_number = nil
-      @sql_state    = nil
-    end
+    attr_accessor :error_number, :sql_state
+    attr_writer   :server_version
 
     # Mysql gem compatibility
     alias_method :errno, :error_number
     alias_method :error, :message
+
+    def initialize(msg, server_version=nil)
+      self.server_version = server_version
+
+      super(clean_message(msg))
+    end
+
+    if "".respond_to? :encode
+      def sql_state=(state)
+        @sql_state = state.encode(ENCODE_OPTS)
+      end
+    end
+
+    private
+
+    # In MySQL 5.5+ error messages are always constructed server-side as UTF-8
+    # then returned in the encoding set by the `character_set_results` system
+    # variable.
+    #
+    # See http://dev.mysql.com/doc/refman/5.5/en/charset-errors.html for
+    # more contetx.
+    #
+    # Before MySQL 5.5 error message template strings are in whatever encoding
+    # is associated with the error message language.
+    # See http://dev.mysql.com/doc/refman/5.1/en/error-message-language.html
+    # for more information.
+    #
+    # The issue is that the user-data inserted in the message could potentially
+    # be in any encoding MySQL supports and is insert into the latin1, euckr or
+    # koi8r string raw. Meaning there's a high probability the string will be
+    # corrupt encoding-wise.
+    #
+    # See http://dev.mysql.com/doc/refman/5.1/en/charset-errors.html for
+    # more information.
+    #
+    # So in an attempt to make sure the error message string is always in a valid
+    # encoding, we'll assume UTF-8 and clean the string of anything that's not a
+    # valid UTF-8 character.
+    #
+    # Except for if we're on 1.8, where we'll do nothing ;)
+    #
+    # Returns a valid UTF-8 string in Ruby 1.9+, the original string on Ruby 1.8
+    def clean_message(message)
+      return message if !message.respond_to?(:encoding)
+
+      if @server_version && @server_version > 50500
+        message.encode(ENCODE_OPTS)
+      else
+        if message.respond_to? :scrub
+          message.scrub(REPLACEMENT_CHAR).encode(ENCODE_OPTS)
+        else
+          # This is ugly as hell but Ruby 1.9 doesn't provide a way to clean a string
+          # and retain it's valid UTF-8 characters, that I know of.
+
+          new_message = "".force_encoding(Encoding::UTF_8)
+          message.chars.each do |char|
+            if char.valid_encoding?
+              new_message << char
+            else
+              new_message << REPLACEMENT_CHAR
+            end
+          end
+          new_message.encode(ENCODE_OPTS)
+        end
+      end
+    end
   end
 end

spec/mysql2/client_spec.rb

@@ -7,13 +7,14 @@
 
     it "should not raise an exception for valid defaults group" do
       lambda {
-        @client = Mysql2::Client.new(:default_file => cnf_file, :default_group => "test")
+        opts = DatabaseCredentials['root'].merge(:default_file => cnf_file, :default_group => "test")
+        @client = Mysql2::Client.new(opts)
       }.should_not raise_error(Mysql2::Error)
     end
 
     it "should not raise an exception without default group" do
       lambda {
-        @client = Mysql2::Client.new(:default_file => cnf_file)
+        @client = Mysql2::Client.new(DatabaseCredentials['root'].merge(:default_file => cnf_file))
       }.should_not raise_error(Mysql2::Error)
     end
   end
@@ -655,18 +656,22 @@ def connect *args
   if defined? Encoding
     context "strings returned by #info" do
       it "should default to the connection's encoding if Encoding.default_internal is nil" do
-        Encoding.default_internal = nil
-        @client.info[:version].encoding.should eql(Encoding.find('utf-8'))
+        with_internal_encoding nil do
+          @client.info[:version].encoding.should eql(Encoding.find('utf-8'))
 
-        client2 = Mysql2::Client.new(DatabaseCredentials['root'].merge(:encoding => 'ascii'))
-        client2.info[:version].encoding.should eql(Encoding.find('us-ascii'))
+          client2 = Mysql2::Client.new(DatabaseCredentials['root'].merge(:encoding => 'ascii'))
+          client2.info[:version].encoding.should eql(Encoding.find('us-ascii'))
+        end
       end
 
       it "should use Encoding.default_internal" do
-        Encoding.default_internal = Encoding.find('utf-8')
-        @client.info[:version].encoding.should eql(Encoding.default_internal)
-        Encoding.default_internal = Encoding.find('us-ascii')
-        @client.info[:version].encoding.should eql(Encoding.default_internal)
+        with_internal_encoding 'utf-8' do
+          @client.info[:version].encoding.should eql(Encoding.default_internal)
+        end
+
+        with_internal_encoding 'us-ascii' do
+          @client.info[:version].encoding.should eql(Encoding.default_internal)
+        end
       end
     end
   end
@@ -694,18 +699,22 @@ def connect *args
   if defined? Encoding
     context "strings returned by #server_info" do
       it "should default to the connection's encoding if Encoding.default_internal is nil" do
-        Encoding.default_internal = nil
-        @client.server_info[:version].encoding.should eql(Encoding.find('utf-8'))
+        with_internal_encoding nil do
+          @client.server_info[:version].encoding.should eql(Encoding.find('utf-8'))
 
-        client2 = Mysql2::Client.new(DatabaseCredentials['root'].merge(:encoding => 'ascii'))
-        client2.server_info[:version].encoding.should eql(Encoding.find('us-ascii'))
+          client2 = Mysql2::Client.new(DatabaseCredentials['root'].merge(:encoding => 'ascii'))
+          client2.server_info[:version].encoding.should eql(Encoding.find('us-ascii'))
+        end
       end
 
       it "should use Encoding.default_internal" do
-        Encoding.default_internal = Encoding.find('utf-8')
-        @client.server_info[:version].encoding.should eql(Encoding.default_internal)
-        Encoding.default_internal = Encoding.find('us-ascii')
-        @client.server_info[:version].encoding.should eql(Encoding.default_internal)
+        with_internal_encoding 'utf-8' do
+          @client.server_info[:version].encoding.should eql(Encoding.default_internal)
+        end
+
+        with_internal_encoding 'us-ascii' do
+          @client.server_info[:version].encoding.should eql(Encoding.default_internal)
+        end
       end
     end
   end

spec/mysql2/error_spec.rb

@@ -1,86 +1,83 @@
 # encoding: UTF-8
-require 'spec_helper'
-
-# The matrix of error encoding tests:
-# ('Enc = X' means 'Encoding.default_internal = X')
-#                  MySQL < 5.5   MySQL >= 5.5
-# Ruby 1.8         N/A           N/A
-# Ruby 1.9+
-#  Enc = nil
-#  :enc = nil      BINARY        UTF-8
-#
-#  Enc = XYZ
-#  :enc = XYZ      BINARY        XYZ
-#
-#  Enc = FOO
-#  :enc = BAR      BINARY        FOO
-#
 
+require 'spec_helper'
 
 describe Mysql2::Error do
-  shared_examples "mysql2 error" do
+  let(:client) { Mysql2::Client.new(DatabaseCredentials['root']) }
+
+  let :error do
     begin
-      err_client = Mysql2::Client.new(DatabaseCredentials['root'])
-      err_client.query("HAHAHA")
+      client.query("HAHAHA")
     rescue Mysql2::Error => e
       error = e
     ensure
-      err_client.close
+      client.close
     end
 
-    subject { error }
-    it { should respond_to(:error_number) }
-    it { should respond_to(:sql_state) }
+    error
+  end
+
+  it "responds to error_number and sql_state, with aliases" do
+    error.should respond_to(:error_number)
+    error.should respond_to(:sql_state)
 
     # Mysql gem compatibility
-    it { should respond_to(:errno) }
-    it { should respond_to(:error) }
+    error.should respond_to(:errno)
+    error.should respond_to(:error)
   end
 
-  shared_examples "mysql2 error encoding" do |db_enc, def_enc, err_enc|
-    Encoding.default_internal = def_enc
+  if "".respond_to? :encoding
+    let :error do
+      client = Mysql2::Client.new(DatabaseCredentials['root'])
+      begin
+        client.query("\xE9\x80\xA0\xE5\xAD\x97")
+      rescue Mysql2::Error => e
+        error = e
+      ensure
+        client.close
+      end
 
-    begin
-      err_client = Mysql2::Client.new(DatabaseCredentials['root'].merge(:encoding => db_enc))
-      err_client.query("造字")
-    rescue Mysql2::Error => e
-      error = e
-    ensure
-      err_client.close
+      error
     end
 
-    subject { error.message.encoding }
-    it "#message   should transcode from #{db_enc.inspect} to #{err_enc}" do should eql(err_enc) end
+    let :bad_err do
+      client = Mysql2::Client.new(DatabaseCredentials['root'])
+      begin
+        client.query("\xE5\xC6\x7D\x1F")
+      rescue Mysql2::Error => e
+        error = e
+      ensure
+        client.close
+      end
 
-    subject { error.error.encoding }
-    it "#error     should transcode from #{db_enc.inspect} to #{err_enc}" do should eql(err_enc) end
+      error
+    end
 
-    subject { error.sql_state.encoding }
-    it "#sql_state should transcode from #{db_enc.inspect} to #{err_enc}" do should eql(err_enc) end
-  end
+    it "returns error messages as UTF-8 by default" do
+      with_internal_encoding nil do
+        error.message.encoding.should eql(Encoding::UTF_8)
+        error.message.valid_encoding?
 
-  shared_examples "mysql2 error encoding (MySQL < 5.5)" do |db_enc, def_enc, err_enc|
-    include_examples "mysql2 error encoding", db_enc, def_enc, err_enc
-  end
+        bad_err.message.encoding.should eql(Encoding::UTF_8)
+        bad_err.message.valid_encoding?
 
-  shared_examples "mysql2 error encoding (MySQL >= 5.5)" do |db_enc, def_enc, err_enc|
-    include_examples "mysql2 error encoding", db_enc, def_enc, err_enc
-  end
+        bad_err.message.should include("??}\u001F")
+      end
+    end
+
+    it "returns sql state as ASCII" do
+      error.sql_state.encoding.should eql(Encoding::US_ASCII)
+      error.sql_state.valid_encoding?
+    end
+
+    it "returns error messages and sql state in Encoding.default_internal if set" do
+      with_internal_encoding 'UTF-16LE' do
+        error.message.encoding.should eql(Encoding.default_internal)
+        error.message.valid_encoding?
 
-  it_behaves_like "mysql2 error"
-
-  unless RUBY_VERSION =~ /1.8/
-    mysql_ver = Mysql2::Client.new(DatabaseCredentials['root']).server_info[:id]
-    if mysql_ver < 50505
-      it_behaves_like "mysql2 error encoding (MySQL < 5.5)", nil, nil, Encoding::ASCII_8BIT
-      it_behaves_like "mysql2 error encoding (MySQL < 5.5)", 'utf8', Encoding::UTF_8, Encoding::ASCII_8BIT
-      it_behaves_like "mysql2 error encoding (MySQL < 5.5)", 'big5', Encoding::Big5, Encoding::ASCII_8BIT
-      it_behaves_like "mysql2 error encoding (MySQL < 5.5)", 'big5', Encoding::US_ASCII, Encoding::ASCII_8BIT
-    else
-      it_behaves_like "mysql2 error encoding (MySQL >= 5.5)", nil, nil, Encoding::UTF_8
-      it_behaves_like "mysql2 error encoding (MySQL >= 5.5)", 'utf8', Encoding::UTF_8, Encoding::UTF_8
-      it_behaves_like "mysql2 error encoding (MySQL >= 5.5)", 'big5', Encoding::Big5, Encoding::Big5
-      it_behaves_like "mysql2 error encoding (MySQL >= 5.5)", 'big5', Encoding::US_ASCII, Encoding::US_ASCII
+        bad_err.message.encoding.should eql(Encoding.default_internal)
+        bad_err.message.valid_encoding?
+      end
     end
   end
 end