From 04000d3f6f4829de3e71bc781f68a2141bab0134 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 23 Jul 2021 12:43:22 +0200 Subject: [PATCH] Fixed corruption issue related to the utf8 encoding that is not failing for invalid data. Encoding is now strict on encoding issues and if any encoding issues occur (likely due to a compressed or encrypted body) it falls back to body storage. --- .../BodyStorage/BodyStorageEnricherTests.cs | 28 +++++++++++++ .../BodyStorage/BodyStorageFeature.cs | 41 ++++++++++++++----- .../BodyStorage/BodyStorageEnricherTests.cs | 17 ++++++++ .../BodyStorage/BodyStorageEnricher.cs | 27 +++++++++--- 4 files changed, 96 insertions(+), 17 deletions(-) diff --git a/src/ServiceControl.Audit.UnitTests/BodyStorage/BodyStorageEnricherTests.cs b/src/ServiceControl.Audit.UnitTests/BodyStorage/BodyStorageEnricherTests.cs index 4b5b0f401c..3c924ce91e 100644 --- a/src/ServiceControl.Audit.UnitTests/BodyStorage/BodyStorageEnricherTests.cs +++ b/src/ServiceControl.Audit.UnitTests/BodyStorage/BodyStorageEnricherTests.cs @@ -236,6 +236,34 @@ public async Task Should_store_body_in_storage_when_below_threshold() Assert.AreEqual("/messages/someid/body", metadata["BodyUrl"]); } + [Test] + public async Task Should_store_body_in_storage_when_encoding_fails() + { + var fakeStorage = new FakeBodyStorage(); + var maxBodySizeToStore = 100000; + var settings = new Settings + { + MaxBodySizeToStore = maxBodySizeToStore, + EnableFullTextSearchOnBodies = true + }; + + var enricher = new BodyStorageFeature.BodyStorageEnricher(fakeStorage, settings); + var body = new byte[] { 0x00, 0xDE }; + var metadata = new Dictionary(); + + var headers = new Dictionary + { + { Headers.MessageId, "someid" }, + { "ServiceControl.Retry.UniqueMessageId", "someid" } + }; + + var message = new ProcessedMessage(headers, metadata); + + await enricher.StoreAuditMessageBody(body, message); + + Assert.IsTrue(fakeStorage.StoredBodySize > 0); + } + class FakeBodyStorage : IBodyStorage { public int StoredBodySize { get; set; } diff --git a/src/ServiceControl.Audit/Auditing/BodyStorage/BodyStorageFeature.cs b/src/ServiceControl.Audit/Auditing/BodyStorage/BodyStorageFeature.cs index 530bbd7a75..a1f6253fbf 100644 --- a/src/ServiceControl.Audit/Auditing/BodyStorage/BodyStorageFeature.cs +++ b/src/ServiceControl.Audit/Auditing/BodyStorage/BodyStorageFeature.cs @@ -1,5 +1,6 @@ namespace ServiceControl.Audit.Auditing.BodyStorage { + using System; using System.Collections.Generic; using System.Text; using System.Threading.Tasks; @@ -7,6 +8,7 @@ using Infrastructure.Settings; using NServiceBus; using NServiceBus.Features; + using NServiceBus.Logging; using RavenAttachments; class BodyStorageFeature : Feature @@ -73,23 +75,38 @@ async ValueTask TryStoreBody(byte[] body, ProcessedMessage processedMessag var isBelowMaxSize = bodySize <= settings.MaxBodySizeToStore; var avoidsLargeObjectHeap = bodySize < LargeObjectHeapThreshold; - if (isBelowMaxSize && avoidsLargeObjectHeap && !isBinary) + if (isBelowMaxSize) { - if (settings.EnableFullTextSearchOnBodies) + var useEmbeddedBody = avoidsLargeObjectHeap && !isBinary; + var useBodyStore = !useEmbeddedBody; + + if (useEmbeddedBody) { - processedMessage.MessageMetadata.Add("Body", Encoding.UTF8.GetString(body)); + try + { + if (settings.EnableFullTextSearchOnBodies) + { + processedMessage.MessageMetadata.Add("Body", enc.GetString(body)); + } + else + { + processedMessage.Body = enc.GetString(body); + } + } + catch (DecoderFallbackException e) + { + useBodyStore = true; + log.Info("Body for {bodyId} could not be stored embedded, fallback to body storage", e); + } } - else + + if (useBodyStore) { - processedMessage.Body = Encoding.UTF8.GetString(body); + await StoreBodyInBodyStorage(body, bodyId, contentType, bodySize) + .ConfigureAwait(false); + storedInBodyStorage = true; } } - else if (isBelowMaxSize) - { - await StoreBodyInBodyStorage(body, bodyId, contentType, bodySize) - .ConfigureAwait(false); - storedInBodyStorage = true; - } processedMessage.MessageMetadata.Add("BodyUrl", bodyUrl); return storedInBodyStorage; @@ -104,6 +121,8 @@ await bodyStorage.Store(bodyId, contentType, bodySize, bodyStream) } } + static readonly Encoding enc = new UTF8Encoding(true, true); + static readonly ILog log = LogManager.GetLogger(); IBodyStorage bodyStorage; Settings settings; diff --git a/src/ServiceControl.UnitTests/BodyStorage/BodyStorageEnricherTests.cs b/src/ServiceControl.UnitTests/BodyStorage/BodyStorageEnricherTests.cs index 349ada2ae8..83fdceb61a 100644 --- a/src/ServiceControl.UnitTests/BodyStorage/BodyStorageEnricherTests.cs +++ b/src/ServiceControl.UnitTests/BodyStorage/BodyStorageEnricherTests.cs @@ -101,6 +101,23 @@ public async Task Should_store_body_in_storage_when_not_binary_and_above_LOH_thr Assert.IsFalse(metadata.ContainsKey("Body")); } + [Test] + public async Task Should_store_body_in_storage_when_encoding_fails() + { + var fakeStorage = new FakeBodyStorage(); + var settings = new Settings(); + + var enricher = new BodyStorageEnricher(fakeStorage, settings); + var body = new byte[] { 0x00, 0xDE }; + var metadata = new Dictionary(); + + var attempt = new FailedMessage.ProcessingAttempt { MessageMetadata = metadata, Headers = new Dictionary() }; + + await enricher.StoreErrorMessageBody(body, attempt); + + Assert.IsTrue(fakeStorage.StoredBodySize > 0); + } + class FakeBodyStorage : IBodyStorage { public int StoredBodySize { get; set; } diff --git a/src/ServiceControl/Operations/BodyStorage/BodyStorageEnricher.cs b/src/ServiceControl/Operations/BodyStorage/BodyStorageEnricher.cs index 91042b45bf..4d8466f591 100644 --- a/src/ServiceControl/Operations/BodyStorage/BodyStorageEnricher.cs +++ b/src/ServiceControl/Operations/BodyStorage/BodyStorageEnricher.cs @@ -5,6 +5,7 @@ using System.Text; using System.Threading.Tasks; using NServiceBus; + using NServiceBus.Logging; using ServiceBus.Management.Infrastructure.Settings; using ProcessingAttempt = MessageFailures.FailedMessage.ProcessingAttempt; @@ -49,18 +50,30 @@ async ValueTask StoreBody(byte[] body, ProcessingAttempt processingAttempt, int var isBinary = contentType.Contains("binary"); var avoidsLargeObjectHeap = bodySize < LargeObjectHeapThreshold; - if (avoidsLargeObjectHeap && !isBinary) + var useEmbeddedBody = avoidsLargeObjectHeap && !isBinary; + var useBodyStore = !useEmbeddedBody; + + if (useEmbeddedBody) { - if (settings.EnableFullTextSearchOnBodies) + try { - processingAttempt.MessageMetadata.Add("Body", Encoding.UTF8.GetString(body)); + if (settings.EnableFullTextSearchOnBodies) + { + processingAttempt.MessageMetadata.Add("Body", enc.GetString(body)); + } + else + { + processingAttempt.Body = enc.GetString(body); + } } - else + catch (DecoderFallbackException e) { - processingAttempt.Body = Encoding.UTF8.GetString(body); + useBodyStore = true; + log.Info("Body for {bodyId} could not be stored embedded, fallback to body storage", e); } } - else + + if (useBodyStore) { await StoreBodyInBodyStorage(body, bodyId, contentType, bodySize) .ConfigureAwait(false); @@ -78,6 +91,8 @@ await bodyStorage.Store(bodyId, contentType, bodySize, bodyStream) } } + static readonly Encoding enc = new UTF8Encoding(true, true); + static readonly ILog log = LogManager.GetLogger(); readonly IBodyStorage bodyStorage; readonly Settings settings;