Merge pull request #185 from Microsoft/dm/ref-overflow

Prevent overflow for self-referencing schemas

Author: darrelmiller

src/Microsoft.OpenApi.Readers/Interface/IOpenApiVersionService.cs

@@ -13,19 +13,6 @@ namespace Microsoft.OpenApi.Readers.Interface
     /// </summary>
     internal interface IOpenApiVersionService
     {
-        /// <summary>
-        /// Load the referenced <see cref="IOpenApiReferenceable"/> object from a <see cref="OpenApiReference"/> object
-        /// </summary>
-        /// <param name="context">Instance of ParsingContext to use for retrieving references.</param>
-        /// <param name="reference">The <see cref="OpenApiReference"/> object.</param>
-        /// <param name="referencedObject">The object that is being referenced.</param>
-        /// <returns>
-        /// If the reference is found, return true and the referenced object in the out parameter.
-        /// In the case of tag, it is psosible that the referenced object does not exist. In this case,
-        /// a new tag will be returned in the outer parameter and the return value will be false.
-        /// If reference is null, no object will be returned and the return value will be false.
-        /// </returns>
-        bool TryLoadReference(ParsingContext context, OpenApiReference reference, out IOpenApiReferenceable referencedObject);
 
         /// <summary>
         /// Parse the string to a <see cref="OpenApiReference"/> object.

src/Microsoft.OpenApi.Readers/OpenApiReaderSettings.cs

@@ -10,11 +10,35 @@
 
 namespace Microsoft.OpenApi.Readers
 {
+    /// <summary>
+    /// Indicates if and when the reader should convert unresolved references into resolved objects
+    /// </summary>
+    public enum ReferenceResolutionSetting
+    {
+        /// <summary>
+        /// Create placeholder objects with an OpenApiReference instance and UnresolvedReference set to true.
+        /// </summary>
+        DoNotResolveReferences,
+        /// <summary>
+        /// Convert local references to references of valid domain objects.
+        /// </summary>
+        ResolveLocalReferences,
+        /// <summary>
+        /// Convert all references to references of valid domain objects.
+        /// </summary>
+        ResolveAllReferences
+    }
+
     /// <summary>
     /// Configuration settings to control how OpenAPI documents are parsed
     /// </summary>
     public class OpenApiReaderSettings
     {
+        /// <summary>
+        /// Indicates how references in the source document should be handled.
+        /// </summary>
+        public ReferenceResolutionSetting ReferenceResolution { get; set; } = ReferenceResolutionSetting.ResolveLocalReferences;
+
         /// <summary>
         /// Dictionary of parsers for converting extensions into strongly typed classes
         /// </summary>

src/Microsoft.OpenApi.Readers/OpenApiStreamReader.cs

@@ -1,13 +1,14 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. 
 
+using System;
 using System.IO;
 using System.Linq;
 using Microsoft.OpenApi.Extensions;
 using Microsoft.OpenApi.Models;
 using Microsoft.OpenApi.Readers.Interface;
+using Microsoft.OpenApi.Readers.Services;
 using Microsoft.OpenApi.Services;
-using Microsoft.OpenApi.Validations;
 using SharpYaml;
 using SharpYaml.Serialization;
 
@@ -60,6 +61,20 @@ public OpenApiDocument Read(Stream input, out OpenApiDiagnostic diagnostic)
             // Parse the OpenAPI Document
             var document = context.Parse(yamlDocument, diagnostic);
 
+            // Resolve References if requested
+            switch (_settings.ReferenceResolution) 
+            {
+                case ReferenceResolutionSetting.ResolveAllReferences:
+                    throw new ArgumentException(Properties.SRResource.CannotResolveRemoteReferencesSynchronously);
+                case ReferenceResolutionSetting.ResolveLocalReferences:
+                    var resolver = new OpenApiReferenceResolver(document);
+                    var walker = new OpenApiWalker(resolver);
+                    walker.Walk(document);
+                    break;
+                case ReferenceResolutionSetting.DoNotResolveReferences:
+                    break;
+            }
+
             // Validate the document
             var errors = document.Validate(_settings.RuleSet);
             foreach (var item in errors)

src/Microsoft.OpenApi.Readers/ParseNodes/ListNode.cs

@@ -7,6 +7,8 @@
 using System.Linq;
 using Microsoft.OpenApi.Any;
 using Microsoft.OpenApi.Exceptions;
+using Microsoft.OpenApi.Interfaces;
+using Microsoft.OpenApi.Models;
 using SharpYaml.Serialization;
 
 namespace Microsoft.OpenApi.Readers.ParseNodes

src/Microsoft.OpenApi.Readers/ParseNodes/MapNode.cs

@@ -78,26 +78,33 @@ public override Dictionary<string, T> CreateMap<T>(Func<MapNode, T> map)
             return nodes.ToDictionary(k => k.key, v => v.value);
         }
 
-        public override Dictionary<string, T> CreateMapWithReference<T>(
-            ReferenceType referenceType,
-            string refpointerbase,
-            Func<MapNode, T> map)
-        {
-            var yamlMap = _node;
-            if (yamlMap == null)
-            {
-                throw new OpenApiException($"Expected map at line {yamlMap.Start.Line} while parsing {typeof(T).Name}");
-            }
-
-            var nodes = yamlMap.Select(
-                n => new
-                {
-                    key = n.Key.GetScalarValue(),
-                    value = GetReferencedObject<T>(referenceType, refpointerbase + n.Key.GetScalarValue()) ??
-                    map(new MapNode(Context, Diagnostic, (YamlMappingNode)n.Value))
-                });
-            return nodes.ToDictionary(k => k.key, v => v.value);
-        }
+         public override Dictionary<string, T> CreateMapWithReference<T>(
+             ReferenceType referenceType,
+             Func<MapNode, T> map)
+         {
+             var yamlMap = _node;
+             if (yamlMap == null)
+             {
+                 throw new OpenApiException($"Expected map at line {yamlMap.Start.Line} while parsing {typeof(T).Name}");
+             }
+
+             var nodes = yamlMap.Select(
+                 n => {
+                     var entry = new
+                     {
+                         key = n.Key.GetScalarValue(),
+                         value = map(new MapNode(Context, Diagnostic, (YamlMappingNode)n.Value))
+                     };
+                     entry.value.Reference = new OpenApiReference()
+                     {
+                         Type = referenceType,
+                         Id = entry.key
+                     };
+                     return entry;
+                 }
+                 );
+             return nodes.ToDictionary(k => k.key, v => v.value);
+         }
 
         public override Dictionary<string, T> CreateSimpleMap<T>(Func<ValueNode, T> map)
         {
@@ -133,12 +140,13 @@ public override string GetRaw()
         }
 
         public T GetReferencedObject<T>(ReferenceType referenceType, string referenceId)
-            where T : IOpenApiReferenceable
+            where T : IOpenApiReferenceable, new()
         {
-            return (T)Context.GetReferencedObject(
-                Diagnostic,
-                referenceType,
-                referenceId);
+            return new T()
+                    {
+                        UnresolvedReference = true,
+                        Reference = Context.VersionService.ConvertToOpenApiReference(referenceId,referenceType)  
+                    };
         }
 
         public string GetReferencePointer()

src/Microsoft.OpenApi.Readers/ParseNodes/ParseNode.cs

@@ -79,7 +79,6 @@ public virtual Dictionary<string, T> CreateMap<T>(Func<MapNode, T> map)
 
         public virtual Dictionary<string, T> CreateMapWithReference<T>(
             ReferenceType referenceType,
-            string refpointer,
             Func<MapNode, T> map)
             where T : class, IOpenApiReferenceable
         {

src/Microsoft.OpenApi.Readers/ParsingContext.cs

@@ -22,16 +22,13 @@ namespace Microsoft.OpenApi.Readers
     public class ParsingContext
     {
         private readonly Stack<string> _currentLocation = new Stack<string>();
-        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 Dictionary<string, Func<IOpenApiAny, IOpenApiExtension>> ExtensionParsers { get; set; }  = new Dictionary<string, Func<IOpenApiAny, IOpenApiExtension>>();
-
         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
         /// </summary>
@@ -131,48 +128,6 @@ public string GetLocation()
             return "#/" + string.Join("/", _currentLocation.Reverse().ToArray());
         }
 
-        /// <summary>
-        /// Gets the referenced object.
-        /// </summary>
-        public IOpenApiReferenceable GetReferencedObject(
-            OpenApiDiagnostic diagnostic,
-            ReferenceType referenceType,
-            string referenceString)
-        {
-            _referenceStore.TryGetValue(referenceString, out var referencedObject);
-
-            // If reference has already been accessed once, simply return the same reference object.
-            if (referencedObject != null)
-            {
-                return referencedObject;
-            }
-
-            var reference = VersionService.ConvertToOpenApiReference(referenceString, referenceType);
-
-            var isReferencedObjectFound = VersionService.TryLoadReference(this, reference, out referencedObject);
-
-            if (isReferencedObjectFound)
-            {
-                // Populate the Reference section of the object, so that the writers
-                // can recognize that this is referencing another object.
-                referencedObject.Reference = reference;
-                _referenceStore.Add(referenceString, referencedObject);
-            }
-            else if (referencedObject != null)
-            {
-                return referencedObject;
-            }
-            else
-            {
-                diagnostic.Errors.Add(
-                    new OpenApiError(
-                        GetLocation(),
-                        $"Cannot resolve the reference {referenceString}"));
-            }
-
-            return referencedObject;
-        }
-
         /// <summary>
         /// Gets the value from the temporary storage matching the given key.
         /// </summary>
@@ -201,5 +156,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 for current context.</param>
+        /// <returns>If method returns false a loop was detected and the key is not added.</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">Identifier of loop to clear</param>
+        internal void ClearLoop(string loopid)
+        {
+            _loopStacks[loopid].Clear();
+        }
+
+        /// <summary>
+        /// Exit from the context in cycle detection
+        /// </summary>
+        /// <param name="loopid">Identifier of loop</param>
+        public void PopLoop(string loopid)
+        {
+            if (_loopStacks[loopid].Count > 0)
+            {
+                _loopStacks[loopid].Pop();
+            }
+        }
+
     }
 }

src/Microsoft.OpenApi.Readers/Properties/SRResource.Designer.cs

@@ -120,6 +120,9 @@
   <data name="ArgumentNullOrWhiteSpace" xml:space="preserve">
     <value>The argument '{0}' is null, empty or consists only of white-space.</value>
   </data>
+  <data name="CannotResolveRemoteReferencesSynchronously" xml:space="preserve">
+    <value>"Cannot resolve remote references automatically in a syncronous call."</value>
+  </data>
   <data name="LoadReferencedObjectFromExternalNotImplmented" xml:space="preserve">
     <value>Not implemented to find referenced element from external resource.</value>
   </data>