From 4c41ad88ed031e6fdea93223c0c4735e8c7846a6 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Fri, 23 Oct 2026 22:09:18 +0200 Subject: [PATCH 1/4] fix: drop attachment envelope items with an empty payload Non-seekable attachment streams have no known length when the envelope is created, so the existing zero-length check can't catch them. Skip the item at serialization time, once its payload is buffered and before its header is written. Co-Authored-By: Claude --- src/Sentry/Protocol/Envelopes/Envelope.cs | 12 +++-- src/Sentry/Protocol/Envelopes/EnvelopeItem.cs | 34 +++++++++++++- .../Envelopes/AttachmentSerializationTests.cs | 45 +++++++++++++++++++ .../Protocol/Envelopes/EnvelopeTests.cs | 12 +---- 4 files changed, 87 insertions(+), 16 deletions(-) diff --git a/src/Sentry/Protocol/Envelopes/Envelope.cs b/src/Sentry/Protocol/Envelopes/Envelope.cs index 0174dbd32a..edb432dc5b 100644 --- a/src/Sentry/Protocol/Envelopes/Envelope.cs +++ b/src/Sentry/Protocol/Envelopes/Envelope.cs @@ -155,8 +155,10 @@ internal async Task SerializeAsync( { try { - await item.SerializeAsync(stream, logger, cancellationToken).ConfigureAwait(false); - await stream.WriteNewlineAsync(cancellationToken).ConfigureAwait(false); + if (await item.SerializeAsync(stream, logger, dropEmptyAttachment: true, cancellationToken).ConfigureAwait(false)) + { + await stream.WriteNewlineAsync(cancellationToken).ConfigureAwait(false); + } } catch (Exception e) { @@ -180,8 +182,10 @@ internal void Serialize(Stream stream, IDiagnosticLogger? logger, ISystemClock c { try { - item.Serialize(stream, logger); - stream.WriteNewline(); + if (item.Serialize(stream, logger, dropEmptyAttachment: true)) + { + stream.WriteNewline(); + } } catch (Exception e) { diff --git a/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs b/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs index 364d15087e..00957c89c7 100644 --- a/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs +++ b/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs @@ -175,7 +175,11 @@ private static void SerializeHeader( } /// - public async Task SerializeAsync(Stream stream, IDiagnosticLogger? logger, + public Task SerializeAsync(Stream stream, IDiagnosticLogger? logger, + CancellationToken cancellationToken = default) => + SerializeAsync(stream, logger, dropEmptyAttachment: false, cancellationToken); + + internal async Task SerializeAsync(Stream stream, IDiagnosticLogger? logger, bool dropEmptyAttachment, CancellationToken cancellationToken = default) { // Always calculate the length of the payload, as Sentry will reject envelopes that have incorrect lengths @@ -189,6 +193,11 @@ public async Task SerializeAsync(Stream stream, IDiagnosticLogger? logger, await using (payloadBuffer.ConfigureAwait(false)) #endif { + if (dropEmptyAttachment && IsEmptyAttachment(payloadBuffer, logger)) + { + return false; + } + // Write to the outbound stream asynchronously. It's likely either an HttpRequestStream or a FileStream. // Header @@ -199,11 +208,15 @@ public async Task SerializeAsync(Stream stream, IDiagnosticLogger? logger, // Payload await payloadBuffer.CopyToAsync(stream, cancellationToken).ConfigureAwait(false); + return true; } } /// - public void Serialize(Stream stream, IDiagnosticLogger? logger) + public void Serialize(Stream stream, IDiagnosticLogger? logger) => + Serialize(stream, logger, dropEmptyAttachment: false); + + internal bool Serialize(Stream stream, IDiagnosticLogger? logger, bool dropEmptyAttachment) { // Always calculate the length of the payload, as Sentry will reject envelopes that have incorrect lengths // in item headers. Don't trust any previously calculated value to be correct. @@ -211,6 +224,11 @@ public void Serialize(Stream stream, IDiagnosticLogger? logger) using var payloadBuffer = BufferPayload(logger); + if (dropEmptyAttachment && IsEmptyAttachment(payloadBuffer, logger)) + { + return false; + } + // Header var headerWithLength = Header.ToDict(); headerWithLength[LengthKey] = payloadBuffer.Length; @@ -219,6 +237,18 @@ public void Serialize(Stream stream, IDiagnosticLogger? logger) // Payload payloadBuffer.CopyTo(stream); + return true; + } + + private bool IsEmptyAttachment(MemoryStream payloadBuffer, IDiagnosticLogger? logger) + { + if (payloadBuffer.Length != 0 || TryGetType() != TypeValueAttachment) + { + return false; + } + + logger?.LogWarning("Did not add '{0}' to envelope because the attachment was empty.", TryGetFileName()); + return true; } /// diff --git a/test/Sentry.Tests/Protocol/Envelopes/AttachmentSerializationTests.cs b/test/Sentry.Tests/Protocol/Envelopes/AttachmentSerializationTests.cs index 061fdaddca..41c3893998 100644 --- a/test/Sentry.Tests/Protocol/Envelopes/AttachmentSerializationTests.cs +++ b/test/Sentry.Tests/Protocol/Envelopes/AttachmentSerializationTests.cs @@ -389,4 +389,49 @@ public void FromEvent_EmptyByteAttachment_DoesNotAddAttachment() .Should() .Be(0); } + + [Fact] + public async Task Serialization_EmptyNonSeekableStreamAttachment_DropsAttachment() + { + // Arrange + using var envelope = CreateEventEnvelopeWithEmptyNonSeekableAttachment(); + + // Act + var serialized = await envelope.SerializeToStringAsync(_testOutputLogger, _fakeClock); + + // Assert + serialized.Should().NotContain("empty.jpg"); + using var stream = new MemoryStream(Encoding.UTF8.GetBytes(serialized)); + using var deserialized = await Envelope.DeserializeAsync(stream); + deserialized.Items.Should().ContainSingle() + .Which.TryGetType().Should().Be(EnvelopeItem.TypeValueEvent); + } + + [Fact] + public async Task Serialization_EmptyNonSeekableStreamAttachmentSynchronously_DropsAttachment() + { + // Arrange + using var envelope = CreateEventEnvelopeWithEmptyNonSeekableAttachment(); + + // Act + var serialized = envelope.SerializeToString(_testOutputLogger, _fakeClock); + + // Assert + serialized.Should().NotContain("empty.jpg"); + using var stream = new MemoryStream(Encoding.UTF8.GetBytes(serialized)); + using var deserialized = await Envelope.DeserializeAsync(stream); + deserialized.Items.Should().ContainSingle() + .Which.TryGetType().Should().Be(EnvelopeItem.TypeValueEvent); + } + + private Envelope CreateEventEnvelopeWithEmptyNonSeekableAttachment() + { + var attachment = new SentryAttachment( + AttachmentType.Default, + new StreamAttachmentContent(new NonSeekableReadStream(new MemoryStream())), + "empty.jpg", + "image/jpeg"); + + return Envelope.FromEvent(new SentryEvent(), _testOutputLogger, [attachment]); + } } diff --git a/test/Sentry.Tests/Protocol/Envelopes/EnvelopeTests.cs b/test/Sentry.Tests/Protocol/Envelopes/EnvelopeTests.cs index 8963e3e223..ad91895919 100644 --- a/test/Sentry.Tests/Protocol/Envelopes/EnvelopeTests.cs +++ b/test/Sentry.Tests/Protocol/Envelopes/EnvelopeTests.cs @@ -361,7 +361,7 @@ public async Task Deserialization_EnvelopeWithTwoItems_Success() } [Fact] - public async Task Serialization_EnvelopeWithTwoEmptyItems_Success() + public async Task Serialization_EnvelopeWithTwoEmptyAttachments_DropsAttachments() { // Arrange using var envelope = new Envelope( @@ -393,16 +393,12 @@ public async Task Serialization_EnvelopeWithTwoEmptyItems_Success() // Assert output.Should().Be(""" {"event_id":"9ec79c33ec9942ab8353589fcb2e04dc","sent_at":"9999-12-31T23:59:59.9999999+00:00"} - {"type":"attachment","length":0} - - {"type":"attachment","length":0} - """); } [Fact] - public void Serialization_EnvelopeWithTwoEmptyItems_Success_Synchronous() + public void Serialization_EnvelopeWithTwoEmptyAttachments_DropsAttachments_Synchronous() { // Arrange using var envelope = new Envelope( @@ -434,10 +430,6 @@ public void Serialization_EnvelopeWithTwoEmptyItems_Success_Synchronous() // Assert output.Should().Be(""" {"event_id":"9ec79c33ec9942ab8353589fcb2e04dc","sent_at":"9999-12-31T23:59:59.9999999+00:00"} - {"type":"attachment","length":0} - - {"type":"attachment","length":0} - """); } From 5a7c52e5df1dd7d92410780144e166e23d236722 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Fri, 23 Oct 2026 22:11:05 +0200 Subject: [PATCH 2/4] fix: only drop empty attachments whose length was unknown Keeps explicit length-0 items, as in the envelope spec's framing examples, serializing as before. Co-Authored-By: Claude --- src/Sentry/Protocol/Envelopes/EnvelopeItem.cs | 8 ++++---- .../Sentry.Tests/Protocol/Envelopes/EnvelopeTests.cs | 12 ++++++++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs b/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs index 00957c89c7..af4388e494 100644 --- a/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs +++ b/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs @@ -193,7 +193,7 @@ internal async Task SerializeAsync(Stream stream, IDiagnosticLogger? logge await using (payloadBuffer.ConfigureAwait(false)) #endif { - if (dropEmptyAttachment && IsEmptyAttachment(payloadBuffer, logger)) + if (dropEmptyAttachment && IsEmptyAttachmentOfUnknownLength(payloadBuffer, logger)) { return false; } @@ -224,7 +224,7 @@ internal bool Serialize(Stream stream, IDiagnosticLogger? logger, bool dropEmpty using var payloadBuffer = BufferPayload(logger); - if (dropEmptyAttachment && IsEmptyAttachment(payloadBuffer, logger)) + if (dropEmptyAttachment && IsEmptyAttachmentOfUnknownLength(payloadBuffer, logger)) { return false; } @@ -240,9 +240,9 @@ internal bool Serialize(Stream stream, IDiagnosticLogger? logger, bool dropEmpty return true; } - private bool IsEmptyAttachment(MemoryStream payloadBuffer, IDiagnosticLogger? logger) + private bool IsEmptyAttachmentOfUnknownLength(MemoryStream payloadBuffer, IDiagnosticLogger? logger) { - if (payloadBuffer.Length != 0 || TryGetType() != TypeValueAttachment) + if (payloadBuffer.Length != 0 || TryGetLength() is not null || TryGetType() != TypeValueAttachment) { return false; } diff --git a/test/Sentry.Tests/Protocol/Envelopes/EnvelopeTests.cs b/test/Sentry.Tests/Protocol/Envelopes/EnvelopeTests.cs index ad91895919..8963e3e223 100644 --- a/test/Sentry.Tests/Protocol/Envelopes/EnvelopeTests.cs +++ b/test/Sentry.Tests/Protocol/Envelopes/EnvelopeTests.cs @@ -361,7 +361,7 @@ public async Task Deserialization_EnvelopeWithTwoItems_Success() } [Fact] - public async Task Serialization_EnvelopeWithTwoEmptyAttachments_DropsAttachments() + public async Task Serialization_EnvelopeWithTwoEmptyItems_Success() { // Arrange using var envelope = new Envelope( @@ -393,12 +393,16 @@ public async Task Serialization_EnvelopeWithTwoEmptyAttachments_DropsAttachments // Assert output.Should().Be(""" {"event_id":"9ec79c33ec9942ab8353589fcb2e04dc","sent_at":"9999-12-31T23:59:59.9999999+00:00"} + {"type":"attachment","length":0} + + {"type":"attachment","length":0} + """); } [Fact] - public void Serialization_EnvelopeWithTwoEmptyAttachments_DropsAttachments_Synchronous() + public void Serialization_EnvelopeWithTwoEmptyItems_Success_Synchronous() { // Arrange using var envelope = new Envelope( @@ -430,6 +434,10 @@ public void Serialization_EnvelopeWithTwoEmptyAttachments_DropsAttachments_Synch // Assert output.Should().Be(""" {"event_id":"9ec79c33ec9942ab8353589fcb2e04dc","sent_at":"9999-12-31T23:59:59.9999999+00:00"} + {"type":"attachment","length":0} + + {"type":"attachment","length":0} + """); } From 3c27ce89c00d3873972bdd40a8b59c5d3b09eea8 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Fri, 23 Oct 2026 22:28:54 +0200 Subject: [PATCH 3/4] fix: keep empty attachments read back from the cache Deserialization strips the length header, so keying the drop on an unknown length also dropped deliberately empty attachments after a cache round trip. Key it on the payload built from a stream of unknown length instead. Co-Authored-By: Claude --- src/Sentry/Protocol/Envelopes/EnvelopeItem.cs | 2 +- .../Envelopes/AttachmentSerializationTests.cs | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs b/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs index af4388e494..3b1e84f7a6 100644 --- a/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs +++ b/src/Sentry/Protocol/Envelopes/EnvelopeItem.cs @@ -242,7 +242,7 @@ internal bool Serialize(Stream stream, IDiagnosticLogger? logger, bool dropEmpty private bool IsEmptyAttachmentOfUnknownLength(MemoryStream payloadBuffer, IDiagnosticLogger? logger) { - if (payloadBuffer.Length != 0 || TryGetLength() is not null || TryGetType() != TypeValueAttachment) + if (payloadBuffer.Length != 0 || TryGetLength() is not null || Payload is not BufferedStreamSerializable) { return false; } diff --git a/test/Sentry.Tests/Protocol/Envelopes/AttachmentSerializationTests.cs b/test/Sentry.Tests/Protocol/Envelopes/AttachmentSerializationTests.cs index 41c3893998..dccc7a305a 100644 --- a/test/Sentry.Tests/Protocol/Envelopes/AttachmentSerializationTests.cs +++ b/test/Sentry.Tests/Protocol/Envelopes/AttachmentSerializationTests.cs @@ -424,6 +424,27 @@ public async Task Serialization_EmptyNonSeekableStreamAttachmentSynchronously_Dr .Which.TryGetType().Should().Be(EnvelopeItem.TypeValueEvent); } + [Fact] + public async Task Serialization_EmptyAttachmentOfKnownLengthAfterRoundTrip_KeepsAttachment() + { + // Arrange + var attachment = new SentryAttachment( + AttachmentType.Default, + new StreamAttachmentContent(new MemoryStream()), + "empty.log", + "text/plain"); + using var envelope = Envelope.FromAttachment(SentryId.Create(), attachment, _testOutputLogger); + using var cached = new MemoryStream(Encoding.UTF8.GetBytes( + await envelope.SerializeToStringAsync(_testOutputLogger, _fakeClock))); + using var deserialized = await Envelope.DeserializeAsync(cached); + + // Act + var serialized = await deserialized.SerializeToStringAsync(_testOutputLogger, _fakeClock); + + // Assert + serialized.Should().Contain("empty.log"); + } + private Envelope CreateEventEnvelopeWithEmptyNonSeekableAttachment() { var attachment = new SentryAttachment( From dac0cddd018002cd9969e5374c38ea1115e209ad Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Wed, 28 Oct 2026 12:20:29 +0100 Subject: [PATCH 4/4] test: assert dropped attachments leave no blank line in the envelope Relay's item parser rejects a trailing lone newline with UnexpectedEof, dropping the whole envelope, but Envelope.DeserializeAsync skips it, so no test caught Envelope writing the newline for a dropped item. Co-Authored-By: Claude --- .../Protocol/Envelopes/AttachmentSerializationTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Sentry.Tests/Protocol/Envelopes/AttachmentSerializationTests.cs b/test/Sentry.Tests/Protocol/Envelopes/AttachmentSerializationTests.cs index dccc7a305a..69b2d7229d 100644 --- a/test/Sentry.Tests/Protocol/Envelopes/AttachmentSerializationTests.cs +++ b/test/Sentry.Tests/Protocol/Envelopes/AttachmentSerializationTests.cs @@ -400,7 +400,7 @@ public async Task Serialization_EmptyNonSeekableStreamAttachment_DropsAttachment var serialized = await envelope.SerializeToStringAsync(_testOutputLogger, _fakeClock); // Assert - serialized.Should().NotContain("empty.jpg"); + serialized.Should().NotContain("empty.jpg").And.NotContain("\n\n"); using var stream = new MemoryStream(Encoding.UTF8.GetBytes(serialized)); using var deserialized = await Envelope.DeserializeAsync(stream); deserialized.Items.Should().ContainSingle() @@ -417,7 +417,7 @@ public async Task Serialization_EmptyNonSeekableStreamAttachmentSynchronously_Dr var serialized = envelope.SerializeToString(_testOutputLogger, _fakeClock); // Assert - serialized.Should().NotContain("empty.jpg"); + serialized.Should().NotContain("empty.jpg").And.NotContain("\n\n"); using var stream = new MemoryStream(Encoding.UTF8.GetBytes(serialized)); using var deserialized = await Envelope.DeserializeAsync(stream); deserialized.Items.Should().ContainSingle()