Skip to content

Commit 929a5d0

Browse files
committed
Restrict serializable_hash to accepted options, only for tests
1 parent aad7779 commit 929a5d0

File tree

7 files changed

+57
-3
lines changed

7 files changed

+57
-3
lines changed

lib/active_model/serializer.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
# reified when subclassed to decorate a resource.
1919
module ActiveModel
2020
class Serializer
21+
# @see #serializable_hash for more details on these valid keys.
22+
SERIALIZABLE_HASH_VALID_KEYS = [:only, :except, :methods, :include, :root].freeze
2123
extend ActiveSupport::Autoload
2224
autoload :Adapter
2325
autoload :Null

lib/active_model_serializers/adapter/attributes.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ def initialize(serializer, options = {})
88
end
99

1010
def serializable_hash(options = nil)
11-
options ||= {}
11+
options = serialization_options(options)
1212

1313
if serializer.respond_to?(:each)
1414
serializable_hash_for_collection(options)

lib/active_model_serializers/adapter/base.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ def cached_name
1919
@cached_name ||= self.class.name.demodulize.underscore
2020
end
2121

22+
# Subclasses that implement this method must first call
23+
# options = serialization_options(options)
2224
def serializable_hash(_options = nil)
2325
fail NotImplementedError, 'This is an abstract method. Should be implemented at the concrete adapter.'
2426
end
@@ -41,6 +43,12 @@ def cache_check(serializer)
4143

4244
private
4345

46+
# see https://github.com/rails-api/active_model_serializers/pull/965
47+
# When <tt>options</tt> is +nil+, sets it to +{}+
48+
def serialization_options(options)
49+
options ||= {} # rubocop:disable Lint/UselessAssignment
50+
end
51+
4452
def meta
4553
instance_options.fetch(:meta, nil)
4654
end

lib/active_model_serializers/adapter/json.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ module ActiveModelSerializers
22
module Adapter
33
class Json < Base
44
def serializable_hash(options = nil)
5-
options ||= {}
5+
options = serialization_options(options)
66
serialized_hash = { root => Attributes.new(serializer, instance_options).serializable_hash(options) }
77
self.class.transform_key_casing!(serialized_hash, instance_options)
88
end

test/action_controller/json_api/transform_test.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,11 @@ def render_resource_with_transform_nil
6060
end
6161

6262
def render_resource_with_transform_with_global_config
63-
setup_post
6463
old_transform = ActiveModelSerializers.config.key_transform
64+
setup_post
6565
ActiveModelSerializers.config.key_transform = :camel_lower
6666
render json: @post, serializer: PostSerializer, adapter: :json_api
67+
ensure
6768
ActiveModelSerializers.config.key_transform = old_transform
6869
end
6970
end

test/adapter_test.rb

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,33 @@ def test_serializable_hash_is_abstract_method
1414
end
1515
end
1616

17+
def test_serialization_options_ensures_option_is_a_hash
18+
adapter = Class.new(ActiveModelSerializers::Adapter::Base) do
19+
def serializable_hash(options = nil)
20+
serialization_options(options)
21+
end
22+
end.new(@serializer)
23+
assert_equal({}, adapter.serializable_hash(nil))
24+
assert_equal({}, adapter.serializable_hash({}))
25+
ensure
26+
ActiveModelSerializers::Adapter.adapter_map.delete_if { |k, _| k =~ /class/ }
27+
end
28+
29+
def test_serialization_options_ensures_option_is_one_of_valid_options
30+
adapter = Class.new(ActiveModelSerializers::Adapter::Base) do
31+
def serializable_hash(options = nil)
32+
serialization_options(options)
33+
end
34+
end.new(@serializer)
35+
filtered_options = { now: :see_me, then: :not }
36+
valid_options = ActiveModel::Serializer::SERIALIZABLE_HASH_VALID_KEYS.each_with_object({}) do |option, result|
37+
result[option] = option
38+
end
39+
assert_equal(valid_options, adapter.serializable_hash(filtered_options.merge(valid_options)))
40+
ensure
41+
ActiveModelSerializers::Adapter.adapter_map.delete_if { |k, _| k =~ /class/ }
42+
end
43+
1744
def test_serializer
1845
assert_equal @serializer, @adapter.serializer
1946
end

test/test_helper.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,22 @@
1515
require 'action_controller/test_case'
1616
require 'action_controller/railtie'
1717
require 'active_model_serializers'
18+
# For now, we only restrict the options to serializable_hash/as_json/to_json
19+
# in tests, to ensure developers don't add any unsupported options.
20+
# There's no known benefit, at this time, to having the filtering run in
21+
# production when the excluded options would simply not be used.
22+
#
23+
# However, for documentation purposes, the constant
24+
# ActiveModel::Serializer::SERIALIZABLE_HASH_VALID_KEYS is defined
25+
# in the Serializer.
26+
ActiveModelSerializers::Adapter::Base.class_eval do
27+
alias_method :original_serialization_options, :serialization_options
28+
29+
def serialization_options(options)
30+
original_serialization_options(options)
31+
.slice(*ActiveModel::Serializer::SERIALIZABLE_HASH_VALID_KEYS)
32+
end
33+
end
1834
require 'fileutils'
1935
FileUtils.mkdir_p(File.expand_path('../../tmp/cache', __FILE__))
2036

0 commit comments

Comments
 (0)