From 235adaa4e78df9a9efd3ad97ca66613a708e0d6c Mon Sep 17 00:00:00 2001 From: Raghu Ram Date: Mon, 3 Feb 2025 17:22:15 -0500 Subject: [PATCH] add exception handling for zlib compressed data errors --- lib/event_source/operations/mime_decode.rb | 24 ++++++++----------- .../protocols/amqp/bunny_queue_proxy.rb | 4 ---- .../operations/mime_decode_spec.rb | 13 ++++++++++ .../protocols/amqp/bunny_queue_proxy_spec.rb | 4 ---- 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/lib/event_source/operations/mime_decode.rb b/lib/event_source/operations/mime_decode.rb index 1229169..3e99857 100644 --- a/lib/event_source/operations/mime_decode.rb +++ b/lib/event_source/operations/mime_decode.rb @@ -8,6 +8,7 @@ module Operations # Operation for decoding payloads, including decompression using Zlib. class MimeDecode include Dry::Monads[:result, :do] + include EventSource::Logging # Supported MIME types for decoding. MIME_TYPES = %w[application/zlib application/json].freeze @@ -61,11 +62,15 @@ def validate_payload(payload, mime_type) # @return [Dry::Monads::Success] if decoding is successful # @return [Dry::Monads::Failure] if decoding fails def decode(payload, mime_type) - decoded_data = Zlib.inflate(payload) if mime_type == 'application/zlib' && binary_payload?(payload) - - Success(decoded_data || payload) - rescue Zlib::Error => e - Failure("Failed to decode payload using Zlib: #{e.message}") + return Success(payload) unless mime_type == 'application/zlib' + + begin + decoded_data = Zlib.inflate(payload) + Success(decoded_data) + rescue Zlib::Error => e + logger.error "Zlib errored while inflating payload: #{payload} \n with #{e.class}: #{e.message}, \n returning original payload." + Success(payload) + end end # Checks whether the payload is binary-encoded. @@ -79,15 +84,6 @@ def binary_payload?(payload) payload.encoding == Encoding::BINARY end - # For future reference, here is the implementation of the `zlib_compressed?` method: - # Binary encoding check alone is unreliable since the payload may not be zlib-compressed. - # Instead, verify if the payload starts with "\x78" to determine zlib compression. - def zlib_compressed?(payload) - return false unless payload.is_a?(String) - - payload.start_with?("\x78") - end - def valid_json_string?(data) data.is_a?(String) && JSON.parse(data) true diff --git a/lib/event_source/protocols/amqp/bunny_queue_proxy.rb b/lib/event_source/protocols/amqp/bunny_queue_proxy.rb index 421c330..b4e9b52 100644 --- a/lib/event_source/protocols/amqp/bunny_queue_proxy.rb +++ b/lib/event_source/protocols/amqp/bunny_queue_proxy.rb @@ -180,10 +180,6 @@ def decode_payload(payload) if output.success? output.value! else - logger.error '*' * 40 - logger.error payload.encoding if payload.respond_to?(:encoding) - logger.error payload.inspect - logger.error "Failed to decode message \n due to: #{output.failure} \n payload: #{payload}" raise EventSource::Error::PayloadDecodeError, output.failure end end diff --git a/spec/event_source/operations/mime_decode_spec.rb b/spec/event_source/operations/mime_decode_spec.rb index 17ddf62..7eaa084 100644 --- a/spec/event_source/operations/mime_decode_spec.rb +++ b/spec/event_source/operations/mime_decode_spec.rb @@ -93,6 +93,19 @@ expect(result.failure).to eq("Payload must be binary-encoded for MIME type 'application/zlib'.") end end + + context 'when Zlib.inflate raises an exception' do + let(:invalid_payload) { "NotCompressedData" } + + it 'returns the original payload wrapped in Success' do + allow(Zlib).to receive(:inflate).and_raise(Zlib::DataError, "invalid compressed data") + + result = subject.call('application/zlib', invalid_payload) + + expect(result).to be_a(Dry::Monads::Success) + expect(result.value!).to eq(invalid_payload) + end + end end end end diff --git a/spec/event_source/protocols/amqp/bunny_queue_proxy_spec.rb b/spec/event_source/protocols/amqp/bunny_queue_proxy_spec.rb index 213d438..0880f92 100644 --- a/spec/event_source/protocols/amqp/bunny_queue_proxy_spec.rb +++ b/spec/event_source/protocols/amqp/bunny_queue_proxy_spec.rb @@ -408,10 +408,6 @@ end it 'logs the error and raises a PayloadDecodeError' do - expect(instance.logger).to receive(:error).with('*' * 40) - expect(instance.logger).to receive(:error).with(payload.encoding) if payload.respond_to?(:encoding) - expect(instance.logger).to receive(:error).with(payload.inspect) - expect(instance.logger).to receive(:error).with("Failed to decode message \n due to: #{failure_message} \n payload: #{payload}") expect do instance.decode_payload(payload) end.to raise_error(EventSource::Error::PayloadDecodeError, failure_message)