Merge pull request rails#45618 from sambostock/active-job-big-decimal…

…-serialization

Fix `BigDecimal `(de)serialization for `JSON` adapters

activejob/CHANGELOG.md

@@ -1,3 +1,20 @@
+*   Fix BigDecimal (de)serialization for adapters using JSON.
+
+    Previously, BigDecimal was listed as not needing a serializer.  However,
+    when used with an adapter storing the job arguments as JSON, it would get
+    serialized as a simple String, resulting in deserialization also producing
+    a String (instead of a BigDecimal).
+
+    By using a serializer, we ensure the round trip is safe.
+
+    To ensure applications using BigDecimal job arguments are not subject to
+    race conditions during deployment (where a replica running a version of
+    Rails without BigDecimalSerializer fails to deserialize an argument
+    serialized with it), `ActiveJob.use_big_decimal_serializer` is disabled by
+    default, and can be set to true in a following deployment..
+
+    *Sam Bostock*
+
 *   Preserve full-precision `enqueued_at` timestamps for serialized jobs,
     allowing more accurate reporting of how long a job spent waiting in the
     queue before it was performed.

activejob/lib/active_job.rb

@@ -41,4 +41,12 @@ module ActiveJob
 
   autoload :TestCase
   autoload :TestHelper
+
+  ##
+  # :singleton-method:
+  # If false, Rails will preserve the legacy serialization of BigDecimal job arguments as Strings.
+  # If true, Rails will use the new BigDecimalSerializer to (de)serialize BigDecimal losslessly.
+  # This behavior will be removed in Rails 7.2.
+  singleton_class.attr_accessor :use_big_decimal_serializer
+  self.use_big_decimal_serializer = false
 end

activejob/lib/active_job/arguments.rb

@@ -47,7 +47,7 @@ def deserialize(arguments)
 
     private
       # :nodoc:
-      PERMITTED_TYPES = [ NilClass, String, Integer, Float, BigDecimal, TrueClass, FalseClass ]
+      PERMITTED_TYPES = [ NilClass, String, Integer, Float, TrueClass, FalseClass ]
       # :nodoc:
       GLOBALID_KEY = "_aj_globalid"
       # :nodoc:
@@ -93,6 +93,17 @@ def serialize_argument(argument)
         when -> (arg) { arg.respond_to?(:permitted?) }
           serialize_indifferent_hash(argument.to_h)
         else
+          if BigDecimal === argument && !ActiveJob.use_big_decimal_serializer
+            ActiveSupport::Deprecation.warn(<<~MSG)
+              Primitive serialization of BigDecimal job arguments is deprecated as it may serialize via .to_s using certain queue adapters.
+              Enable config.active_job.use_big_decimal_serializer to use BigDecimalSerializer instead, which will be mandatory in Rails 7.2.
+
+              Note that if you application has multiple replicas, you should only enable this setting after successfully deploying your app to Rails 7.1 first.
+              This will ensure that during your deployment all replicas are capable of deserializing arguments serialized with BigDecimalSerializer.
+            MSG
+            return argument
+          end
+
           Serializers.serialize(argument)
         end
       end
@@ -103,6 +114,8 @@ def deserialize_argument(argument)
           argument
         when *PERMITTED_TYPES
           argument
+        when BigDecimal # BigDecimal may have been legacy serialized; Remove in 7.2
+          argument
         when Array
           argument.map { |arg| deserialize_argument(arg) }
         when Hash

activejob/lib/active_job/serializers.rb

@@ -18,6 +18,7 @@ module Serializers # :nodoc:
     autoload :TimeSerializer
     autoload :ModuleSerializer
     autoload :RangeSerializer
+    autoload :BigDecimalSerializer
 
     mattr_accessor :_additional_serializers
     self._additional_serializers = Set.new
@@ -63,6 +64,7 @@ def add_serializers(*new_serializers)
       TimeWithZoneSerializer,
       TimeSerializer,
       ModuleSerializer,
-      RangeSerializer
+      RangeSerializer,
+      BigDecimalSerializer
   end
 end

activejob/lib/active_job/serializers/big_decimal_serializer.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+require "bigdecimal"
+
+module ActiveJob
+  module Serializers
+    class BigDecimalSerializer < ObjectSerializer # :nodoc:
+      def serialize(big_decimal)
+        super("value" => big_decimal.to_s)
+      end
+
+      def deserialize(hash)
+        BigDecimal(hash["value"])
+      end
+
+      private
+        def klass
+          BigDecimal
+        end
+    end
+  end
+end

activejob/test/cases/argument_serialization_test.rb

@@ -1,10 +1,12 @@
 # frozen_string_literal: true
 
+require "bigdecimal"
 require "helper"
 require "active_job/arguments"
 require "models/person"
 require "active_support/core_ext/hash/indifferent_access"
 require "jobs/kwargs_job"
+require "jobs/arguments_round_trip_job"
 require "support/stubs/strong_parameters"
 
 class ArgumentSerializationTest < ActiveSupport::TestCase
@@ -19,7 +21,7 @@ class ClassArgument; end
   end
 
   [ nil, 1, 1.0, 1_000_000_000_000_000_000_000,
-    "a", true, false, BigDecimal(5),
+    "a", true, false,
     :a,
     1.day,
     Date.new(2001, 2, 3),
@@ -50,6 +52,28 @@ class ClassArgument; end
     end
   end
 
+  test "dangerously treats BigDecimal arguments as primitives not requiring serialization by default" do
+    assert_deprecated(<<~MSG.chomp) do
+      Primitive serialization of BigDecimal job arguments is deprecated as it may serialize via .to_s using certain queue adapters.
+      Enable config.active_job.use_big_decimal_serializer to use BigDecimalSerializer instead, which will be mandatory in Rails 7.2.
+
+      Note that if you application has multiple replicas, you should only enable this setting after successfully deploying your app to Rails 7.1 first.
+      This will ensure that during your deployment all replicas are capable of deserializing arguments serialized with BigDecimalSerializer.
+    MSG
+      assert_equal(
+        BigDecimal(5),
+        *ActiveJob::Arguments.deserialize(ActiveJob::Arguments.serialize([BigDecimal(5)])),
+      )
+    end
+  end
+
+  test "safely serializes BigDecimal arguments if configured to use_big_decimal_serializer" do
+    # BigDecimal(5) example should be moved back up into array above in Rails 7.2
+    with_big_decimal_serializer do
+      assert_arguments_unchanged BigDecimal(5)
+    end
+  end
+
   [ Object.new, Person.find("5").to_gid, Class.new ].each do |arg|
     test "does not serialize #{arg.class}" do
       assert_raises ActiveJob::SerializationError do
@@ -208,6 +232,16 @@ def assert_arguments_roundtrip(args)
     end
 
     def perform_round_trip(args)
-      ActiveJob::Arguments.deserialize(ActiveJob::Arguments.serialize(args))
+      ArgumentsRoundTripJob.perform_later(*args) # Actually performed inline
+
+      JobBuffer.last_value
+    end
+
+    def with_big_decimal_serializer(temporary = true)
+      original = ActiveJob.use_big_decimal_serializer
+      ActiveJob.use_big_decimal_serializer = temporary
+      yield
+    ensure
+      ActiveJob.use_big_decimal_serializer = original
     end
 end

activejob/test/jobs/arguments_round_trip_job.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class ArgumentsRoundTripJob < ActiveJob::Base
+  def perform(*arguments)
+    JobBuffer.add(arguments)
+  end
+end

guides/source/configuring.md

@@ -68,6 +68,7 @@ Below are the default values associated with each target version. In cases of co
 - [`config.log_file_size`](#config-log-file-size): `100.megabytes`
 - [`config.active_record.sqlite3_adapter_strict_strings_by_default`](#config-active-record-sqlite3-adapter-strict-strings-by-default): `false`
 - [`config.active_record.allow_deprecated_singular_associations_name`](#config-active-record-allow-deprecated-singular-associations-name): `false`
+- [`config.active_job.use_big_decimal_serializer`](#config-active-job-use-big-decimal-serializer): `false`
 
 
 #### Default Values for Target Version 7.0
@@ -2254,6 +2255,17 @@ The default value depends on the `config.load_defaults` target version:
 Determines whether job context for query tags will be automatically updated via
 an `around_perform`. The default value is `true`.
 
+#### `config.active_job.use_big_decimal_serializer`
+
+Enable the use of BigDecimalSerializer instead of legacy BigDecimal argument
+serialization, which may result in the argument being lossfully converted to
+a String when using certain queue adapters.
+This setting is disabled by default to allow race condition free deployment
+of applications with multiple replicas, in which an old replica would not
+support BigDecimalSerializer..
+In such environments, it should be safe to enable this setting following
+successful deployment of Rails 7.1 which introduces BigDecimalSerializer.
+
 ### Configuring Action Cable
 
 #### `config.action_cable.url`

railties/lib/rails/application/configuration.rb

@@ -265,6 +265,7 @@ def load_defaults(target_version)
           if respond_to?(:active_record)
             active_record.run_commit_callbacks_on_first_saved_instances_in_transaction = false
             active_record.allow_deprecated_singular_associations_name = false
+            active_record.sqlite3_adapter_strict_strings_by_default = true
           end
 
           if respond_to?(:action_dispatch)
@@ -277,6 +278,10 @@ def load_defaults(target_version)
             }
           end
 
+          if respond_to?(:active_job)
+            active_job.use_big_decimal_serializer = false
+          end
+
           if respond_to?(:active_support)
             active_support.default_message_encryptor_serializer = :json
             active_support.default_message_verifier_serializer = :json
@@ -285,10 +290,6 @@ def load_defaults(target_version)
           if respond_to?(:action_controller)
             action_controller.allow_deprecated_parameters_hash_equality = false
           end
-
-          if respond_to?(:active_record)
-            active_record.sqlite3_adapter_strict_strings_by_default = true
-          end
         else
           raise "Unknown version #{target_version.to_s.inspect}"
         end

railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_1.rb.tt

@@ -47,3 +47,13 @@
 
 # Disable deprecated singular associations names
 # Rails.application.config.active_record.allow_deprecated_singular_associations_name = false
+
+# Enable the use of BigDecimalSerializer instead of legacy BigDecimal argument
+# serialization, which may result in the argument being lossfully converted to
+# a String when using certain queue adapters.
+# This setting is disabled by default to allow race condition free deployment
+# of applications with multiple replicas, in which an old replica would not
+# support BigDecimalSerializer..
+# In such environments, it should be safe to enable this setting following
+# successful deployment of Rails 7.1 which introduces BigDecimalSerializer.
+# Rails.application.config.active_job.use_big_decimal_serializer = true