Fixed corruption issue related to the utf8 encoding that is not faili…

…ng 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.
Particular · Jul 26, 2021 · 04000d3 · 04000d3
1 parent b2efd3e
commit 04000d3
Show file tree

Hide file tree

Showing 4 changed files with 96 additions and 17 deletions.
diff --git 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<string, object>();
+
+            var headers = new Dictionary<string, string>
+            {
+                { 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
@@ -1,12 +1,14 @@
 namespace ServiceControl.Audit.Auditing.BodyStorage
 {
+    using System;
     using System.Collections.Generic;
     using System.Text;
     using System.Threading.Tasks;
     using Infrastructure;
     using Infrastructure.Settings;
     using NServiceBus;
     using NServiceBus.Features;
+    using NServiceBus.Logging;
     using RavenAttachments;
 
     class BodyStorageFeature : Feature
@@ -73,23 +75,38 @@ async ValueTask<bool> 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<BodyStorageFeature>();
             IBodyStorage bodyStorage;
             Settings settings;
 

diff --git 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<string, object>();
+
+            var attempt = new FailedMessage.ProcessingAttempt { MessageMetadata = metadata, Headers = new Dictionary<string, string>() };
+
+            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
@@ -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<BodyStorageEnricher>();
         readonly IBodyStorage bodyStorage;
         readonly Settings settings;