From 27682034d3ac54059aaeb78553b75b55385b6cc2 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Tue, 26 Jul 2022 10:53:53 -0700 Subject: [PATCH 1/4] Make sure we never have two sent_at headers --- src/Sentry/Envelopes/Envelope.cs | 7 ++++- .../Internal/Extensions/JsonExtensions.cs | 4 +-- test/Sentry.Testing/MockClock.cs | 4 ++- .../Protocol/Envelopes/EnvelopeTests.cs | 29 ++++++++++++++++++- 4 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/Sentry/Envelopes/Envelope.cs b/src/Sentry/Envelopes/Envelope.cs index 2079d16578..834a8b8c01 100644 --- a/src/Sentry/Envelopes/Envelope.cs +++ b/src/Sentry/Envelopes/Envelope.cs @@ -270,9 +270,14 @@ internal static Envelope FromClientReport(ClientReport clientReport) prevByte = curByte; } - return + var header = Json.Parse(buffer.ToArray(), JsonExtensions.GetDictionaryOrNull) ?? throw new InvalidOperationException("Envelope header is malformed."); + + // The sent_at header should not be included in the result + header.Remove(SentAtKey); + + return header; } /// diff --git a/src/Sentry/Internal/Extensions/JsonExtensions.cs b/src/Sentry/Internal/Extensions/JsonExtensions.cs index 802f780012..ed0b2b3728 100644 --- a/src/Sentry/Internal/Extensions/JsonExtensions.cs +++ b/src/Sentry/Internal/Extensions/JsonExtensions.cs @@ -29,7 +29,7 @@ public static void Deconstruct(this JsonProperty jsonProperty, out string name, value = jsonProperty.Value; } - public static IReadOnlyDictionary? GetDictionaryOrNull(this JsonElement json) + public static Dictionary? GetDictionaryOrNull(this JsonElement json) { if (json.ValueKind != JsonValueKind.Object) { @@ -46,7 +46,7 @@ public static void Deconstruct(this JsonProperty jsonProperty, out string name, return result; } - public static IReadOnlyDictionary? GetStringDictionaryOrNull(this JsonElement json) + public static Dictionary? GetStringDictionaryOrNull(this JsonElement json) { if (json.ValueKind != JsonValueKind.Object) { diff --git a/test/Sentry.Testing/MockClock.cs b/test/Sentry.Testing/MockClock.cs index eb66bcfbde..2cac4f6d29 100644 --- a/test/Sentry.Testing/MockClock.cs +++ b/test/Sentry.Testing/MockClock.cs @@ -2,7 +2,7 @@ public class MockClock : ISystemClock { - private readonly DateTimeOffset _value; + private DateTimeOffset _value; public MockClock(DateTimeOffset value) => _value = value; @@ -11,4 +11,6 @@ public MockClock() : this(DateTimeOffset.MaxValue) } public DateTimeOffset GetUtcNow() => _value; + + public void SetUtcNow(DateTimeOffset value) => _value = value; } diff --git a/test/Sentry.Tests/Protocol/Envelopes/EnvelopeTests.cs b/test/Sentry.Tests/Protocol/Envelopes/EnvelopeTests.cs index bd5b0aa214..46f0620d4b 100644 --- a/test/Sentry.Tests/Protocol/Envelopes/EnvelopeTests.cs +++ b/test/Sentry.Tests/Protocol/Envelopes/EnvelopeTests.cs @@ -8,7 +8,7 @@ public class EnvelopeTests // https://develop.sentry.dev/sdk/envelopes/#full-examples private readonly IDiagnosticLogger _testOutputLogger; - private readonly ISystemClock _fakeClock; + private readonly MockClock _fakeClock; public EnvelopeTests(ITestOutputHelper output) { @@ -698,4 +698,31 @@ value is IReadOnlyDictionary nested && nested["version"] == SdkVersion.Instance.Version; }).Should().BeTrue(); } + + [Fact] + public async Task Serialization_RoundTrip_ReplacesSentAtHeader() + { + // Arrange + using var envelope = new Envelope( + new Dictionary { ["event_id"] = "12c2d058d58442709aa2eca08bf20986" }, + Array.Empty()); + + // Act + _fakeClock.SetUtcNow(DateTimeOffset.MinValue); + var serialized = await envelope.SerializeToStringAsync(_testOutputLogger, _fakeClock); + + using var stream = new MemoryStream(); + using var writer = new StreamWriter(stream); + await writer.WriteAsync(serialized); + await writer.FlushAsync(); + stream.Seek(0, SeekOrigin.Begin); + var deserialized = await Envelope.DeserializeAsync(stream); + + _fakeClock.SetUtcNow(DateTimeOffset.MaxValue); + var output = await deserialized.SerializeToStringAsync(_testOutputLogger, _fakeClock); + + // Assert + output.Should().Be( + "{\"event_id\":\"12c2d058d58442709aa2eca08bf20986\",\"sent_at\":\"9999-12-31T23:59:59.9999999+00:00\"}\n"); + } } From 1ce88feb1868f0a459993ca7191e85874968958a Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Tue, 26 Jul 2022 10:54:12 -0700 Subject: [PATCH 2/4] Avoid writing sent_at header to cache file --- src/Sentry/Envelopes/Envelope.cs | 14 +++++--- .../Internal/Extensions/StreamExtensions.cs | 3 ++ .../Internals/Http/CachingTransportTests.cs | 32 +++++++++++++++++++ 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/Sentry/Envelopes/Envelope.cs b/src/Sentry/Envelopes/Envelope.cs index 834a8b8c01..8d23891ed2 100644 --- a/src/Sentry/Envelopes/Envelope.cs +++ b/src/Sentry/Envelopes/Envelope.cs @@ -56,7 +56,11 @@ private async Task SerializeHeaderAsync( ISystemClock clock, CancellationToken cancellationToken) { - var headerItems = Header.Append(SentAtKey, clock.GetUtcNow()); + // Append the sent_at header, except when writing to disk + var headerItems = !stream.IsFileStream() + ? Header.Append(SentAtKey, clock.GetUtcNow()) + : Header; + var writer = new Utf8JsonWriter(stream); #if NET461 || NETSTANDARD2_0 @@ -72,14 +76,16 @@ private async Task SerializeHeaderAsync( private void SerializeHeader(Stream stream, IDiagnosticLogger? logger, ISystemClock clock) { - var headerItems = Header.Append(SentAtKey, clock.GetUtcNow()); + // Append the sent_at header, except when writing to disk + var headerItems = !stream.IsFileStream() + ? Header.Append(SentAtKey, clock.GetUtcNow()) + : Header; + using var writer = new Utf8JsonWriter(stream); writer.WriteDictionaryValue(headerItems, logger); writer.Flush(); } - // Gets the header and adds a sent_at timestamp - /// public Task SerializeAsync( Stream stream, diff --git a/src/Sentry/Internal/Extensions/StreamExtensions.cs b/src/Sentry/Internal/Extensions/StreamExtensions.cs index 0dfb40ec7b..ccaf770fbe 100644 --- a/src/Sentry/Internal/Extensions/StreamExtensions.cs +++ b/src/Sentry/Internal/Extensions/StreamExtensions.cs @@ -60,5 +60,8 @@ public static async Task WriteByteAsync( return null; } } + + public static bool IsFileStream(this Stream? stream) => + stream is FileStream || stream?.GetType().Name == "MockFileStream"; } } diff --git a/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs b/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs index e61bdc7635..b117f1b35c 100644 --- a/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs +++ b/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs @@ -1,3 +1,4 @@ +using System.IO.Abstractions.TestingHelpers; using System.Net; using System.Net.Http; using System.Net.Sockets; @@ -677,4 +678,35 @@ public async Task TransportResumesWhenNetworkComesBackOnline() var envelopes = innerTransport.GetSentEnvelopes(); envelopes.Should().NotBeEmpty(); } + + [Fact] + public async Task DoesntWriteSentAtHeaderToCacheFile() + { + // Arrange + using var cacheDirectory = new TempDirectory(_fileSystem); + var options = new SentryOptions + { + Dsn = ValidDsn, + DiagnosticLogger = _logger, + Debug = true, + CacheDirectoryPath = cacheDirectory.Path, + FileSystem = _fileSystem + }; + + var innerTransport = Substitute.For(); + await using var transport = CachingTransport.Create(innerTransport, options, startWorker: false); + + using var envelope = Envelope.FromEvent(new SentryEvent()); + + // Act + await transport.SendEnvelopeAsync(envelope); + + // Assert + var filePath = _fileSystem + .EnumerateFiles(cacheDirectory.Path, "*", SearchOption.AllDirectories) + .Single(); + + var contents = _fileSystem.ReadAllTextFromFile(filePath); + Assert.DoesNotContain("sent_at", contents); + } } From 7c293fa8a5a6af04d601b9d37c208ac8de3d6ee6 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Tue, 26 Jul 2022 11:06:00 -0700 Subject: [PATCH 3/4] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35e2c6709c..e888ff1e8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,9 @@ ### Fixes +- URGENT: Fix events rejected due to duplicate `sent_at` header ([#1818](https://github.com/getsentry/sentry-dotnet/pull/1818)) - Fix null ref in aspnet TryGetTraceHeader ([#1807](https://github.com/getsentry/sentry-dotnet/pull/1807)) - ## 3.20.0 ### Features From 351098957c0da6b994e42d897f9ef82de3a9b40d Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Tue, 26 Jul 2022 11:13:39 -0700 Subject: [PATCH 4/4] Update CHANGELOG.md Co-authored-by: Bruno Garcia --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e888ff1e8b..6541346913 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixes -- URGENT: Fix events rejected due to duplicate `sent_at` header ([#1818](https://github.com/getsentry/sentry-dotnet/pull/1818)) +- URGENT: Fix events rejected due to duplicate `sent_at` header when offline caching is enabled through `CacheDirectoryPath` ([#1818](https://github.com/getsentry/sentry-dotnet/pull/1818)) - Fix null ref in aspnet TryGetTraceHeader ([#1807](https://github.com/getsentry/sentry-dotnet/pull/1807)) ## 3.20.0