Prevent overflow for self-referencing schemas

Author: darrelmiller

src/Microsoft.OpenApi.Readers/ParsingContext.cs

@@ -23,9 +23,11 @@ public class ParsingContext
         private readonly Dictionary<string, IOpenApiReferenceable> _referenceStore = new Dictionary<string, IOpenApiReferenceable>();
         private readonly Dictionary<string, object> _tempStorage = new Dictionary<string, object>();
         private IOpenApiVersionService _versionService;
+        private readonly Dictionary<string, Stack<string>> _loopStacks = new Dictionary<string, Stack<string>>();
+
         internal RootNode RootNode { get; set; }
         internal List<OpenApiTag> Tags { get; private set; } = new List<OpenApiTag>();
-
+ 
 
         /// <summary>
         /// Initiates the parsing process.  Not thread safe and should only be called once on a parsing context
@@ -197,5 +199,52 @@ public void StartObject(string objectName)
         {
             _currentLocation.Push(objectName);
         }
+
+        /// <summary>
+        /// Maintain history of traversals to avoid stack overflows from cycles
+        /// </summary>
+        /// <param name="loopId">Any unique identifier for a stack</param>
+        /// <param name="key">Identifier used to </param>
+        /// <returns></returns>
+        public bool PushLoop(string loopId, string key)
+        {
+            Stack<string> stack;
+            if (!_loopStacks.TryGetValue(loopId, out stack))
+            {
+                stack = new Stack<string>();
+                _loopStacks.Add(loopId, stack);
+            }
+
+            if (!stack.Contains(key))
+            {
+                stack.Push(key);
+                return true;
+            } else
+            {
+                return false;  // Loop detected
+            }
+        }
+
+        /// <summary>
+        /// Reset loop tracking stack
+        /// </summary>
+        /// <param name="loopid"></param>
+        internal void ClearLoop(string loopid)
+        {
+            _loopStacks[loopid].Clear();
+        }
+
+        /// <summary>
+        /// Exit from the context in cycle detection
+        /// </summary>
+        /// <param name="loopid"></param>
+        public void PopLoop(string loopid)
+        {
+            if (_loopStacks[loopid].Count > 0)
+            {
+                _loopStacks[loopid].Pop();
+            }
+        }
+
     }
 }

src/Microsoft.OpenApi.Readers/V3/OpenApiSchemaDeserializer.cs

@@ -234,15 +234,30 @@ internal static partial class OpenApiV3Deserializer
             {s => s.StartsWith("x-"), (o, p, n) => o.AddExtension(p, n.CreateAny())}
         };
 
+        private const string schemaLoopId = "schema";
+
         public static OpenApiSchema LoadSchema(ParseNode node)
         {
-            var mapNode = node.CheckMapNode("schema");
+            var mapNode = node.CheckMapNode(OpenApiConstants.Schema);
 
             var pointer = mapNode.GetReferencePointer();
 
             if (pointer != null)
             {
-                return mapNode.GetReferencedObject<OpenApiSchema>(ReferenceType.Schema, pointer);
+                if (node.Context.PushLoop(schemaLoopId, pointer))
+                {
+                    var schema = mapNode.GetReferencedObject<OpenApiSchema>(ReferenceType.Schema, pointer);
+                    node.Context.PopLoop(schemaLoopId);
+                    return schema;
+                } else
+                {
+                    node.Context.ClearLoop(schemaLoopId);
+                    //TODO.  How do we make the object graph have a cycle.  Or should we break the cycle in the graph?
+                    return new OpenApiSchema()
+                    {
+                        Reference = node.Context.VersionService.ConvertToOpenApiReference(pointer,ReferenceType.Schema)  
+                    };
+                }
             }
 
             var domainObject = new OpenApiSchema();

test/Microsoft.OpenApi.Readers.Tests/Microsoft.OpenApi.Readers.Tests.csproj

@@ -12,6 +12,9 @@
         <SignAssembly>true</SignAssembly>
         <AssemblyOriginatorKeyFile>..\..\src\Microsoft.OpenApi.snk</AssemblyOriginatorKeyFile>
     </PropertyGroup>
+    <ItemGroup>
+      <None Remove="V3Tests\Samples\OpenApiSchema\selfReferencingSchema.yaml" />
+    </ItemGroup>
     <ItemGroup>
       <EmbeddedResource Include="ReferenceService\Samples\multipleReferences.v2.yaml">
         <CopyToOutputDirectory>Never</CopyToOutputDirectory>
@@ -106,6 +109,7 @@
       <EmbeddedResource Include="V3Tests\Samples\OpenApiSchema\primitiveSchema.yaml">
         <CopyToOutputDirectory>Never</CopyToOutputDirectory>
       </EmbeddedResource>
+      <EmbeddedResource Include="V3Tests\Samples\OpenApiSchema\selfReferencingSchema.yaml" />
       <EmbeddedResource Include="V3Tests\Samples\OpenApiSchema\simpleSchema.yaml">
         <CopyToOutputDirectory>Never</CopyToOutputDirectory>
       </EmbeddedResource>

test/Microsoft.OpenApi.Readers.Tests/V3Tests/OpenApiSchemaTests.cs

@@ -280,7 +280,7 @@ public void ParseAdvancedSchemaWithReferenceShouldSucceed()
 
                 // Assert
                 var components = openApiDoc.Components;
-                
+
                 diagnostic.ShouldBeEquivalentTo(
                     new OpenApiDiagnostic() { SpecificationVersion = OpenApiSpecVersion.OpenApi3_0 });
 
@@ -430,5 +430,24 @@ public void ParseAdvancedSchemaWithReferenceShouldSucceed()
                     });
             }
         }
+
+
+        [Fact]
+        public void ParseSelfReferencingSchemaShouldNotStackOverflow()
+        {
+            using (var stream = Resources.GetStream(Path.Combine(SampleFolderPath, "selfReferencingSchema.yaml")))
+            {
+                // Act
+                var openApiDoc = new OpenApiStreamReader().Read(stream, out var diagnostic);
+
+                // Assert
+                var components = openApiDoc.Components;
+
+                diagnostic.ShouldBeEquivalentTo(
+                    new OpenApiDiagnostic() { SpecificationVersion = OpenApiSpecVersion.OpenApi3_0 });
+
+
+            }
+        }
     }
 }

test/Microsoft.OpenApi.Readers.Tests/V3Tests/Samples/OpenApiSchema/selfReferencingSchema.yaml

@@ -0,0 +1,26 @@
+openapi: 3.0.0
+info:
+  title: Self Referencing schema
+  version: 1.0.0
+paths: {}
+components:
+  schemas:
+    microsoft.graph.schemaExtension: 
+      allOf: 
+        - 
+          title: "schemaExtension"
+          type: "object"
+          properties: 
+            description: 
+              type: "string"
+              nullable: true
+            targetTypes: 
+              type: "array"
+              items: 
+                type: "string"
+            status: 
+              type: "string"
+            owner: 
+              type: "string"
+            child:
+              $ref: "#/components/schemas/microsoft.graph.schemaExtension"