From a0447ab39c5b279c31c7abdad0a355b5235cd681 Mon Sep 17 00:00:00 2001 From: John Gathogo Date: Wed, 8 Nov 2023 21:00:21 +0300 Subject: [PATCH] Fix delta deleted entry deserialization failure where reason appears before id in payload (#2787) --- .../ODataJsonLightResourceDeserializer.cs | 115 ++++++++++-------- ...ODataJsonLightResourceDeserializerTests.cs | 85 ++++++++++++- 2 files changed, 145 insertions(+), 55 deletions(-) diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightResourceDeserializer.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightResourceDeserializer.cs index 3a1d6e9a8e..5bd70f3359 100644 --- a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightResourceDeserializer.cs +++ b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightResourceDeserializer.cs @@ -225,41 +225,46 @@ internal ODataDeletedResource ReadDeletedEntry() Uri id = null; DeltaDeletedEntryReason reason = DeltaDeletedEntryReason.Changed; - // If the current node is the id property - read it. - if (this.JsonReader.NodeType == JsonNodeType.Property && - string.Equals(JsonLightConstants.ODataIdPropertyName, this.JsonReader.GetPropertyName(), StringComparison.Ordinal)) + while (this.JsonReader.NodeType != JsonNodeType.EndObject && this.JsonReader.NodeType != JsonNodeType.EndOfInput) { - // Read over the property to move to its value. - this.JsonReader.Read(); + if (this.JsonReader.NodeType == JsonNodeType.Property) + { + // If the current node is the id property - read it. + if (string.Equals(JsonLightConstants.ODataIdPropertyName, this.JsonReader.GetPropertyName(), StringComparison.Ordinal)) + { + // Read over the property to move to its value. + this.JsonReader.Read(); - // Read the Id value. - id = this.JsonReader.ReadUriValue(); - Debug.Assert(id != null, "value for Id must be provided"); - } + // Read the Id value. + id = this.JsonReader.ReadUriValue(); + Debug.Assert(id != null, "value for Id must be provided"); - this.AssertJsonCondition(JsonNodeType.Property, JsonNodeType.EndObject); + continue; + } + // If the current node is the reason property - read it. + else if (string.Equals(JsonLightConstants.ODataReasonPropertyName, this.JsonReader.GetPropertyName(), StringComparison.Ordinal)) + { + // Read over the property to move to its value. + this.JsonReader.Read(); - // If the current node is the reason property - read it. - if (this.JsonReader.NodeType == JsonNodeType.Property && - string.Equals(JsonLightConstants.ODataReasonPropertyName, this.JsonReader.GetPropertyName(), StringComparison.Ordinal)) - { - // Read over the property to move to its value. - this.JsonReader.Read(); + // Read the reason value. + if (string.Equals(JsonLightConstants.ODataReasonDeletedValue, this.JsonReader.ReadStringValue(), StringComparison.Ordinal)) + { + reason = DeltaDeletedEntryReason.Deleted; + } - // Read the reason value. - if (string.Equals(JsonLightConstants.ODataReasonDeletedValue, this.JsonReader.ReadStringValue(), StringComparison.Ordinal)) - { - reason = DeltaDeletedEntryReason.Deleted; + continue; + } } - } - // Ignore unknown primitive properties in a 4.0 deleted entry - while (this.JsonReader.NodeType != JsonNodeType.EndObject && this.JsonReader.Read()) - { + this.JsonReader.Read(); + if (this.JsonReader.NodeType == JsonNodeType.StartObject || this.JsonReader.NodeType == JsonNodeType.StartArray) { throw new ODataException(Strings.ODataWriterCore_NestedContentNotAllowedIn40DeletedEntry); } + + // Ignore unknown primitive properties in a 4.0 deleted entry } return ReaderUtils.CreateDeletedResource(id, reason); @@ -2485,44 +2490,52 @@ internal async Task ReadDeletedEntryAsync() Uri id = null; DeltaDeletedEntryReason reason = DeltaDeletedEntryReason.Changed; - // If the current node is the id property - read it. - if (this.JsonReader.NodeType == JsonNodeType.Property && - string.Equals(JsonLightConstants.ODataIdPropertyName, await this.JsonReader.GetPropertyNameAsync().ConfigureAwait(false), StringComparison.Ordinal)) + while (this.JsonReader.NodeType != JsonNodeType.EndObject && this.JsonReader.NodeType != JsonNodeType.EndOfInput) { - // Read over the property to move to its value. - await this.JsonReader.ReadAsync() - .ConfigureAwait(false); + if (this.JsonReader.NodeType == JsonNodeType.Property) + { + string propertyName = await this.JsonReader.GetPropertyNameAsync() + .ConfigureAwait(false); - // Read the Id value. - id = await this.JsonReader.ReadUriValueAsync() - .ConfigureAwait(false); - Debug.Assert(id != null, "value for Id must be provided"); - } + // If the current node is the id property - read it. + if (string.Equals(JsonLightConstants.ODataIdPropertyName, propertyName, StringComparison.Ordinal)) + { + // Read over the property to move to its value. + await this.JsonReader.ReadAsync() + .ConfigureAwait(false); - this.AssertJsonCondition(JsonNodeType.Property, JsonNodeType.EndObject); + // Read the Id value. + id = await this.JsonReader.ReadUriValueAsync() + .ConfigureAwait(false); + Debug.Assert(id != null, "value for Id must be provided"); - // If the current node is the reason property - read it. - if (this.JsonReader.NodeType == JsonNodeType.Property && - string.Equals(JsonLightConstants.ODataReasonPropertyName, await this.JsonReader.GetPropertyNameAsync().ConfigureAwait(false), StringComparison.Ordinal)) - { - // Read over the property to move to its value. - await this.JsonReader.ReadAsync() - .ConfigureAwait(false); + continue; + } + // If the current node is the reason property - read it. + else if (string.Equals(JsonLightConstants.ODataReasonPropertyName, propertyName, StringComparison.Ordinal)) + { + // Read over the property to move to its value. + await this.JsonReader.ReadAsync() + .ConfigureAwait(false); - // Read the reason value. - if (string.Equals(JsonLightConstants.ODataReasonDeletedValue, await this.JsonReader.ReadStringValueAsync().ConfigureAwait(false), StringComparison.Ordinal)) - { - reason = DeltaDeletedEntryReason.Deleted; + // Read the reason value. + if (string.Equals(JsonLightConstants.ODataReasonDeletedValue, await this.JsonReader.ReadStringValueAsync().ConfigureAwait(false), StringComparison.Ordinal)) + { + reason = DeltaDeletedEntryReason.Deleted; + } + + continue; + } } - } - // Ignore unknown primitive properties in a 4.0 deleted entry - while (this.JsonReader.NodeType != JsonNodeType.EndObject && await this.JsonReader.ReadAsync().ConfigureAwait(false)) - { + await this.JsonReader.ReadAsync().ConfigureAwait(false); + if (this.JsonReader.NodeType == JsonNodeType.StartObject || this.JsonReader.NodeType == JsonNodeType.StartArray) { throw new ODataException(ODataErrorStrings.ODataWriterCore_NestedContentNotAllowedIn40DeletedEntry); } + + // Ignore unknown primitive properties in a 4.0 deleted entry } return ReaderUtils.CreateDeletedResource(id, reason); diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightResourceDeserializerTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightResourceDeserializerTests.cs index 2fd6456bd4..4cb6a20fb0 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightResourceDeserializerTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightResourceDeserializerTests.cs @@ -100,7 +100,9 @@ public async Task ReadDeletedResourceAsync(string payload, DeltaDeletedEntryReas [Theory] [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}", DeltaDeletedEntryReason.Deleted)] + [InlineData("{\"reason\":\"deleted\",\"id\":\"http://tempuri.org/Customers(1)\"}", DeltaDeletedEntryReason.Deleted)] [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"changed\"}", DeltaDeletedEntryReason.Changed)] + [InlineData("{\"reason\":\"changed\",\"id\":\"http://tempuri.org/Customers(1)\"}", DeltaDeletedEntryReason.Changed)] public async Task ReadDeletedEntryAsync(string payload, DeltaDeletedEntryReason deletedEntryReason) { using (var jsonInputContext = CreateJsonLightInputContext(payload, model)) @@ -116,11 +118,12 @@ public async Task ReadDeletedEntryAsync(string payload, DeltaDeletedEntryReason } } - [Fact] - public async Task ReadDeletedEntryAsync_IgnoresUnexpectedPrimitiveProperties() + [Theory] + [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\",\"unexpected\":true}")] + [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"unexpected\":true,\"reason\":\"deleted\"}")] + [InlineData("{\"unexpected\":true,\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}")] + public async Task ReadDeletedEntryAsync_IgnoresUnexpectedPrimitiveProperties(string payload) { - var payload = "{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\",\"unexpected\":true}"; - using (var jsonInputContext = CreateJsonLightInputContext(payload, model)) { var jsonLightResourceDeserializer = new ODataJsonLightResourceDeserializer(jsonInputContext); @@ -134,6 +137,45 @@ public async Task ReadDeletedEntryAsync_IgnoresUnexpectedPrimitiveProperties() } } + [Theory] + [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}", DeltaDeletedEntryReason.Deleted)] + [InlineData("{\"reason\":\"deleted\",\"id\":\"http://tempuri.org/Customers(1)\"}", DeltaDeletedEntryReason.Deleted)] + [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"changed\"}", DeltaDeletedEntryReason.Changed)] + [InlineData("{\"reason\":\"changed\",\"id\":\"http://tempuri.org/Customers(1)\"}", DeltaDeletedEntryReason.Changed)] + public void ReadDeletedEntry(string payload, DeltaDeletedEntryReason deletedEntryReason) + { + using (var jsonInputContext = CreateJsonLightInputContext(payload: payload, model: model, isAsync: false)) + { + var jsonLightResourceDeserializer = new ODataJsonLightResourceDeserializer(jsonInputContext); + + AdvanceReaderToFirstProperty(jsonLightResourceDeserializer.JsonReader); + + var deletedResource = jsonLightResourceDeserializer.ReadDeletedEntry(); + + Assert.Equal(deletedEntryReason, deletedResource.Reason); + Assert.Equal("http://tempuri.org/Customers(1)", deletedResource.Id.AbsoluteUri); + } + } + + [Theory] + [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\",\"unexpected\":true}")] + [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"unexpected\":true,\"reason\":\"deleted\"}")] + [InlineData("{\"unexpected\":true,\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}")] + public void ReadDeletedEntry_IgnoresUnexpectedPrimitiveProperties(string payload) + { + using (var jsonInputContext = CreateJsonLightInputContext(payload: payload, model: model, isAsync: false)) + { + var jsonLightResourceDeserializer = new ODataJsonLightResourceDeserializer(jsonInputContext); + + AdvanceReaderToFirstProperty(jsonLightResourceDeserializer.JsonReader); + + var deletedResource = jsonLightResourceDeserializer.ReadDeletedEntry(); + + Assert.Equal(DeltaDeletedEntryReason.Deleted, deletedResource.Reason); + Assert.Equal("http://tempuri.org/Customers(1)", deletedResource.Id.AbsoluteUri); + } + } + [Fact] public async Task ReadDeltaLinkAsync() { @@ -1732,7 +1774,11 @@ public async Task ReadDeletedResourceAsync_ThrowsExceptionForODataRemovedAnnotat [Theory] [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\",\"nested\":{\"foo\":\"bar\"}}")] + [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"nested\":{\"foo\":\"bar\"},\"reason\":\"deleted\"}")] + [InlineData("{\"nested\":{\"foo\":\"bar\"},\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}")] [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\",\"nested\":[{\"foo\":\"bar\"}]}")] + [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"nested\":[{\"foo\":\"bar\"}],\"reason\":\"deleted\"}")] + [InlineData("{\"nested\":[{\"foo\":\"bar\"}],\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}")] public async Task ReadDeletedEntryAsync_ThrowsExceptionForNestedContent(string payload) { using (var jsonInputContext = CreateJsonLightInputContext(payload, model)) @@ -1750,6 +1796,30 @@ public async Task ReadDeletedEntryAsync_ThrowsExceptionForNestedContent(string p } } + [Theory] + [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\",\"nested\":{\"foo\":\"bar\"}}")] + [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"nested\":{\"foo\":\"bar\"},\"reason\":\"deleted\"}")] + [InlineData("{\"nested\":{\"foo\":\"bar\"},\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}")] + [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\",\"nested\":[{\"foo\":\"bar\"}]}")] + [InlineData("{\"id\":\"http://tempuri.org/Customers(1)\",\"nested\":[{\"foo\":\"bar\"}],\"reason\":\"deleted\"}")] + [InlineData("{\"nested\":[{\"foo\":\"bar\"}],\"id\":\"http://tempuri.org/Customers(1)\",\"reason\":\"deleted\"}")] + public void ReadDeletedEntry_ThrowsExceptionForNestedContent(string payload) + { + using (var jsonInputContext = CreateJsonLightInputContext(payload: payload, model: model, isAsync: false)) + { + var jsonLightResourceDeserializer = new ODataJsonLightResourceDeserializer(jsonInputContext); + + AdvanceReaderToFirstProperty(jsonLightResourceDeserializer.JsonReader); + + var exception = Assert.Throws( + () => jsonLightResourceDeserializer.ReadDeletedEntry()); + + Assert.Equal( + ErrorStrings.ODataWriterCore_NestedContentNotAllowedIn40DeletedEntry, + exception.Message); + } + } + [Fact] public async Task ReadNextLinkAnnotationAtResourceSetEndAsync_ThrowsExceptionForDuplicateODataNextLinkAnnotation() { @@ -1947,6 +2017,13 @@ private static async Task AdvanceReaderToFirstPropertyAsync(BufferingJsonReader Assert.Equal(JsonNodeType.Property, bufferingJsonReader.NodeType); } + private static void AdvanceReaderToFirstProperty(BufferingJsonReader bufferingJsonReader) + { + bufferingJsonReader.Read(); // Position the reader on the first node + bufferingJsonReader.Read(); // Read StartObject + Assert.Equal(JsonNodeType.Property, bufferingJsonReader.NodeType); + } + private void InitializeModel() { this.model = new EdmModel();