Guard JSON serialization against json 3.0 encoding errors

sentry-ruby/lib/sentry/breadcrumb.rb

@@ -2,13 +2,10 @@
 
 module Sentry
   class Breadcrumb
-    MAX_NESTING = 10
-    DATA_SERIALIZATION_ERROR_MESSAGE = "[data were removed due to serialization issues]"
-
     # @return [String, nil]
     attr_accessor :category
     # @return [Hash, nil]
-    attr_accessor :data
+    attr_reader :data
     # @return [String, nil]
     attr_reader :level
     # @return [Time, Integer, nil]
@@ -26,7 +23,7 @@ class Breadcrumb
     # @param type [String, nil]
     def initialize(category: nil, data: nil, message: nil, timestamp: nil, level: nil, type: nil)
       @category = category
-      @data = data || {}
+      self.data = data
       @timestamp = timestamp || Sentry.utc_now.to_i
       @type = type
       self.message = message
@@ -37,7 +34,7 @@ def initialize(category: nil, data: nil, message: nil, timestamp: nil, level: ni
     def to_h
       {
         category: @category,
-        data: serialized_data,
+        data: @data,
         level: @level,
         message: @message,
         timestamp: @timestamp,
@@ -51,27 +48,17 @@ def message=(message)
       @message = message && Utils::EncodingHelper.valid_utf_8?(message) ? message.byteslice(0..Event::MAX_MESSAGE_SIZE_IN_BYTES) : ""
     end
 
+    # Sanitizes the breadcrumb's arbitrary, user-supplied data encoding.
+    # @param data [Hash, nil]
+    # @return [void]
+    def data=(data)
+      @data = Utils::EncodingHelper.deep_encode_utf_8(data || {})
+    end
+
     # @param level [String]
     # @return [void]
     def level=(level) # needed to meet the Sentry spec
       @level = level == "warn" ? "warning" : level
     end
-
-    private
-
-    def serialized_data
-      begin
-        ::JSON.parse(::JSON.generate(@data, max_nesting: MAX_NESTING))
-      rescue Exception => e
-        Sentry.sdk_logger.debug(LOGGER_PROGNAME) do
-          <<~MSG
-can't serialize breadcrumb data because of error: #{e}
-data: #{@data}
-          MSG
-        end
-
-        { error: DATA_SERIALIZATION_ERROR_MESSAGE }
-      end
-    end
   end
 end

sentry-ruby/lib/sentry/envelope/item.rb

@@ -72,6 +72,8 @@ def serialize
       end
 
       [result, result.bytesize > size_limit]
+    rescue EncodingError, JSON::GeneratorError => e
+      [nil, false, e]
     end
 
     def size_breakdown

sentry-ruby/lib/sentry/transport.rb

@@ -31,6 +31,7 @@ def initialize(configuration)
       @sdk_logger = configuration.sdk_logger
       @transport_configuration = configuration.transport
       @dsn = configuration.dsn
+      @debug = configuration.debug
       @rate_limits = {}
       @send_client_reports = configuration.send_client_reports
 
@@ -73,7 +74,14 @@ def serialize_envelope(envelope)
       serialized_results = []
 
       envelope.items.each do |item|
-        result, oversized = item.serialize
+        result, oversized, serialization_error = item.serialize
+
+        if serialization_error
+          log_error("[Transport] Failed to serialize envelope item [#{item.type}]", serialization_error, debug: @debug)
+          record_lost_event(:send_error, item.data_category, num: item.item_count)
+
+          next
+        end
 
         if oversized
           log_debug("Envelope item [#{item.type}] is still oversized after size reduction: {#{item.size_breakdown}}")
@@ -85,7 +93,9 @@ def serialize_envelope(envelope)
         serialized_items << item
       end
 
-      data = [JSON.generate(envelope.headers), *serialized_results].join("\n") unless serialized_results.empty?
+      unless serialized_results.empty?
+        data = [JSON.generate(envelope.headers), *serialized_results].join("\n")
+      end
 
       [data, serialized_items]
     end

sentry-ruby/lib/sentry/utils/encoding_helper.rb

@@ -23,6 +23,52 @@ def self.valid_utf_8?(value)
       def self.safe_utf_8_string(value)
         valid_utf_8?(value) ? value : MALFORMED_STRING
       end
+
+      # Recursively walks a Hash/Array/String structure and returns a copy
+      # with every String forced into valid UTF-8 encoding.
+      #
+      # Circular Hash and Array references are replaced with nil in the
+      # returned copy.
+      #
+      # @param value [Object]
+      # @return [Object]
+      def self.deep_encode_utf_8(value, seen = {})
+        case value
+        when String
+          encode_to_utf_8(value)
+        when Hash
+          return nil if seen.key?(value.object_id)
+
+          seen[value.object_id] = true
+          encoded_value = {}
+
+          begin
+            value.each do |key, val|
+              encoded_key = deep_encode_utf_8(key, seen)
+              encoded_value[encoded_key] = deep_encode_utf_8(val, seen)
+            end
+            encoded_value
+          ensure
+            seen.delete(value.object_id)
+          end
+        when Array
+          return nil if seen.key?(value.object_id)
+
+          seen[value.object_id] = true
+          encoded_value = []
+
+          begin
+            value.each do |val|
+              encoded_value << deep_encode_utf_8(val, seen)
+            end
+            encoded_value
+          ensure
+            seen.delete(value.object_id)
+          end
+        else
+          value
+        end
+      end
     end
   end
 end

sentry-ruby/lib/sentry/utils/telemetry_attributes.rb

@@ -19,16 +19,20 @@ def attribute_hash(raw_value)
         result =
           case value
           when String
-            { value: value, type: "string" }
+            { value: Utils::EncodingHelper.encode_to_utf_8(value), type: "string" }
           when TrueClass, FalseClass
             { value: value, type: "boolean" }
           when Integer
             { value: value, type: "integer" }
           when Float
             { value: value, type: "double" }
           else
+            # `value` may be an arbitrary object (e.g. a Hash/Array) that
+            # contains a String with an invalid/non-UTF-8 encoding, which
+            # `JSON.generate` raises on as of json 3.0+. Sanitize it before
+            # generating rather than reacting to the error.
             begin
-              { value: JSON.generate(value), type: "string" }
+              { value: JSON.generate(Utils::EncodingHelper.deep_encode_utf_8(value)), type: "string" }
             rescue
               { value: value, type: "string" }
             end

sentry-ruby/spec/sentry/breadcrumb_buffer_spec.rb

@@ -23,20 +23,6 @@
     )
   end
 
-  let(:problematic_crumb) do
-    # circular reference
-    a = []
-    b = []
-    a.push(b)
-    b.push(a)
-
-    Sentry::Breadcrumb.new(
-      category: "baz",
-      message: "crumb_3",
-      data: a
-    )
-  end
-
   describe "#record" do
     subject do
       described_class.new(1)
@@ -56,18 +42,15 @@
   end
 
   describe "#to_h" do
-    it "doesn't break because of 1 problematic crumb" do
+    it "serializes breadcrumbs" do
       subject.record(crumb_1)
       subject.record(crumb_2)
-      subject.record(problematic_crumb)
 
       result = subject.to_h[:values]
 
       expect(result[0][:category]).to eq("foo")
-      expect(result[0][:data]).to eq({ "name" => "John", "age" => 25 })
+      expect(result[0][:data]).to eq({ name: "John", age: 25 })
       expect(result[1][:category]).to eq("bar")
-      expect(result[2][:category]).to eq("baz")
-      expect(result[2][:data][:error]).to eq("[data were removed due to serialization issues]")
     end
   end
 end

sentry-ruby/spec/sentry/breadcrumb_spec.rb

@@ -79,40 +79,38 @@
       )
     end
 
-    let(:very_deep_crumb) do
-      data = [[[[[ { a: [{ b: [[{ c: 4 }]] }] }]]]]]
-
-      Sentry::Breadcrumb.new(
-        category: "cow",
-        message: "I cause too much recursion",
-        data: data
-      )
-    end
-
-    it "serializes data correctly" do
+    it "returns the sanitized data" do
       result = crumb.to_h
 
       expect(result[:category]).to eq("foo")
       expect(result[:message]).to eq("crumb")
-      expect(result[:data]).to eq({ "name" => "John", "age" => 25 })
+      expect(result[:data]).to eq({ name: "John", age: 25 })
     end
 
-    it "rescues data serialization issue and ditch the data" do
+    it "handles a circular breadcrumb without recursing forever" do
       result = problematic_crumb.to_h
 
       expect(result[:category]).to eq("baz")
       expect(result[:message]).to eq("I cause issues")
-      expect(result[:data][:error]).to eq("[data were removed due to serialization issues]")
-      expect(stringio.string).to match(/can't serialize breadcrumb data because of error: nesting of 10 is too deep/)
+      expect(result[:data]).to eq([[nil]])
+      expect { JSON.generate(result[:data]) }.not_to raise_error
     end
 
-    it "rescues data serialization issue for extremely nested data and ditch the data" do
-      result = very_deep_crumb.to_h
+    it "sanitizes non-UTF-8 encoded strings in data at assignment time (json 3.0+ behavior)" do
+      # json 3.0+ raises Encoding::UndefinedConversionError instead of just
+      # warning when JSON.generate encounters a String tagged with a
+      # non-UTF-8 encoding that contains bytes invalid for the target
+      # encoding. Breadcrumb#data= sanitizes proactively so this never
+      # reaches JSON.generate in the first place.
+      invalid_string = "\xFF\xFEinvalid".dup.force_encoding(Encoding::BINARY)
+      crumb = Sentry::Breadcrumb.new(category: "foo", message: "crumb", data: { note: invalid_string })
+
+      expect(crumb.data[:note].encoding).to eq(Encoding::UTF_8)
+      expect(crumb.data[:note].valid_encoding?).to eq(true)
 
-      expect(result[:category]).to eq("cow")
-      expect(result[:message]).to eq("I cause too much recursion")
-      expect(result[:data][:error]).to eq("[data were removed due to serialization issues]")
-      expect(stringio.string).to match(/can't serialize breadcrumb data because of error: nesting of 10 is too deep/)
+      result = crumb.to_h
+      expect(result[:data][:note].encoding).to eq(Encoding::UTF_8)
+      expect(result[:data][:note].valid_encoding?).to eq(true)
     end
   end
 end

sentry-ruby/spec/sentry/log_event_spec.rb

@@ -187,5 +187,24 @@
 
       expect(hash[:attributes]).not_to have_key("sentry.origin")
     end
+
+    it "sanitizes non-UTF-8 encoded strings in a non-scalar attribute value (json 3.0+ behavior)" do
+      # json 3.0+ raises Encoding::UndefinedConversionError instead of just
+      # warning when JSON.generate encounters a String tagged with a
+      # non-UTF-8 encoding that contains bytes invalid for the target
+      # encoding. `attribute_hash` sanitizes non-scalar values proactively
+      # before generating.
+      invalid_string = "\xFF\xFEinvalid".dup.force_encoding(Encoding::BINARY)
+
+      event = described_class.new(
+        level: :info,
+        body: "Manual log message",
+        attributes: { payload: { note: invalid_string } }
+      )
+
+      hash = event.to_h
+      expect { JSON.generate(hash) }.not_to raise_error
+      expect(hash[:attributes][:payload][:type]).to eq("string")
+    end
   end
 end

sentry-ruby/spec/sentry/transport_spec.rb

@@ -499,6 +499,20 @@
       end
     end
 
+    context "when sending raises an encoding error" do
+      let(:event) { client.event_from_exception(ZeroDivisionError.new("divided by 0")) }
+      let(:envelope) { subject.envelope_from_event(event) }
+
+      before do
+        allow(subject).to receive(:send_data).and_raise(EncodingError, "simulated send error")
+      end
+
+      it "does not handle the error as a serialization failure" do
+        expect { subject.send_envelope(envelope) }.to raise_error(EncodingError, "simulated send error")
+        expect(io.string).not_to match(/Failed to serialize envelope/)
+      end
+    end
+
     context "transaction event" do
       let(:transaction) do
         Sentry::Transaction.new(name: "test transaction", op: "rack.request")
@@ -651,6 +665,37 @@
         expect(io.string).to match(/Sending envelope with items \[log\]/)
       end
     end
+
+    context "when JSON.generate raises an encoding error for an item (json 3.0+ behavior)" do
+      let(:bad_payload) { { message: "bad payload" } }
+      let(:good_payload) { { message: "good payload" } }
+      let(:envelope) do
+        Sentry::Envelope.new.tap do |new_envelope|
+          new_envelope.add_item({ type: "event" }, bad_payload)
+          new_envelope.add_item({ type: "event" }, good_payload)
+        end
+      end
+
+      before do
+        allow(JSON).to receive(:generate).and_wrap_original do |original, value|
+          raise EncodingError, "simulated json 3.0 encoding error" if value.equal?(bad_payload)
+
+          original.call(value)
+        end
+      end
+
+      it "skips the failed item, sends the remaining items, and records the loss" do
+        expect(subject).to receive(:send_data) do |data|
+          expect(data).to include("good payload")
+          expect(data).not_to include("bad payload")
+        end
+
+        expect { subject.send_envelope(envelope) }.not_to raise_error
+
+        expect(io.string).to match(/Failed to serialize envelope item/)
+        expect(subject).to have_recorded_lost_event(:send_error, 'error')
+      end
+    end
   end
 
   describe "#send_event" do