Skip to content

Commit

Permalink
Serializer: reraise all .load errors as UnmarshalError (#1011)
Browse files Browse the repository at this point in the history
* 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.

* Treat all deserialization errors as UnmarshalError
  • Loading branch information
olleolleolle authored Sep 16, 2024
1 parent 718cacb commit f2a46a7
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 90 deletions.
43 changes: 9 additions & 34 deletions lib/dalli/protocol/value_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,42 +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)
serialized = (bitflags & FLAG_SERIALIZED) != 0
serialized ? serializer.load(value) : value
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}"
if serialized
begin
serializer.load(value)
rescue StandardError
raise UnmarshalError, 'Unable to unmarshal value'
end
else
value
end
end

def serializer
Expand Down
64 changes: 8 additions & 56 deletions test/protocol/test_value_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@
end
end

assert_equal exception.message, "Unable to unmarshal value: #{error_message}"
assert_equal exception.cause.message, error_message
end
end

Expand All @@ -198,7 +198,7 @@
end
end

assert_equal exception.message, "Unable to unmarshal value: #{error_message}"
assert_equal exception.cause.message, error_message
end
end

Expand All @@ -212,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

Expand All @@ -226,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

Expand All @@ -259,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

Expand All @@ -289,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

Expand All @@ -303,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

Expand All @@ -334,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
Expand Down

0 comments on commit f2a46a7

Please sign in to comment.