Skip to content

Commit f11dc6c

Browse files
sentry-junior[bot]sl0thentr0py
authored andcommitted
Sanitize encoding at data-fill points instead of reactive deep-encode retries
Per review feedback: drop the reactive 'catch JSON.generate, deep-encode, and retry' pattern from serialization exit points (Envelope::Item, Transport#serialize_envelope headers, Event#to_json_compatible, TelemetryEventBuffer, DebugTransport, DebugStructuredLogger). Those are reverted to plain JSON.generate/JSON.dump. Instead, sanitize encoding proactively wherever the affected data actually gets filled in: - Breadcrumb#data= now force-encodes/scrubs strings in the arbitrary user-supplied data hash to valid UTF-8 (both on construction and on later assignment), matching the existing pattern used for Breadcrumb#message=, SingleException#value, and RequestInterface's body/headers. - Utils::TelemetryAttributes#attribute_hash now sanitizes String attribute values directly, and sanitizes non-scalar (Hash/Array) attribute values before JSON.generate rather than reacting to a raised error. Transport#send_envelope keeps its rescue for EncodingError/ JSON::GeneratorError as a last-resort safety net (log + record lost event) for any case not already covered by the above, without attempting to fix and retry. Updated specs accordingly.
1 parent b124ade commit f11dc6c

13 files changed

Lines changed: 59 additions & 134 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ We recommend migrating to `data_collection` to match the behavior you want event
6666

6767
### Bug Fixes 🐛
6868

69-
- Sanitize payload encodings and rescue `EncodingError`/`JSON::GeneratorError` during envelope/event JSON serialization to support the `json` gem 3.0, which now raises instead of warning when generating JSON from a String tagged with a non-UTF-8 encoding. Fixes #2462
69+
- Sanitize breadcrumb data and structured-log attribute values to valid UTF-8 where they're filled in, and add a last-resort rescue for `EncodingError`/`JSON::GeneratorError` in `Transport#send_envelope`, to support the `json` gem 3.0, which now raises instead of warning when generating JSON from a String tagged with a non-UTF-8 encoding. Fixes #2462
7070

7171
## 6.7.0
7272

sentry-ruby/lib/sentry/breadcrumb.rb

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ class Breadcrumb
88
# @return [String, nil]
99
attr_accessor :category
1010
# @return [Hash, nil]
11-
attr_accessor :data
11+
attr_reader :data
1212
# @return [String, nil]
1313
attr_reader :level
1414
# @return [Time, Integer, nil]
@@ -26,7 +26,7 @@ class Breadcrumb
2626
# @param type [String, nil]
2727
def initialize(category: nil, data: nil, message: nil, timestamp: nil, level: nil, type: nil)
2828
@category = category
29-
@data = data || {}
29+
self.data = data
3030
@timestamp = timestamp || Sentry.utc_now.to_i
3131
@type = type
3232
self.message = message
@@ -51,6 +51,14 @@ def message=(message)
5151
@message = message && Utils::EncodingHelper.valid_utf_8?(message) ? message.byteslice(0..Event::MAX_MESSAGE_SIZE_IN_BYTES) : ""
5252
end
5353

54+
# Sanitizes the breadcrumb's arbitrary, user-supplied data so it doesn't
55+
# carry a String with an invalid/non-UTF-8 encoding into JSON generation.
56+
# @param data [Hash, nil]
57+
# @return [void]
58+
def data=(data)
59+
@data = Utils::EncodingHelper.deep_encode_utf_8(data || {})
60+
end
61+
5462
# @param level [String]
5563
# @return [void]
5664
def level=(level) # needed to meet the Sentry spec
@@ -62,11 +70,6 @@ def level=(level) # needed to meet the Sentry spec
6270
def serialized_data
6371
begin
6472
::JSON.parse(::JSON.generate(@data, max_nesting: MAX_NESTING))
65-
rescue EncodingError, ::JSON::GeneratorError
66-
# As of json 3.0, `JSON.generate` raises instead of warning when it
67-
# encounters a String with an invalid/non-UTF-8 encoding (e.g. a
68-
# BINARY-tagged String). Sanitize and retry once before giving up.
69-
::JSON.parse(::JSON.generate(Utils::EncodingHelper.deep_encode_utf_8(@data), max_nesting: MAX_NESTING))
7073
rescue Exception => e
7174
Sentry.sdk_logger.debug(LOGGER_PROGNAME) do
7275
<<~MSG

sentry-ruby/lib/sentry/debug_structured_logger.rb

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ def capture_log_event(level, message, parameters, **attributes)
4747
attributes: attributes
4848
}
4949

50-
File.open(log_file, "a") { |file| file << safe_json_dump(log_event_json) << "\n" }
50+
File.open(log_file, "a") { |file| file << JSON.dump(log_event_json) << "\n" }
5151
log_event_json
5252
end
5353

@@ -66,15 +66,6 @@ def clear
6666

6767
private
6868

69-
# Sanitizes `value` before dumping to JSON so a String tagged with an
70-
# invalid/non-UTF-8 encoding (which raises with json 3.0+) doesn't
71-
# crash debug logging.
72-
def safe_json_dump(value)
73-
JSON.dump(value)
74-
rescue EncodingError, JSON::GeneratorError
75-
JSON.dump(Sentry::Utils::EncodingHelper.deep_encode_utf_8(value))
76-
end
77-
7869
def initialize_backend(configuration)
7970
StructuredLogger.new(configuration)
8071
end

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

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,11 @@ def item_count
5151
def lost_event_byte_size
5252
return unless byte_data_category
5353

54-
(payload.is_a?(String) ? payload : safe_json_generate(payload)).bytesize
54+
(payload.is_a?(String) ? payload : JSON.generate(payload)).bytesize
5555
end
5656

5757
def to_s
58-
[
59-
safe_json_generate(@headers),
60-
@payload.is_a?(String) ? @payload : safe_json_generate(@payload)
61-
].join("\n")
58+
[JSON.generate(@headers), @payload.is_a?(String) ? @payload : JSON.generate(@payload)].join("\n")
6259
end
6360

6461
def serialize
@@ -79,24 +76,12 @@ def serialize
7976

8077
def size_breakdown
8178
payload.map do |key, value|
82-
"#{key}: #{safe_json_generate(value).bytesize}"
79+
"#{key}: #{JSON.generate(value).bytesize}"
8380
end.join(", ")
8481
end
8582

8683
private
8784

88-
# Sanitizes `value` (removing invalid UTF-8 byte sequences from any
89-
# tagged strings) before generating JSON, and retries once if
90-
# `JSON.generate` still raises an encoding related error.
91-
#
92-
# See `Utils::EncodingHelper.deep_encode_utf_8` for details on why this
93-
# is necessary as of json 3.0.
94-
def safe_json_generate(value)
95-
JSON.generate(value)
96-
rescue EncodingError, JSON::GeneratorError
97-
JSON.generate(Utils::EncodingHelper.deep_encode_utf_8(value))
98-
end
99-
10085
def remove_breadcrumbs!
10186
if payload.key?(:breadcrumbs)
10287
payload.delete(:breadcrumbs)

sentry-ruby/lib/sentry/event.rb

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,7 @@ def to_h
121121

122122
# @return [Hash]
123123
def to_json_compatible
124-
hash = to_h
125-
126-
JSON.parse(JSON.generate(hash))
127-
rescue EncodingError, JSON::GeneratorError
128-
JSON.parse(JSON.generate(Utils::EncodingHelper.deep_encode_utf_8(hash)))
124+
JSON.parse(JSON.generate(to_h))
129125
end
130126

131127
private

sentry-ruby/lib/sentry/telemetry_event_buffer.rb

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ def add_item(item)
6262
@client.transport.record_lost_event(
6363
:queue_overflow,
6464
@data_category,
65-
num_bytes: safe_json_bytesize(item.to_h)
65+
num_bytes: JSON.generate(item.to_h).bytesize
6666
)
6767
else
6868
@pending_items << item
@@ -103,7 +103,7 @@ def send_items
103103
envelope_items << processed_item.to_h
104104
else
105105
discarded_count += 1
106-
discarded_bytes += safe_json_bytesize(item.to_h)
106+
discarded_bytes += JSON.generate(item.to_h).bytesize
107107
end
108108
end
109109
else
@@ -131,14 +131,5 @@ def send_items
131131
ensure
132132
clear!
133133
end
134-
135-
# Sanitizes `value` before generating JSON so that a String tagged with
136-
# an invalid/non-UTF-8 encoding (which raises with json 3.0+) doesn't
137-
# crash telemetry buffering.
138-
def safe_json_bytesize(value)
139-
JSON.generate(value).bytesize
140-
rescue EncodingError, JSON::GeneratorError
141-
JSON.generate(Utils::EncodingHelper.deep_encode_utf_8(value)).bytesize
142-
end
143134
end
144135
end

sentry-ruby/lib/sentry/transport.rb

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ def send_envelope(envelope)
7070
rescue EncodingError, JSON::GeneratorError => e
7171
# As of json 3.0, `JSON.generate` raises instead of warning when it
7272
# encounters a String with an invalid/non-UTF-8 encoding (e.g. a
73-
# BINARY-tagged String). Individual envelope items already sanitize
74-
# their own payloads, but this is a last resort so a single bad
75-
# event can't crash the background worker.
73+
# BINARY-tagged String). We sanitize known data-entry points (e.g.
74+
# breadcrumb data, log attributes), but this is a last resort so an
75+
# unexpected case can't crash the background worker.
7676
log_error("[Transport] Failed to serialize envelope", e, debug: @debug)
7777

7878
# `serialized_items` may still be nil here if the error was raised
@@ -101,13 +101,7 @@ def serialize_envelope(envelope)
101101
end
102102

103103
unless serialized_results.empty?
104-
headers = begin
105-
JSON.generate(envelope.headers)
106-
rescue EncodingError, JSON::GeneratorError
107-
JSON.generate(Utils::EncodingHelper.deep_encode_utf_8(envelope.headers))
108-
end
109-
110-
data = [headers, *serialized_results].join("\n")
104+
data = [JSON.generate(envelope.headers), *serialized_results].join("\n")
111105
end
112106

113107
[data, serialized_items]

sentry-ruby/lib/sentry/transport/debug_transport.rb

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ def log_envelope(envelope)
3636
end
3737
}
3838

39-
File.open(log_file, "a") { |file| file << safe_json_dump(envelope_json) << "\n" }
39+
File.open(log_file, "a") { |file| file << JSON.dump(envelope_json) << "\n" }
4040
end
4141

4242
def logged_envelopes
@@ -54,15 +54,6 @@ def clear
5454

5555
private
5656

57-
# Sanitizes `value` before dumping to JSON so a String tagged with an
58-
# invalid/non-UTF-8 encoding (which raises with json 3.0+) doesn't
59-
# crash debug logging.
60-
def safe_json_dump(value)
61-
JSON.dump(value)
62-
rescue EncodingError, JSON::GeneratorError
63-
JSON.dump(Utils::EncodingHelper.deep_encode_utf_8(value))
64-
end
65-
6657
def initialize_backend(configuration)
6758
backend = configuration.dsn.local? ? DummyTransport : HTTPTransport
6859
backend.new(configuration)

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

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,26 +19,20 @@ def attribute_hash(raw_value)
1919
result =
2020
case value
2121
when String
22-
{ value: value, type: "string" }
22+
{ value: Utils::EncodingHelper.encode_to_utf_8(value), type: "string" }
2323
when TrueClass, FalseClass
2424
{ value: value, type: "boolean" }
2525
when Integer
2626
{ value: value, type: "integer" }
2727
when Float
2828
{ value: value, type: "double" }
2929
else
30+
# `value` may be an arbitrary object (e.g. a Hash/Array) that
31+
# contains a String with an invalid/non-UTF-8 encoding, which
32+
# `JSON.generate` raises on as of json 3.0+. Sanitize it before
33+
# generating rather than reacting to the error.
3034
begin
31-
{ value: JSON.generate(value), type: "string" }
32-
rescue EncodingError, JSON::GeneratorError
33-
# As of json 3.0, `JSON.generate` raises instead of warning
34-
# when it encounters a String with an invalid/non-UTF-8
35-
# encoding (e.g. a BINARY-tagged String). Sanitize and retry
36-
# once before giving up.
37-
begin
38-
{ value: JSON.generate(Sentry::Utils::EncodingHelper.deep_encode_utf_8(value)), type: "string" }
39-
rescue
40-
{ value: value, type: "string" }
41-
end
35+
{ value: JSON.generate(Utils::EncodingHelper.deep_encode_utf_8(value)), type: "string" }
4236
rescue
4337
{ value: value, type: "string" }
4438
end

sentry-ruby/spec/sentry/breadcrumb_spec.rb

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -115,22 +115,17 @@
115115
expect(stringio.string).to match(/can't serialize breadcrumb data because of error: nesting of 10 is too deep/)
116116
end
117117

118-
it "sanitizes and retries when JSON.generate raises an encoding error (json 3.0+ behavior)" do
119-
# json 3.0+ raises Encoding::UndefinedConversionError (a subclass of
120-
# EncodingError) instead of just warning when JSON.generate encounters
121-
# a String tagged with a non-UTF-8 encoding that contains bytes
122-
# invalid for the target encoding. Simulate that here regardless of
123-
# the json gem version actually loaded.
118+
it "sanitizes non-UTF-8 encoded strings in data at assignment time (json 3.0+ behavior)" do
119+
# json 3.0+ raises Encoding::UndefinedConversionError instead of just
120+
# warning when JSON.generate encounters a String tagged with a
121+
# non-UTF-8 encoding that contains bytes invalid for the target
122+
# encoding. Breadcrumb#data= sanitizes proactively so this never
123+
# reaches JSON.generate in the first place.
124124
invalid_string = "\xFF\xFEinvalid".dup.force_encoding(Encoding::BINARY)
125125
crumb = Sentry::Breadcrumb.new(category: "foo", message: "crumb", data: { note: invalid_string })
126126

127-
call_count = 0
128-
allow(JSON).to receive(:generate).and_wrap_original do |original, *args|
129-
call_count += 1
130-
raise EncodingError, "simulated json 3.0 encoding error" if call_count == 1
131-
132-
original.call(*args)
133-
end
127+
expect(crumb.data[:note].encoding).to eq(Encoding::UTF_8)
128+
expect(crumb.data[:note].valid_encoding?).to eq(true)
134129

135130
result = crumb.to_h
136131
expect(result[:data]).not_to eq({ error: Sentry::Breadcrumb::DATA_SERIALIZATION_ERROR_MESSAGE })

0 commit comments

Comments
 (0)