Merge pull request rails#47493 from olefriis/quote_binary_strings

Quote binary strings in Arel

Author: matthewd

activemodel/lib/active_model/type/immutable_string.rb

@@ -46,13 +46,20 @@ def type
       def serialize(value)
         case value
         when ::Numeric, ::Symbol, ActiveSupport::Duration then value.to_s
+        when ::String then serialize_cast_value(value)
         when true then @true
         when false then @false
         else super
         end
       end
 
       def serialize_cast_value(value) # :nodoc:
+        if value&.encoding == Encoding::BINARY
+          # If we can treat the bytes as UTF-8 without changing them, then use UTF-8 as encoding
+          new_value = value.dup.force_encoding(Encoding::UTF_8)
+          return new_value if new_value.valid_encoding?
+        end
+
         value
       end
 

activemodel/test/cases/type/immutable_string_test.rb

@@ -17,6 +17,26 @@ class ImmutableStringTest < ActiveModel::TestCase
         assert_same s, type.cast(s)
         assert_same s, type.deserialize(s)
       end
+
+      test "leaves validly encoded strings untouched" do
+        s = "string with àccénts".encode(Encoding::ISO_8859_1)
+        type = Type::ImmutableString.new
+        assert_same s, type.serialize(s)
+      end
+
+      test "serializes valid, binary-encoded strings to UTF-8" do
+        s = "string with àccénts".b
+        type = Type::ImmutableString.new
+        serialized = type.serialize(s)
+        assert_equal Encoding::UTF_8, serialized.encoding
+        assert_equal s.bytes, serialized.bytes
+      end
+
+      test "leaves true binary data untouched" do
+        binary_data = "\xEE\x49\xC7".b
+        type = Type::ImmutableString.new
+        assert_same binary_data, type.serialize(binary_data)
+      end
     end
   end
 end

activerecord/lib/active_record/connection_adapters/abstract/quoting.rb

@@ -16,7 +16,10 @@ def quote(value)
         when false      then quoted_false
         when nil        then "NULL"
         # BigDecimals need to be put in a non-normalized form and quoted.
-        when BigDecimal then value.to_s("F")
+        # Additionally, for Ruby 2.7, the string returned by `to_s` is ASCII-8BIT.
+        # We want to avoid that, as that will cause the string to be quoted as
+        # binary. It is safe to force the encoding to US-ASCII.
+        when BigDecimal then value.to_s("F").force_encoding(Encoding::US_ASCII)
         when Numeric then value.to_s
         when Type::Binary::Data then quoted_binary(value)
         when Type::Time::Value then "'#{quoted_time(value)}'"

activerecord/lib/active_record/connection_adapters/mysql/quoting.rb

@@ -6,14 +6,30 @@ module ActiveRecord
   module ConnectionAdapters
     module MySQL
       module Quoting # :nodoc:
+        def quote(value)
+          case value
+          when String
+            if value.encoding == Encoding::BINARY
+              quoted_binary(value)
+            else
+              "'#{quote_string(value.to_s)}'"
+            end
+          else
+            super
+          end
+        end
+
         def cast_bound_value(value)
           case value
           when Rational
             value.to_f.to_s
+          when BigDecimal
+            # For Ruby 2.7, the string returned by `to_s` is ASCII-8BIT.
+            # We want to avoid that, as that will cause the string to be quoted as
+            # binary. It is safe to force the encoding to US-ASCII.
+            value.to_s("F").force_encoding(Encoding::US_ASCII)
           when Numeric
             value.to_s
-          when BigDecimal
-            value.to_s("F")
           when true
             "1"
           when false
@@ -51,7 +67,11 @@ def quoted_date(value)
         end
 
         def quoted_binary(value)
-          "x'#{value.hex}'"
+          if value.is_a? String
+            "x'#{value.unpack1("H*")}'"
+          else
+            "x'#{value.hex}'"
+          end
         end
 
         def unquote_identifier(identifier)

activerecord/lib/active_record/connection_adapters/sqlite3/quoting.rb

@@ -4,6 +4,19 @@ module ActiveRecord
   module ConnectionAdapters
     module SQLite3
       module Quoting # :nodoc:
+        def quote(value)
+          case value
+          when String
+            if value.encoding == Encoding::BINARY
+              quoted_binary(value)
+            else
+              "'#{quote_string(value.to_s)}'"
+            end
+          else
+            super
+          end
+        end
+
         def quote_string(s)
           ::SQLite3::Database.quote(s)
         end
@@ -26,7 +39,11 @@ def quoted_time(value)
         end
 
         def quoted_binary(value)
-          "x'#{value.hex}'"
+          if value.is_a? String
+            "x'#{value.unpack1("H*")}'"
+          else
+            "x'#{value.hex}'"
+          end
         end
 
         def quoted_true
@@ -62,12 +79,6 @@ def type_cast(value) # :nodoc:
           case value
           when BigDecimal
             value.to_f
-          when String
-            if value.encoding == Encoding::ASCII_8BIT
-              super(value.encode(Encoding::UTF_8))
-            else
-              super
-            end
           else
             super
           end

activerecord/test/cases/binary_test.rb

@@ -37,4 +37,19 @@ def test_load_save
       assert_equal data, bin.reload.data, "Reloaded data differs from original"
     end
   end
+
+  unless current_adapter?(:PostgreSQLAdapter)
+    def test_does_not_cause_database_warnings
+      original_db_warnings_action = ActiveRecord.db_warnings_action
+      ActiveRecord.db_warnings_action = :raise
+
+      Binary.delete_all
+      binary_data = "\xEE\x49\xC7".b
+      Binary.connection.insert(Arel.sql("INSERT INTO binaries(data) VALUES(?)", binary_data))
+      binary = Binary.first
+      assert_equal binary_data, binary.data
+    ensure
+      ActiveRecord.db_warnings_action = original_db_warnings_action || :ignore
+    end
+  end
 end