From 6513d075d6aaddd058cd6951b25d8215d33c32d0 Mon Sep 17 00:00:00 2001 From: Michael Fry Date: Wed, 13 Nov 2019 16:25:49 +1300 Subject: [PATCH 1/2] Recursively denormalize references if entry point is 'serialize without references' --- src/Microsoft.OpenApi/Models/OpenApiSchema.cs | 81 ++++--- .../Models/OpenApiComponentsTests.cs | 19 +- .../Models/OpenApiSchemaTests.cs | 197 ++++++++++++++++++ 3 files changed, 270 insertions(+), 27 deletions(-) diff --git a/src/Microsoft.OpenApi/Models/OpenApiSchema.cs b/src/Microsoft.OpenApi/Models/OpenApiSchema.cs index 008f4ae21..f236ae8db 100644 --- a/src/Microsoft.OpenApi/Models/OpenApiSchema.cs +++ b/src/Microsoft.OpenApi/Models/OpenApiSchema.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. +using System; using System.Collections.Generic; using Microsoft.OpenApi.Any; using Microsoft.OpenApi.Interfaces; @@ -257,7 +258,13 @@ public void SerializeAsV3(IOpenApiWriter writer) return; } - SerializeAsV3WithoutReference(writer); + writer.WriteStartObject(); + + WriteSchemaPropertiesAsV3( + writer, + (w, s) => s.SerializeAsV3(w)); + + writer.WriteEndObject(); } /// @@ -267,6 +274,17 @@ public void SerializeAsV3WithoutReference(IOpenApiWriter writer) { writer.WriteStartObject(); + WriteSchemaPropertiesAsV3( + writer, + (w, s) => s.SerializeAsV3WithoutReference(w)); + + writer.WriteEndObject(); + } + + private void WriteSchemaPropertiesAsV3( + IOpenApiWriter writer, + Action serializeSchema) + { // title writer.WriteProperty(OpenApiConstants.Title, Title); @@ -319,22 +337,22 @@ public void SerializeAsV3WithoutReference(IOpenApiWriter writer) writer.WriteProperty(OpenApiConstants.Type, Type); // allOf - writer.WriteOptionalCollection(OpenApiConstants.AllOf, AllOf, (w, s) => s.SerializeAsV3(w)); + writer.WriteOptionalCollection(OpenApiConstants.AllOf, AllOf, serializeSchema); // anyOf - writer.WriteOptionalCollection(OpenApiConstants.AnyOf, AnyOf, (w, s) => s.SerializeAsV3(w)); + writer.WriteOptionalCollection(OpenApiConstants.AnyOf, AnyOf, serializeSchema); // oneOf - writer.WriteOptionalCollection(OpenApiConstants.OneOf, OneOf, (w, s) => s.SerializeAsV3(w)); + writer.WriteOptionalCollection(OpenApiConstants.OneOf, OneOf, serializeSchema); // not - writer.WriteOptionalObject(OpenApiConstants.Not, Not, (w, s) => s.SerializeAsV3(w)); + writer.WriteOptionalObject(OpenApiConstants.Not, Not, serializeSchema); // items - writer.WriteOptionalObject(OpenApiConstants.Items, Items, (w, s) => s.SerializeAsV3(w)); + writer.WriteOptionalObject(OpenApiConstants.Items, Items, serializeSchema); // properties - writer.WriteOptionalMap(OpenApiConstants.Properties, Properties, (w, s) => s.SerializeAsV3(w)); + writer.WriteOptionalMap(OpenApiConstants.Properties, Properties, serializeSchema); // additionalProperties if (AdditionalPropertiesAllowed) @@ -342,7 +360,7 @@ public void SerializeAsV3WithoutReference(IOpenApiWriter writer) writer.WriteOptionalObject( OpenApiConstants.AdditionalProperties, AdditionalProperties, - (w, s) => s.SerializeAsV3(w)); + serializeSchema); } else { @@ -362,7 +380,7 @@ public void SerializeAsV3WithoutReference(IOpenApiWriter writer) writer.WriteProperty(OpenApiConstants.Nullable, Nullable, false); // discriminator - writer.WriteOptionalObject(OpenApiConstants.Discriminator, Discriminator, (w, s) => s.SerializeAsV3(w)); + writer.WriteOptionalObject(OpenApiConstants.Discriminator, Discriminator, (w, d) => d.SerializeAsV3(w)); // readOnly writer.WriteProperty(OpenApiConstants.ReadOnly, ReadOnly, false); @@ -371,10 +389,10 @@ public void SerializeAsV3WithoutReference(IOpenApiWriter writer) writer.WriteProperty(OpenApiConstants.WriteOnly, WriteOnly, false); // xml - writer.WriteOptionalObject(OpenApiConstants.Xml, Xml, (w, s) => s.SerializeAsV2(w)); + writer.WriteOptionalObject(OpenApiConstants.Xml, Xml, (w, x) => x.SerializeAsV2(w)); // externalDocs - writer.WriteOptionalObject(OpenApiConstants.ExternalDocs, ExternalDocs, (w, s) => s.SerializeAsV3(w)); + writer.WriteOptionalObject(OpenApiConstants.ExternalDocs, ExternalDocs, (w, ed) => ed.SerializeAsV3(w)); // example writer.WriteOptionalObject(OpenApiConstants.Example, Example, (w, e) => w.WriteAny(e)); @@ -384,8 +402,6 @@ public void SerializeAsV3WithoutReference(IOpenApiWriter writer) // extensions writer.WriteExtensions(Extensions, OpenApiSpecVersion.OpenApi3_0); - - writer.WriteEndObject(); } /// @@ -435,7 +451,16 @@ internal void SerializeAsV2( parentRequiredProperties = new HashSet(); } - SerializeAsV2WithoutReference(writer, parentRequiredProperties, propertyName); + writer.WriteStartObject(); + + WriteSchemaPropertiesAsV2( + writer, + parentRequiredProperties, + propertyName, + (w, s) => s.SerializeAsV2(w), + (w, key, s) => s.SerializeAsV2(w, Required, key)); + + writer.WriteEndObject(); } /// @@ -451,7 +476,14 @@ internal void SerializeAsV2WithoutReference( string propertyName) { writer.WriteStartObject(); - WriteAsSchemaProperties(writer, parentRequiredProperties, propertyName); + + WriteSchemaPropertiesAsV2( + writer, + parentRequiredProperties, + propertyName, + (w, s) => s.SerializeAsV2WithoutReference(w), + (w, key, s) => s.SerializeAsV2WithoutReference(w, Required, key)); + writer.WriteEndObject(); } @@ -519,10 +551,12 @@ internal void WriteAsItemsProperties(IOpenApiWriter writer) writer.WriteExtensions(Extensions, OpenApiSpecVersion.OpenApi2_0); } - internal void WriteAsSchemaProperties( + private void WriteSchemaPropertiesAsV2( IOpenApiWriter writer, ISet parentRequiredProperties, - string propertyName) + string propertyName, + Action serializeSchema, + Action serializeSchemaProperty) { if (writer == null) { @@ -590,20 +624,19 @@ internal void WriteAsSchemaProperties( writer.WriteProperty(OpenApiConstants.Type, Type); // items - writer.WriteOptionalObject(OpenApiConstants.Items, Items, (w, s) => s.SerializeAsV2(w)); + writer.WriteOptionalObject(OpenApiConstants.Items, Items, serializeSchema); // allOf - writer.WriteOptionalCollection(OpenApiConstants.AllOf, AllOf, (w, s) => s.SerializeAsV2(w)); + writer.WriteOptionalCollection(OpenApiConstants.AllOf, AllOf, serializeSchema); // properties - writer.WriteOptionalMap(OpenApiConstants.Properties, Properties, (w, key, s) => - s.SerializeAsV2(w, Required, key)); + writer.WriteOptionalMap(OpenApiConstants.Properties, Properties, serializeSchemaProperty); // additionalProperties writer.WriteOptionalObject( OpenApiConstants.AdditionalProperties, AdditionalProperties, - (w, s) => s.SerializeAsV2(w)); + serializeSchema); // discriminator writer.WriteProperty(OpenApiConstants.Discriminator, Discriminator?.PropertyName); @@ -617,10 +650,10 @@ internal void WriteAsSchemaProperties( } // xml - writer.WriteOptionalObject(OpenApiConstants.Xml, Xml, (w, s) => s.SerializeAsV2(w)); + writer.WriteOptionalObject(OpenApiConstants.Xml, Xml, (w, x) => x.SerializeAsV2(w)); // externalDocs - writer.WriteOptionalObject(OpenApiConstants.ExternalDocs, ExternalDocs, (w, s) => s.SerializeAsV2(w)); + writer.WriteOptionalObject(OpenApiConstants.ExternalDocs, ExternalDocs, (w, ed) => ed.SerializeAsV2(w)); // example writer.WriteOptionalObject(OpenApiConstants.Example, Example, (w, e) => w.WriteAny(e)); diff --git a/test/Microsoft.OpenApi.Tests/Models/OpenApiComponentsTests.cs b/test/Microsoft.OpenApi.Tests/Models/OpenApiComponentsTests.cs index b06866087..38b393aca 100644 --- a/test/Microsoft.OpenApi.Tests/Models/OpenApiComponentsTests.cs +++ b/test/Microsoft.OpenApi.Tests/Models/OpenApiComponentsTests.cs @@ -81,6 +81,13 @@ public class OpenApiComponentsTests { Type = ReferenceType.Schema, Id = "schema2" + }, + Properties = new Dictionary + { + ["property2"] = new OpenApiSchema + { + Type = "integer" + } } } }, @@ -99,7 +106,7 @@ public class OpenApiComponentsTests Type = "integer" } } - }, + } }, SecuritySchemes = new Dictionary { @@ -343,7 +350,11 @@ public void SerializeAdvancedComponentsWithReferenceAsJsonV3Works() ""type"": ""integer"" }, ""property3"": { - ""$ref"": ""#/components/schemas/schema2"" + ""properties"": { + ""property2"": { + ""type"": ""integer"" + } + } } } }, @@ -432,7 +443,9 @@ public void SerializeAdvancedComponentsWithReferenceAsYamlV3Works() property2: type: integer property3: - $ref: '#/components/schemas/schema2' + properties: + property2: + type: integer schema2: properties: property2: diff --git a/test/Microsoft.OpenApi.Tests/Models/OpenApiSchemaTests.cs b/test/Microsoft.OpenApi.Tests/Models/OpenApiSchemaTests.cs index 2015985e4..1939c15e5 100644 --- a/test/Microsoft.OpenApi.Tests/Models/OpenApiSchemaTests.cs +++ b/test/Microsoft.OpenApi.Tests/Models/OpenApiSchemaTests.cs @@ -161,6 +161,30 @@ public class OpenApiSchemaTests } }; + public static OpenApiSchema NestedReferenceSchema = new OpenApiSchema + { + Title = "title1", + Type = "object", + Properties = new Dictionary + { + ["property1"] = new OpenApiSchema + { + Reference = new OpenApiReference + { + Type = ReferenceType.Schema, + Id = "schemaObject1" + }, + Properties = new Dictionary + { + ["nestedProperty1"] = new OpenApiSchema + { + Type = "integer" + } + } + } + } + }; + public static OpenApiSchema AdvancedSchemaWithRequiredPropertiesObject = new OpenApiSchema { Title = "title1", @@ -397,6 +421,101 @@ public void SerializeReferencedSchemaAsV3WithoutReferenceJsonWorks() actual.Should().Be(expected); } + [Fact] + public void SerializeReferencedSchemaAsV2WithoutReferenceJsonWorks() + { + // Arrange + var outputStringWriter = new StringWriter(CultureInfo.InvariantCulture); + var writer = new OpenApiJsonWriter(outputStringWriter); + + var expected = @"{ + ""title"": ""title1"", + ""default"": 15, + ""multipleOf"": 3, + ""maximum"": 42, + ""minimum"": 10, + ""exclusiveMinimum"": true, + ""type"": ""integer"", + ""externalDocs"": { + ""url"": ""http://example.com/externalDocs"" + } +}"; + + // Act + ReferencedSchema.SerializeAsV2WithoutReference(writer); + writer.Flush(); + var actual = outputStringWriter.GetStringBuilder().ToString(); + + // Assert + actual = actual.MakeLineBreaksEnvironmentNeutral(); + expected = expected.MakeLineBreaksEnvironmentNeutral(); + actual.Should().Be(expected); + } + + [Fact] + public void SerializeNestedReferencedSchemaAsV3WithoutReferenceJsonWorks() + { + // Arrange + var outputStringWriter = new StringWriter(CultureInfo.InvariantCulture); + var writer = new OpenApiJsonWriter(outputStringWriter); + + var expected = @"{ + ""title"": ""title1"", + ""type"": ""object"", + ""properties"": { + ""property1"": { + ""properties"": { + ""nestedProperty1"": { + ""type"": ""integer"" + } + } + } + } +}"; + + // Act + NestedReferenceSchema.SerializeAsV3WithoutReference(writer); + writer.Flush(); + var actual = outputStringWriter.GetStringBuilder().ToString(); + + // Assert + actual = actual.MakeLineBreaksEnvironmentNeutral(); + expected = expected.MakeLineBreaksEnvironmentNeutral(); + actual.Should().Be(expected); + } + + [Fact] + public void SerializeNestedReferencedSchemaAsV2WithoutReferenceJsonWorks() + { + // Arrange + var outputStringWriter = new StringWriter(CultureInfo.InvariantCulture); + var writer = new OpenApiJsonWriter(outputStringWriter); + + var expected = @"{ + ""title"": ""title1"", + ""type"": ""object"", + ""properties"": { + ""property1"": { + ""properties"": { + ""nestedProperty1"": { + ""type"": ""integer"" + } + } + } + } +}"; + + // Act + NestedReferenceSchema.SerializeAsV2WithoutReference(writer); + writer.Flush(); + var actual = outputStringWriter.GetStringBuilder().ToString(); + + // Assert + actual = actual.MakeLineBreaksEnvironmentNeutral(); + expected = expected.MakeLineBreaksEnvironmentNeutral(); + actual.Should().Be(expected); + } + [Fact] public void SerializeReferencedSchemaAsV3JsonWorks() { @@ -419,6 +538,84 @@ public void SerializeReferencedSchemaAsV3JsonWorks() actual.Should().Be(expected); } + [Fact] + public void SerializeReferencedSchemaAsV2JsonWorks() + { + // Arrange + var outputStringWriter = new StringWriter(CultureInfo.InvariantCulture); + var writer = new OpenApiJsonWriter(outputStringWriter); + + var expected = @"{ + ""$ref"": ""#/definitions/schemaObject1"" +}"; + + // Act + ReferencedSchema.SerializeAsV2(writer); + writer.Flush(); + var actual = outputStringWriter.GetStringBuilder().ToString(); + + // Assert + actual = actual.MakeLineBreaksEnvironmentNeutral(); + expected = expected.MakeLineBreaksEnvironmentNeutral(); + actual.Should().Be(expected); + } + + [Fact] + public void SerializeNestedReferencedSchemaAsV3JsonWorks() + { + // Arrange + var outputStringWriter = new StringWriter(CultureInfo.InvariantCulture); + var writer = new OpenApiJsonWriter(outputStringWriter); + + var expected = @"{ + ""title"": ""title1"", + ""type"": ""object"", + ""properties"": { + ""property1"": { + ""$ref"": ""#/components/schemas/schemaObject1"" + } + } +}"; + + // Act + NestedReferenceSchema.SerializeAsV3(writer); + writer.Flush(); + var actual = outputStringWriter.GetStringBuilder().ToString(); + + // Assert + actual = actual.MakeLineBreaksEnvironmentNeutral(); + expected = expected.MakeLineBreaksEnvironmentNeutral(); + actual.Should().Be(expected); + } + + [Fact] + public void SerializeNestedReferencedSchemaAsV2JsonWorks() + { + // Arrange + var outputStringWriter = new StringWriter(CultureInfo.InvariantCulture); + var writer = new OpenApiJsonWriter(outputStringWriter); + + var expected = @"{ + ""title"": ""title1"", + ""type"": ""object"", + ""properties"": { + ""property1"": { + ""$ref"": ""#/definitions/schemaObject1"" + } + } +}"; + + // Act + NestedReferenceSchema.SerializeAsV2(writer); + writer.Flush(); + var actual = outputStringWriter.GetStringBuilder().ToString(); + + // Assert + actual = actual.MakeLineBreaksEnvironmentNeutral(); + expected = expected.MakeLineBreaksEnvironmentNeutral(); + actual.Should().Be(expected); + } + [Fact] public void SerializeSchemaWRequiredPropertiesAsV2JsonWorks() { From f72eaf92af4f202a3dbabcebbe7baf6e6766498c Mon Sep 17 00:00:00 2001 From: Michael Fry Date: Fri, 15 Nov 2019 12:20:40 +1300 Subject: [PATCH 2/2] Defend against stack overflow in the minimal way --- src/Microsoft.OpenApi/Models/OpenApiSchema.cs | 72 ++++++++++++++----- .../Models/OpenApiSchemaTests.cs | 70 ++++++++++++++++++ 2 files changed, 126 insertions(+), 16 deletions(-) diff --git a/src/Microsoft.OpenApi/Models/OpenApiSchema.cs b/src/Microsoft.OpenApi/Models/OpenApiSchema.cs index f236ae8db..ce2d76052 100644 --- a/src/Microsoft.OpenApi/Models/OpenApiSchema.cs +++ b/src/Microsoft.OpenApi/Models/OpenApiSchema.cs @@ -260,9 +260,7 @@ public void SerializeAsV3(IOpenApiWriter writer) writer.WriteStartObject(); - WriteSchemaPropertiesAsV3( - writer, - (w, s) => s.SerializeAsV3(w)); + WriteSchemaPropertiesAsV3(writer, (w, s) => s.SerializeAsV3(w)); writer.WriteEndObject(); } @@ -272,13 +270,22 @@ public void SerializeAsV3(IOpenApiWriter writer) /// public void SerializeAsV3WithoutReference(IOpenApiWriter writer) { + SerializeAsV3WithoutReference(writer, new OpenApiSchemaVisitor()); + } + + private void SerializeAsV3WithoutReference( + IOpenApiWriter writer, + OpenApiSchemaVisitor visitor) + { + visitor.Enter(this); + writer.WriteStartObject(); - WriteSchemaPropertiesAsV3( - writer, - (w, s) => s.SerializeAsV3WithoutReference(w)); + WriteSchemaPropertiesAsV3(writer, (w, s) => s.SerializeAsV3WithoutReference(w, visitor)); writer.WriteEndObject(); + + visitor.Exit(); } private void WriteSchemaPropertiesAsV3( @@ -419,6 +426,7 @@ public void SerializeAsV2WithoutReference(IOpenApiWriter writer) { SerializeAsV2WithoutReference( writer: writer, + visitor: new OpenApiSchemaVisitor(), parentRequiredProperties: new HashSet(), propertyName: null); } @@ -463,28 +471,40 @@ internal void SerializeAsV2( writer.WriteEndObject(); } - /// - /// Serialize to OpenAPI V2 document without using reference and handles not marking the provided property - /// as readonly if its included in the provided list of required properties of parent schema. - /// - /// The open api writer. - /// The list of required properties in parent schema. - /// The property name that will be serialized. - internal void SerializeAsV2WithoutReference( + private void SerializeAsV2WithoutReference( IOpenApiWriter writer, + OpenApiSchemaVisitor visitor, ISet parentRequiredProperties, string propertyName) { + visitor.Enter(this); + writer.WriteStartObject(); + Action serializeSchema = (w, s) => + s.SerializeAsV2WithoutReference( + w, + visitor, + parentRequiredProperties: null, + propertyName: null); + + Action serializeSchemaProperty = (w, key, s) => + s.SerializeAsV2WithoutReference( + w, + visitor, + Required, + key); + WriteSchemaPropertiesAsV2( writer, parentRequiredProperties, propertyName, - (w, s) => s.SerializeAsV2WithoutReference(w), - (w, key, s) => s.SerializeAsV2WithoutReference(w, Required, key)); + serializeSchema, + serializeSchemaProperty); writer.WriteEndObject(); + + visitor.Exit(); } internal void WriteAsItemsProperties(IOpenApiWriter writer) @@ -661,5 +681,25 @@ private void WriteSchemaPropertiesAsV2( // extensions writer.WriteExtensions(Extensions, OpenApiSpecVersion.OpenApi2_0); } + + private class OpenApiSchemaVisitor + { + private readonly Stack _visitedSchemas = new Stack(); + + public void Enter(OpenApiSchema schema) + { + if (_visitedSchemas.Contains(schema)) + { + throw new NotSupportedException("Serializing circular references in schemas is not yet supported"); + } + + _visitedSchemas.Push(schema); + } + + public void Exit() + { + _visitedSchemas.Pop(); + } + } } } \ No newline at end of file diff --git a/test/Microsoft.OpenApi.Tests/Models/OpenApiSchemaTests.cs b/test/Microsoft.OpenApi.Tests/Models/OpenApiSchemaTests.cs index 1939c15e5..f285a23a6 100644 --- a/test/Microsoft.OpenApi.Tests/Models/OpenApiSchemaTests.cs +++ b/test/Microsoft.OpenApi.Tests/Models/OpenApiSchemaTests.cs @@ -616,6 +616,76 @@ public void SerializeNestedReferencedSchemaAsV2JsonWorks() actual.Should().Be(expected); } + [Fact] + public void SeralizeSchemaWCircularReferencesAsV3Throws() + { + // Arrange + var outputStringWriter = new StringWriter(CultureInfo.InvariantCulture); + var writer = new OpenApiJsonWriter(outputStringWriter); + + var circularlyReferencedSchema1 = new OpenApiSchema + { + Title = "title1", + Type = "object", + Properties = new Dictionary() + }; + + var circularlyReferencedSchema2 = new OpenApiSchema + { + Title = "title2", + Type = "object", + Properties = new Dictionary + { + ["property2"] = circularlyReferencedSchema1 + } + }; + + circularlyReferencedSchema1.Properties["property1"] = circularlyReferencedSchema2; + + // Act + Action action = () => circularlyReferencedSchema1.SerializeAsV3WithoutReference(writer); + + // Assert + action + .ShouldThrow() + .WithMessage("Serializing circular references in schemas is not yet supported"); + } + + [Fact] + public void SeralizeSchemaWCircularReferencesAsV2Throws() + { + // Arrange + var outputStringWriter = new StringWriter(CultureInfo.InvariantCulture); + var writer = new OpenApiJsonWriter(outputStringWriter); + + var circularlyReferencedSchema1 = new OpenApiSchema + { + Title = "title1", + Type = "object", + Properties = new Dictionary() + }; + + var circularlyReferencedSchema2 = new OpenApiSchema + { + Title = "title2", + Type = "object", + Properties = new Dictionary + { + ["property2"] = circularlyReferencedSchema1 + } + }; + + circularlyReferencedSchema1.Properties["property1"] = circularlyReferencedSchema2; + + // Act + Action action = () => circularlyReferencedSchema1.SerializeAsV2WithoutReference(writer); + + // Assert + action + .ShouldThrow() + .WithMessage("Serializing circular references in schemas is not yet supported"); + } + [Fact] public void SerializeSchemaWRequiredPropertiesAsV2JsonWorks() {