From 6a781c8ff2f04e513bce88d96986a55e2ea15cf4 Mon Sep 17 00:00:00 2001 From: Olle Jonsson Date: Sun, 11 Aug 2024 06:48:30 +0200 Subject: [PATCH 1/2] Serializer: for Marshal, catch all .load errors In order to avoid missing an error message to filter out, treat any Marshal.load error as a failed serialization, and trust Ruby's e.cause system to provide a lineage of the error's true beginning. --- lib/dalli/protocol/value_serializer.rb | 17 +++++++++++++++-- test/protocol/test_value_serializer.rb | 1 + 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/dalli/protocol/value_serializer.rb b/lib/dalli/protocol/value_serializer.rb index f1e3cd87..5136c748 100644 --- a/lib/dalli/protocol/value_serializer.rb +++ b/lib/dalli/protocol/value_serializer.rb @@ -42,9 +42,22 @@ def store(value, req_options, bitflags) NAME_ERR_STR = 'uninitialized constant' # rubocop:enable Layout/LineLength - def retrieve(value, bitflags) + def retrieve(value, bitflags) # rubocop:disable Metrics/MethodLength serialized = (bitflags & FLAG_SERIALIZED) != 0 - serialized ? serializer.load(value) : value + if serialized + if serializer.is_a?(Marshal) + begin + serializer.load(value) + rescue StandardError => e + raise UnmarshalError, "Unable to unmarshal value: #{e.message}" + end + else + # Use Dalli's existing exception filtering for deserialization when not using Marshal to serialize. + serializer.load(value) + end + else + value + end rescue TypeError => e filter_type_error(e) rescue ArgumentError => e diff --git a/test/protocol/test_value_serializer.rb b/test/protocol/test_value_serializer.rb index ef73ccc0..8b7d002d 100644 --- a/test/protocol/test_value_serializer.rb +++ b/test/protocol/test_value_serializer.rb @@ -158,6 +158,7 @@ describe 'when the bitflags specify serialization' do it 'should deserialize the value' do + serializer.expect :is_a?, true, [Marshal] serializer.expect :load, deserialized_dummy, [raw_value] bitflags = rand(32) bitflags |= 0x1 From d19285cfb4cd830aa5509f4fffbbfa1c83049183 Mon Sep 17 00:00:00 2001 From: Olle Jonsson Date: Sun, 11 Aug 2024 07:15:34 +0200 Subject: [PATCH 2/2] Treat all deserialization errors as UnmarshalError --- lib/dalli/protocol/value_serializer.rb | 46 ++---------------- test/protocol/test_value_serializer.rb | 65 ++++---------------------- 2 files changed, 12 insertions(+), 99 deletions(-) diff --git a/lib/dalli/protocol/value_serializer.rb b/lib/dalli/protocol/value_serializer.rb index 5136c748..a9bbb224 100644 --- a/lib/dalli/protocol/value_serializer.rb +++ b/lib/dalli/protocol/value_serializer.rb @@ -33,55 +33,17 @@ def store(value, req_options, bitflags) [store_value, bitflags] end - # TODO: Some of these error messages need to be validated. It's not obvious - # that all of them are actually generated by the invoked code - # in current systems - # rubocop:disable Layout/LineLength - TYPE_ERR_REGEXP = %r{needs to have method `_load'|exception class/object expected|instance of IO needed|incompatible marshal file format}.freeze - ARGUMENT_ERR_REGEXP = /undefined class|marshal data too short/.freeze - NAME_ERR_STR = 'uninitialized constant' - # rubocop:enable Layout/LineLength - - def retrieve(value, bitflags) # rubocop:disable Metrics/MethodLength + def retrieve(value, bitflags) serialized = (bitflags & FLAG_SERIALIZED) != 0 if serialized - if serializer.is_a?(Marshal) - begin - serializer.load(value) - rescue StandardError => e - raise UnmarshalError, "Unable to unmarshal value: #{e.message}" - end - else - # Use Dalli's existing exception filtering for deserialization when not using Marshal to serialize. + begin serializer.load(value) + rescue StandardError + raise UnmarshalError, 'Unable to unmarshal value' end else value end - rescue TypeError => e - filter_type_error(e) - rescue ArgumentError => e - filter_argument_error(e) - rescue NameError => e - filter_name_error(e) - end - - def filter_type_error(err) - raise err unless TYPE_ERR_REGEXP.match?(err.message) - - raise UnmarshalError, "Unable to unmarshal value: #{err.message}" - end - - def filter_argument_error(err) - raise err unless ARGUMENT_ERR_REGEXP.match?(err.message) - - raise UnmarshalError, "Unable to unmarshal value: #{err.message}" - end - - def filter_name_error(err) - raise err unless err.message.include?(NAME_ERR_STR) - - raise UnmarshalError, "Unable to unmarshal value: #{err.message}" end def serializer diff --git a/test/protocol/test_value_serializer.rb b/test/protocol/test_value_serializer.rb index 8b7d002d..3518bdd9 100644 --- a/test/protocol/test_value_serializer.rb +++ b/test/protocol/test_value_serializer.rb @@ -158,7 +158,6 @@ describe 'when the bitflags specify serialization' do it 'should deserialize the value' do - serializer.expect :is_a?, true, [Marshal] serializer.expect :load, deserialized_dummy, [raw_value] bitflags = rand(32) bitflags |= 0x1 @@ -182,7 +181,7 @@ end end - assert_equal exception.message, "Unable to unmarshal value: #{error_message}" + assert_equal exception.cause.message, error_message end end @@ -199,7 +198,7 @@ end end - assert_equal exception.message, "Unable to unmarshal value: #{error_message}" + assert_equal exception.cause.message, error_message end end @@ -213,7 +212,7 @@ vs.retrieve(raw_value, Dalli::Protocol::ValueSerializer::FLAG_SERIALIZED) end - assert_equal exception.message, "Unable to unmarshal value: #{error_message}" + assert_equal exception.cause.message, error_message end end @@ -227,23 +226,7 @@ vs.retrieve(raw_value, Dalli::Protocol::ValueSerializer::FLAG_SERIALIZED) end - assert exception.message.start_with?("Unable to unmarshal value: #{error_message}") - end - end - - describe 'when deserialization raises a TypeError with a non-matching message' do - let(:error_message) { SecureRandom.hex(10) } - let(:serializer) { Marshal } - - it 're-raises TypeError' do - error = ->(_arg) { raise TypeError, error_message } - exception = serializer.stub :load, error do - assert_raises TypeError do - vs.retrieve(raw_value, Dalli::Protocol::ValueSerializer::FLAG_SERIALIZED) - end - end - - assert_equal error_message, exception.message + assert exception.cause.message.start_with?(error_message) end end @@ -260,23 +243,7 @@ end end - assert exception.message.start_with?("Unable to unmarshal value: #{error_message}") - end - end - - describe 'when deserialization raises a NameError with a non-matching message' do - let(:error_message) { SecureRandom.hex(10) } - let(:serializer) { Marshal } - - it 're-raises NameError' do - error = ->(_arg) { raise NameError, error_message } - exception = serializer.stub :load, error do - assert_raises NameError do - vs.retrieve(raw_value, Dalli::Protocol::ValueSerializer::FLAG_SERIALIZED) - end - end - - assert exception.message.start_with?(error_message) + assert exception.cause.message.start_with?(error_message) end end @@ -290,7 +257,7 @@ vs.retrieve(raw_value, Dalli::Protocol::ValueSerializer::FLAG_SERIALIZED) end - assert_equal exception.message, "Unable to unmarshal value: #{error_message}" + assert_equal exception.cause.message, error_message end end @@ -304,23 +271,7 @@ vs.retrieve(raw_value, Dalli::Protocol::ValueSerializer::FLAG_SERIALIZED) end - assert_equal exception.message, "Unable to unmarshal value: #{error_message}" - end - end - - describe 'when deserialization raises an ArgumentError with a non-matching message' do - let(:error_message) { SecureRandom.hex(10) } - let(:serializer) { Marshal } - - it 're-raises ArgumentError' do - error = ->(_arg) { raise ArgumentError, error_message } - exception = serializer.stub :load, error do - assert_raises ArgumentError do - vs.retrieve(raw_value, Dalli::Protocol::ValueSerializer::FLAG_SERIALIZED) - end - end - - assert_equal exception.message, error_message + assert_equal exception.cause.message, error_message end end @@ -335,7 +286,7 @@ deserialized_value end - it 'raises UnmarshalError for non-seriaized data' do + it 'raises UnmarshalError for non-serialized data' do assert_raises Dalli::UnmarshalError do vs.retrieve(:not_serialized_value, Dalli::Protocol::ValueSerializer::FLAG_SERIALIZED) end