Fix possible NullReferenceException on a TransactionalBatch code path (…

…#1086) * Add missing null check on a Transactional batch code path; don't promote operation status for internal use * Add to changelog
Azure · Dec 10, 2019 · df298c4 · df298c4
1 parent 3fa0eba
commit df298c4
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 6 deletions.
diff --git a/Microsoft.Azure.Cosmos/src/Batch/TransactionalBatchResponse.cs b/Microsoft.Azure.Cosmos/src/Batch/TransactionalBatchResponse.cs
@@ -211,7 +211,8 @@ IEnumerator IEnumerable.GetEnumerator()
         internal static async Task<TransactionalBatchResponse> FromResponseMessageAsync(
             ResponseMessage responseMessage,
             ServerBatchRequest serverRequest,
-            CosmosSerializer serializer)
+            CosmosSerializer serializer,
+            bool shouldPromoteOperationStatus = true)
         {
             using (responseMessage)
             {
@@ -230,7 +231,13 @@ internal static async Task<TransactionalBatchResponse> FromResponseMessageAsync(
                     if (content.ReadByte() == (int)HybridRowVersion.V1)
                     {
                         content.Position = 0;
-                        response = await TransactionalBatchResponse.PopulateFromContentAsync(content, responseMessage, serverRequest, serializer);
+                        response = await TransactionalBatchResponse.PopulateFromContentAsync(
+                            content,
+                            responseMessage,
+                            serverRequest,
+                            serializer,
+                            shouldPromoteOperationStatus);
+
                         if (response == null)
                         {
                             // Convert any payload read failures as InternalServerError
@@ -247,7 +254,8 @@ internal static async Task<TransactionalBatchResponse> FromResponseMessageAsync(
                         }
                     }
                 }
-                else
+
+                if (response == null)
                 {
                     response = new TransactionalBatchResponse(
                         responseMessage.StatusCode,
@@ -317,7 +325,8 @@ private static async Task<TransactionalBatchResponse> PopulateFromContentAsync(
             Stream content,
             ResponseMessage responseMessage,
             ServerBatchRequest serverRequest,
-            CosmosSerializer serializer)
+            CosmosSerializer serializer,
+            bool shouldPromoteOperationStatus)
         {
             List<TransactionalBatchOperationResult> results = new List<TransactionalBatchOperationResult>();
 
@@ -348,7 +357,8 @@ record =>
 
             // Promote the operation error status as the Batch response error status if we have a MultiStatus response
             // to provide users with status codes they are used to.
-            if ((int)responseMessage.StatusCode == (int)StatusCodes.MultiStatus)
+            if ((int)responseMessage.StatusCode == (int)StatusCodes.MultiStatus
+                && shouldPromoteOperationStatus)
             {
                 foreach (TransactionalBatchOperationResult result in results)
                 {

diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchUnitTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/Batch/BatchUnitTests.cs
@@ -9,6 +9,7 @@ namespace Microsoft.Azure.Cosmos.Tests
     using System.IO;
     using System.Linq;
     using System.Net;
+    using System.Text;
     using System.Threading;
     using System.Threading.Tasks;
     using Microsoft.Azure.Cosmos.Client.Core.Tests;
@@ -338,6 +339,33 @@ public async Task BatchSingleServerResponseAsync()
             Assert.IsNull(batchResponse[1].ResourceStream);
         }
 
+        [TestMethod]
+        [Owner("abpai")]
+        public async Task BatchJsonServerResponseAsync()
+        {
+            TestHandler testHandler = new TestHandler((request, cancellationToken) =>
+            {
+                HttpStatusCode serverStatusCode = HttpStatusCode.Gone;
+                Stream serverContent = new MemoryStream(Encoding.UTF8.GetBytes("{ \"random\": 1 }"));
+
+                ResponseMessage responseMessage = new ResponseMessage(serverStatusCode, requestMessage: null, errorMessage: null)
+                {
+                    Content = serverContent
+                };
+
+                return Task.FromResult(responseMessage);
+            });
+
+            Container container = BatchUnitTests.GetContainer(testHandler);
+
+            TransactionalBatchResponse batchResponse = await new BatchCore((ContainerCore)container, new Cosmos.PartitionKey(BatchUnitTests.PartitionKey1))
+                .ReadItem("id1")
+                .ReadItem("id2")
+                .ExecuteAsync();
+
+            Assert.AreEqual(HttpStatusCode.Gone, batchResponse.StatusCode);
+        }
+
         /// <summary>
         /// Test to make sure IsFeedRequest is true for Batch operation
         /// </summary>

diff --git a/changelog.md b/changelog.md
@@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - [#1075](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/1075) Including header size details for BadRequest with large headers
-
+- [#1086](https://github.com/Azure/azure-cosmos-dotnet-v3/pull/1086) Fix possible NullReferenceException on a TransactionalBatch code path
 
 
 ### Fixed