From 5f130244a075394c2e5d9bc11562cca422afac26 Mon Sep 17 00:00:00 2001 From: "Gavin Barron (from Dev Box)" Date: Thu, 5 Nov 2026 16:01:22 -0800 Subject: [PATCH 1/2] fix: improved external ref file loading Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 357913a9-48cd-4007-897f-54956d3171aa --- README.md | 8 + .../Models/JsonSchemaReference.cs | 17 + .../Models/OpenApiDocument.cs | 2 +- src/Microsoft.OpenApi/PublicAPI.Unshipped.txt | 2 + .../Reader/OpenApiModelFactory.cs | 78 ++-- .../Reader/OpenApiReaderSettings.cs | 15 + .../Reader/Services/DefaultStreamLoader.cs | 15 +- .../OpenApiRemoteReferenceCollector.cs | 19 +- .../Reader/Services/OpenApiWorkspaceLoader.cs | 166 +++++-- .../Services/OpenApiWorkspace.cs | 15 +- .../OpenApiWorkspaceStreamTests.cs | 410 ++++++++++++++++++ .../Reader/OpenApiReaderSettingsTests.cs | 9 + 12 files changed, 677 insertions(+), 79 deletions(-) diff --git a/README.md b/README.md index a4cf4171e..7a02f4a08 100644 --- a/README.md +++ b/README.md @@ -84,6 +84,14 @@ var (openApiDocument, _) = await OpenApiDocument.LoadAsync("https://raw.githubus var outputString = await openApiDocument.SerializeAsJsonAsync(OpenApiSpecVersion.OpenApi2_0); ``` +External references are loaded only when `OpenApiReaderSettings.LoadExternalRefs` is enabled. +`MaxExternalReferenceDepth` defaults to 64 external document hops (the root is depth zero) +and can be set to a higher positive value for deeper descriptions. It is independent of +JSON/YAML nesting limits. External documents are reused within a load, allowing valid +recursive schemas across files without repeated requests. Exceeding the depth limit or +encountering an external reference-only loop returns a `ReadResult` with a null `Document` +and an error in `Diagnostic.Errors`. + # Validating/Testing OpenAPI descriptions [Microsoft.OpenApi.Hidi](https://www.nuget.org/packages/Microsoft.OpenApi.Hidi) is a command line tool for validating and transforming OpenAPI descriptions. The OpenAPI.NET v3-compatible tool is maintained in [OpenAPI.NET.OData's main branch](https://github.com/microsoft/OpenAPI.NET.OData/tree/main/src/Microsoft.OpenApi.Hidi). diff --git a/src/Microsoft.OpenApi/Models/JsonSchemaReference.cs b/src/Microsoft.OpenApi/Models/JsonSchemaReference.cs index c495ddbea..60cca8f29 100644 --- a/src/Microsoft.OpenApi/Models/JsonSchemaReference.cs +++ b/src/Microsoft.OpenApi/Models/JsonSchemaReference.cs @@ -361,6 +361,23 @@ internal bool IsDynamicRefOnly && ReferenceV3 is string refV3 && !refV3.StartsWith("#/components/", StringComparison.OrdinalIgnoreCase); + internal bool IsReferenceOnly => + Description is null && Title is null && Default is null && Deprecated is null && + ReadOnly is null && WriteOnly is null && Examples is null && Extensions is not { Count: > 0 } && + SchemaId is null && Schema is null && Comment is null && Vocabulary is null && + DynamicRef is null && DynamicAnchor is null && Definitions is null && Anchor is null && + ExclusiveMaximum is null && ExclusiveMinimum is null && SchemaType is null && !WasConstExplicitlySet && + Format is null && Maximum is null && Minimum is null && MaxLength is null && MinLength is null && + Pattern is null && MultipleOf is null && AllOf is null && OneOf is null && AnyOf is null && + Not is null && Required is null && Items is null && MaxItems is null && MinItems is null && + UniqueItems is null && Contains is null && MaxContains is null && MinContains is null && + Properties is null && PatternProperties is null && MaxProperties is null && MinProperties is null && + AdditionalPropertiesAllowed is null && AdditionalProperties is null && Discriminator is null && + Example is null && Enum is null && UnevaluatedProperties is null && UnevaluatedPropertiesSchema is null && + ExternalDocs is null && Xml is null && UnrecognizedKeywords is not { Count: > 0 } && + DependentRequired is null && ContentEncoding is null && ContentMediaType is null && ContentSchema is null && + PropertyNames is null && DependentSchemas is null && If is null && Then is null && Else is null; + /// /// Parameterless constructor /// diff --git a/src/Microsoft.OpenApi/Models/OpenApiDocument.cs b/src/Microsoft.OpenApi/Models/OpenApiDocument.cs index 03086600d..cb014a34c 100644 --- a/src/Microsoft.OpenApi/Models/OpenApiDocument.cs +++ b/src/Microsoft.OpenApi/Models/OpenApiDocument.cs @@ -661,7 +661,7 @@ private static string ConvertByteArrayToString(byte[] hash) relativePath = $"#{OpenApiConstants.ComponentsSegment}{reference.Type.GetDisplayName()}/{id}"; } - Uri? externalResourceUri = useExternal ? Workspace?.GetDocumentId(reference.ExternalResource) : null; + Uri? externalResourceUri = useExternal ? Workspace?.GetDocumentId(reference.ExternalResource, BaseUri) : null; uriLocation = useExternal && externalResourceUri is not null ? externalResourceUri.AbsoluteUri + relativePath diff --git a/src/Microsoft.OpenApi/PublicAPI.Unshipped.txt b/src/Microsoft.OpenApi/PublicAPI.Unshipped.txt index 7dc5c5811..8cd30258b 100644 --- a/src/Microsoft.OpenApi/PublicAPI.Unshipped.txt +++ b/src/Microsoft.OpenApi/PublicAPI.Unshipped.txt @@ -1 +1,3 @@ #nullable enable +Microsoft.OpenApi.Reader.OpenApiReaderSettings.MaxExternalReferenceDepth.get -> int +Microsoft.OpenApi.Reader.OpenApiReaderSettings.MaxExternalReferenceDepth.set -> void diff --git a/src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs b/src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs index 2abd04873..0b488d854 100644 --- a/src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs +++ b/src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs @@ -79,7 +79,8 @@ public static async Task LoadAsync(string url, OpenApiReaderSettings var (stream, format) = await RetrieveStreamAndFormatAsync(url, settings, token).ConfigureAwait(false); using (stream) { - return await LoadAsync(stream, format, settings, token).ConfigureAwait(false); + Uri.TryCreate(url, UriKind.Absolute, out var sourceUri); + return await LoadStreamAsync(stream, format, settings, sourceUri, token).ConfigureAwait(false); } } @@ -112,40 +113,46 @@ public static async Task LoadAsync(string url, OpenApiReaderSettings /// Propagates notification that operations should be cancelled. /// The Open API format /// - public static async Task LoadAsync(Stream input, string? format = null, OpenApiReaderSettings? settings = null, CancellationToken cancellationToken = default) + public static Task LoadAsync(Stream input, string? format = null, OpenApiReaderSettings? settings = null, CancellationToken cancellationToken = default) + => LoadStreamAsync(input, format, settings ?? new OpenApiReaderSettings(), null, cancellationToken); + + private static async Task LoadStreamAsync(Stream input, string? format, OpenApiReaderSettings settings, Uri? sourceUri, CancellationToken cancellationToken) { #if NET6_0_OR_GREATER ArgumentNullException.ThrowIfNull(input); #else if (input is null) throw new ArgumentNullException(nameof(input)); #endif - settings ??= new OpenApiReaderSettings(); - + sourceUri = input is FileStream sourceFile ? new Uri(sourceFile.Name) : sourceUri; Stream? preparedStream = null; - if (format is null) + try { - (preparedStream, format) = await PrepareStreamForReadingAsync(input, format, cancellationToken).ConfigureAwait(false); - } + if (format is null) + { + (preparedStream, format) = await PrepareStreamForReadingAsync(input, format, cancellationToken).ConfigureAwait(false); + } - // Use StreamReader to process the prepared stream (buffered for YAML, direct for JSON) - var result = await InternalLoadAsync(preparedStream ?? input, format, settings, cancellationToken).ConfigureAwait(false); - if (!settings.LeaveStreamOpen) + return await InternalLoadAsync(preparedStream ?? input, format, settings, sourceUri, cancellationToken).ConfigureAwait(false); + } + finally { + if (!settings.LeaveStreamOpen) + { #if NETSTANDARD2_1_OR_GREATER || NETCOREAPP || NET5_0_OR_GREATER - await input.DisposeAsync().ConfigureAwait(false); + await input.DisposeAsync().ConfigureAwait(false); #else - input.Dispose(); + input.Dispose(); #endif - } - if (preparedStream is not null && preparedStream != input) - { + } + if (preparedStream is not null && preparedStream != input) + { #if NETSTANDARD2_1_OR_GREATER || NETCOREAPP || NET5_0_OR_GREATER - await preparedStream.DisposeAsync().ConfigureAwait(false); + await preparedStream.DisposeAsync().ConfigureAwait(false); #else - preparedStream.Dispose(); + preparedStream.Dispose(); #endif + } } - return result; } /// @@ -240,40 +247,35 @@ public static ReadResult Parse(string input, private static readonly Lazy DefaultReaderSettings = new(() => new OpenApiReaderSettings()); - private static async Task InternalLoadAsync(Stream input, string format, OpenApiReaderSettings settings, CancellationToken cancellationToken = default) + internal static Task ReadDocumentAsync(Stream input, string format, Uri location, OpenApiReaderSettings settings, CancellationToken cancellationToken) + => settings.GetReader(format).ReadAsync(input, location, settings, cancellationToken); + + private static async Task InternalLoadAsync(Stream input, string format, OpenApiReaderSettings settings, Uri? sourceUri, CancellationToken cancellationToken = default) { settings ??= DefaultReaderSettings.Value; - var reader = settings.GetReader(format); - var location = - (input is FileStream fileStream ? new Uri(fileStream.Name) : null) ?? + var maxExternalReferenceDepth = settings.MaxExternalReferenceDepth; + var location = (sourceUri?.IsFile == true ? sourceUri : null) ?? settings.BaseUrl ?? + sourceUri ?? new Uri(OpenApiConstants.BaseRegistryUri); - var readResult = await reader.ReadAsync(input, location, settings, cancellationToken).ConfigureAwait(false); + var readResult = await ReadDocumentAsync(input, format, location, settings, cancellationToken).ConfigureAwait(false); - if (settings.LoadExternalRefs) + if (settings.LoadExternalRefs && readResult.Document is { } document) { - var diagnosticExternalRefs = await LoadExternalRefsAsync(readResult.Document, settings, format, cancellationToken).ConfigureAwait(false); - // Merge diagnostics of external reference - if (diagnosticExternalRefs != null) + var workspace = document.Workspace ?? new OpenApiWorkspace(); + var loader = new OpenApiWorkspaceLoader(workspace, + settings.CustomExternalLoader ?? new DefaultStreamLoader(settings.HttpClient), settings, maxExternalReferenceDepth); + readResult.Diagnostic ??= new(); + if (!await loader.LoadAsync(document, format, readResult.Diagnostic, sourceUri, cancellationToken).ConfigureAwait(false)) { - readResult.Diagnostic?.Errors.AddRange(diagnosticExternalRefs.Errors); - readResult.Diagnostic?.Warnings.AddRange(diagnosticExternalRefs.Warnings); + readResult.Document = null; } } return readResult; } - private static async Task LoadExternalRefsAsync(OpenApiDocument? document, OpenApiReaderSettings settings, string? format = null, CancellationToken token = default) - { - // Load this document into the workspace - var streamLoader = new DefaultStreamLoader(settings.HttpClient); - var workspace = document?.Workspace ?? new OpenApiWorkspace(); - var workspaceLoader = new OpenApiWorkspaceLoader(workspace, settings.CustomExternalLoader ?? streamLoader, settings); - return await workspaceLoader.LoadAsync(new BaseOpenApiReference() { ExternalResource = "/" }, document, format ?? OpenApiConstants.Json, null, token).ConfigureAwait(false); - } - private static ReadResult InternalLoad(MemoryStream input, string format, OpenApiReaderSettings settings) { settings ??= DefaultReaderSettings.Value; diff --git a/src/Microsoft.OpenApi/Reader/OpenApiReaderSettings.cs b/src/Microsoft.OpenApi/Reader/OpenApiReaderSettings.cs index 21214b589..7d4c3f649 100644 --- a/src/Microsoft.OpenApi/Reader/OpenApiReaderSettings.cs +++ b/src/Microsoft.OpenApi/Reader/OpenApiReaderSettings.cs @@ -101,6 +101,21 @@ public Dictionary Readers /// public bool LoadExternalRefs { get; set; } = false; + private int _maxExternalReferenceDepth = 64; + + /// + /// Maximum number of external document hops loaded along a reference path. Defaults to 64. + /// The root document has depth zero. This does not limit JSON/YAML nesting or internal references. + /// When exceeded, loading returns a null document and an error diagnostic. + /// + /// The value is not positive. + public int MaxExternalReferenceDepth + { + get => _maxExternalReferenceDepth; + set => _maxExternalReferenceDepth = value > 0 ? value : + throw new ArgumentOutOfRangeException(nameof(value), "External reference depth must be greater than zero."); + } + /// /// Dictionary of parsers for converting extensions into strongly typed classes /// diff --git a/src/Microsoft.OpenApi/Reader/Services/DefaultStreamLoader.cs b/src/Microsoft.OpenApi/Reader/Services/DefaultStreamLoader.cs index fb6d86bab..f7bb00450 100644 --- a/src/Microsoft.OpenApi/Reader/Services/DefaultStreamLoader.cs +++ b/src/Microsoft.OpenApi/Reader/Services/DefaultStreamLoader.cs @@ -28,11 +28,7 @@ public DefaultStreamLoader(HttpClient httpClient) /// public async Task LoadAsync(Uri baseUrl, Uri uri, CancellationToken cancellationToken = default) { - var absoluteUri = baseUrl.AbsoluteUri.Equals(OpenApiConstants.BaseRegistryUri) switch - { - true => new Uri(Path.Combine(Directory.GetCurrentDirectory(), uri.ToString())), - _ => new Uri(baseUrl, uri), - }; + var absoluteUri = ResolveUri(baseUrl, uri); return absoluteUri.Scheme switch { @@ -46,5 +42,14 @@ await _httpClient.GetStreamAsync(absoluteUri).ConfigureAwait(false), _ => throw new ArgumentException("Unsupported scheme"), }; } + + internal static Uri ResolveUri(Uri baseUrl, Uri uri) + { + var resolved = uri.IsAbsoluteUri ? uri : + baseUrl.AbsoluteUri.Equals(OpenApiConstants.BaseRegistryUri) ? + new Uri(Path.Combine(Directory.GetCurrentDirectory(), uri.ToString())) : + new Uri(baseUrl, uri); + return new UriBuilder(resolved) { Fragment = string.Empty }.Uri; + } } } diff --git a/src/Microsoft.OpenApi/Reader/Services/OpenApiRemoteReferenceCollector.cs b/src/Microsoft.OpenApi/Reader/Services/OpenApiRemoteReferenceCollector.cs index a741f9a66..d545e6e3d 100644 --- a/src/Microsoft.OpenApi/Reader/Services/OpenApiRemoteReferenceCollector.cs +++ b/src/Microsoft.OpenApi/Reader/Services/OpenApiRemoteReferenceCollector.cs @@ -11,6 +11,7 @@ namespace Microsoft.OpenApi.Reader internal class OpenApiRemoteReferenceCollector : OpenApiVisitorBase { private readonly Dictionary _references = new(); + internal List ReferenceHolders { get; } = new(); /// /// List of all external references collected from OpenApiDocument @@ -26,7 +27,21 @@ public IEnumerable References /// public override void Visit(IOpenApiReferenceHolder referenceHolder) { - var reference = referenceHolder switch + ReferenceHolders.Add(referenceHolder); + var reference = GetReference(referenceHolder); + AddExternalReferences(reference); + if (reference is JsonSchemaReference schema) + { + var walker = new OpenApiWalker(this); + foreach (var child in OpenApiWorkspace.EnumerateChildren(schema)) + { + walker.Walk(child); + } + } + } + + internal static BaseOpenApiReference GetReference(IOpenApiReferenceHolder referenceHolder) + => referenceHolder switch { IOpenApiReferenceHolder { Reference: OpenApiReferenceWithDescriptionAndSummary withSummary } => withSummary, IOpenApiReferenceHolder { Reference: OpenApiReferenceWithDescription withDescription } => withDescription, @@ -34,8 +49,6 @@ public override void Visit(IOpenApiReferenceHolder referenceHolder) IOpenApiReferenceHolder { Reference: BaseOpenApiReference baseReference } => baseReference, _ => throw new OpenApiException($"Unsupported reference holder type: {referenceHolder.GetType().FullName}") }; - AddExternalReferences(reference); - } /// /// Collect external references diff --git a/src/Microsoft.OpenApi/Reader/Services/OpenApiWorkspaceLoader.cs b/src/Microsoft.OpenApi/Reader/Services/OpenApiWorkspaceLoader.cs index e793aa985..7385b683a 100644 --- a/src/Microsoft.OpenApi/Reader/Services/OpenApiWorkspaceLoader.cs +++ b/src/Microsoft.OpenApi/Reader/Services/OpenApiWorkspaceLoader.cs @@ -1,4 +1,7 @@ using System; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; @@ -9,59 +12,160 @@ internal class OpenApiWorkspaceLoader private readonly OpenApiWorkspace _workspace; private readonly IStreamLoader _loader; private readonly OpenApiReaderSettings _readerSettings; + private readonly int _maxDepth; - public OpenApiWorkspaceLoader(OpenApiWorkspace workspace, IStreamLoader loader, OpenApiReaderSettings readerSettings) + public OpenApiWorkspaceLoader(OpenApiWorkspace workspace, IStreamLoader loader, OpenApiReaderSettings readerSettings, int maxDepth) { _workspace = workspace; _loader = loader; _readerSettings = readerSettings; + _maxDepth = maxDepth; } - internal async Task LoadAsync(BaseOpenApiReference reference, - OpenApiDocument? document, - string? format = null, - OpenApiDiagnostic? diagnostic = null, - CancellationToken cancellationToken = default) + internal async Task LoadAsync(OpenApiDocument document, string format, OpenApiDiagnostic diagnostic, Uri? sourceUri, CancellationToken cancellationToken) { - _workspace.AddDocumentId(reference.ExternalResource, document?.BaseUri); - var version = diagnostic?.SpecificationVersion ?? OpenApiSpecVersion.OpenApi3_0; - if (document is not null) + var documents = new Dictionary(StringComparer.Ordinal); + if (sourceUri is not null) { - _workspace.RegisterComponents(document); - document.Workspace = _workspace; + var location = DefaultStreamLoader.ResolveUri(sourceUri, sourceUri).AbsoluteUri; + documents[location] = document; + _workspace.AddDocumentId(location, document.BaseUri); } - - // Collect remote references by walking document - var referenceCollector = new OpenApiRemoteReferenceCollector(); - var collectorWalker = new OpenApiWalker(referenceCollector); - collectorWalker.Walk(document); + var references = new List(); + var pending = new Stack<(OpenApiDocument Document, int Depth, IEnumerator References)>(); + RegisterDocument(document, 0); + try + { + while (pending.Count > 0) + { + cancellationToken.ThrowIfCancellationRequested(); + var frame = pending.Peek(); + if (!frame.References.MoveNext()) + { + frame.References.Dispose(); + pending.Pop(); + continue; + } + + var reference = frame.References.Current; + var resource = reference.ExternalResource!; + var uri = DefaultStreamLoader.ResolveUri(frame.Document.BaseUri, new Uri(resource, UriKind.RelativeOrAbsolute)); + if (!documents.TryGetValue(uri.AbsoluteUri, out var loadedDocument)) + { + if (frame.Depth >= _maxDepth) + { + diagnostic.Errors.Add(new(reference.ReferenceV3, + $"External reference depth limit {_maxDepth} exceeded while loading '{uri}' from '{frame.Document.BaseUri}'.")); + return false; + } + + using var input = await _loader.LoadAsync(frame.Document.BaseUri, new Uri(resource, UriKind.RelativeOrAbsolute), cancellationToken).ConfigureAwait(false); + var result = await OpenApiModelFactory.ReadDocumentAsync(input, format, uri, _readerSettings, cancellationToken).ConfigureAwait(false); + if (result.Diagnostic is { } externalDiagnostic) + { + diagnostic.AppendDiagnostic(externalDiagnostic, uri.AbsoluteUri); + } + loadedDocument = result.Document; + documents.Add(uri.AbsoluteUri, loadedDocument); + if (loadedDocument is not null) + { + RegisterDocument(loadedDocument, frame.Depth + 1); + } + } + + _workspace.AddDocumentId(resource, loadedDocument?.BaseUri); + _workspace.AddDocumentId(uri.AbsoluteUri, loadedDocument?.BaseUri); + } - diagnostic ??= new() { SpecificationVersion = version }; + return ValidateReferenceChains(references, documents, diagnostic, cancellationToken); + } + finally + { + foreach (var frame in pending) + { + frame.References.Dispose(); + } + } - // Walk references - foreach (var item in referenceCollector.References) + void RegisterDocument(OpenApiDocument loaded, int depth) { + loaded.Workspace = _workspace; + _workspace.RegisterComponents(loaded); + var collector = new OpenApiRemoteReferenceCollector(); + new OpenApiWalker(collector).Walk(loaded); + references.AddRange(collector.ReferenceHolders); + pending.Push((loaded, depth, collector.References.GetEnumerator())); + } + } - // If not already in workspace, load it and process references - if (item.ExternalResource is not null && !_workspace.Contains(item.ExternalResource)) + private static bool ValidateReferenceChains(List references, Dictionary documents, OpenApiDiagnostic diagnostic, CancellationToken cancellationToken) + { + var completed = new HashSet(ReferenceIdentityComparer.Instance); + foreach (var start in references) + { + var path = new Dictionary(ReferenceIdentityComparer.Instance); + var chain = new List(); + IOpenApiReferenceHolder? current = start; + while (current is not null && !completed.Contains(current)) { - var uri = new Uri(item.ExternalResource, UriKind.RelativeOrAbsolute); - var input = await _loader.LoadAsync(item.HostDocument!.BaseUri, uri, cancellationToken).ConfigureAwait(false); - var result = await OpenApiDocument.LoadAsync(input, format, _readerSettings, cancellationToken).ConfigureAwait(false); - // Merge diagnostics - if (result.Diagnostic != null) + cancellationToken.ThrowIfCancellationRequested(); + var reference = OpenApiRemoteReferenceCollector.GetReference(current); + if (current is OpenApiSchemaReference schema && !schema.Reference.IsReferenceOnly) + { + break; + } + if (reference.IsExternal && reference.HostDocument is { } host) { - diagnostic.AppendDiagnostic(result.Diagnostic, item.ExternalResource); + var location = DefaultStreamLoader.ResolveUri(host.BaseUri, new Uri(reference.ExternalResource!, UriKind.RelativeOrAbsolute)); + if (!documents.TryGetValue(location.AbsoluteUri, out var loaded) || loaded is null) + { + break; + } } - if (result.Document != null) + if (path.TryGetValue(current, out var cycleStart)) { - var loadDiagnostic = await LoadAsync(item, result.Document, format, diagnostic, cancellationToken).ConfigureAwait(false); - diagnostic = loadDiagnostic; + if (chain.Skip(cycleStart).Any(static item => item.IsExternal)) + { + diagnostic.Errors.Add(new(reference.ReferenceV3, + $"Circular external reference-only chain detected: {string.Join(" -> ", chain.Select(static item => $"{item.HostDocument?.BaseUri}: {item.ReferenceV3}"))}.")); + return false; + } + break; } + + path.Add(current, chain.Count); + chain.Add(reference); + IOpenApiReferenceable? target; + try + { + target = reference.HostDocument?.ResolveReference(reference, reference.IsExternal, null); + if (target is null && current is IOpenApiSchema parentSchema) + { + target = reference.HostDocument?.ResolveReference(reference, reference.IsExternal, parentSchema); + } + } + catch (InvalidOperationException ex) + { + if (chain.Any(static item => item.IsExternal)) + { + diagnostic.Errors.Add(new(reference.ReferenceV3, + $"Could not resolve external reference chain from '{chain[0].HostDocument?.BaseUri}' ({chain[0].ReferenceV3}): {ex.Message}")); + return false; + } + break; + } + current = target as IOpenApiReferenceHolder; } + completed.UnionWith(path.Keys); } + return true; + } - return diagnostic; + private sealed class ReferenceIdentityComparer : IEqualityComparer + { + public static ReferenceIdentityComparer Instance { get; } = new(); + public bool Equals(IOpenApiReferenceHolder? x, IOpenApiReferenceHolder? y) => ReferenceEquals(x, y); + public int GetHashCode(IOpenApiReferenceHolder obj) => RuntimeHelpers.GetHashCode(obj); } } } diff --git a/src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs b/src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs index 8872a8983..f7a2c47a2 100644 --- a/src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs +++ b/src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs @@ -597,7 +597,7 @@ private static IEnumerable EnumerateChildren(IOpenApiSchema s) /// Needed because JsonSchemaReference is not an IOpenApiSchema — reading via the interface /// on OpenApiSchemaReference would delegate to Target and cross document boundaries. /// - private static IEnumerable EnumerateChildren(JsonSchemaReference r) + internal static IEnumerable EnumerateChildren(JsonSchemaReference r) { if (r.Definitions is not null) foreach (var c in r.Definitions.Values) yield return c; @@ -807,6 +807,19 @@ public void AddDocumentId(string? key, Uri? value) return null; } + internal Uri? GetDocumentId(string? key, Uri baseUri) + { + if (key is not null) + { + var location = DefaultStreamLoader.ResolveUri(baseUri, new Uri(key, UriKind.RelativeOrAbsolute)); + if (_documentsIdRegistry.TryGetValue(location.AbsoluteUri, out var id)) + { + return id; + } + } + return GetDocumentId(key); + } + /// /// Verify if workspace contains a component based on its URL. /// diff --git a/test/Microsoft.OpenApi.Readers.Tests/OpenApiWorkspaceTests/OpenApiWorkspaceStreamTests.cs b/test/Microsoft.OpenApi.Readers.Tests/OpenApiWorkspaceTests/OpenApiWorkspaceStreamTests.cs index 34f4f2d45..024f122ed 100644 --- a/test/Microsoft.OpenApi.Readers.Tests/OpenApiWorkspaceTests/OpenApiWorkspaceStreamTests.cs +++ b/test/Microsoft.OpenApi.Readers.Tests/OpenApiWorkspaceTests/OpenApiWorkspaceStreamTests.cs @@ -1,7 +1,10 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Net.Http; +using System.Text; +using System.Text.Json; using System.Threading; using System.Threading.Tasks; using Microsoft.OpenApi.Reader; @@ -13,6 +16,413 @@ public class OpenApiWorkspaceStreamTests { // Use OpenApiWorkspace to load a document and a referenced document + [Theory] + [InlineData(1, "json")] + [InlineData(2, "json")] + [InlineData(3, "yaml")] + public async Task ExternalReferenceOnlyCyclesFailWithoutReloadingDocuments(int length, string format) + { + var loader = new RecordingLoader(); + for (var i = 0; i < length; i++) + { + loader.Documents[$"http://example.test/{i}.yaml"] = + Document($$$"""{"A":{"$ref":"{{{(i + 1) % length}}}.yaml#/components/schemas/A"}}"""); + } + var settings = Settings(loader); + settings.RuleSet = new ValidationRuleSet(); + var result = await Load(Document("""{"Main":{"$ref":"0.yaml#/components/schemas/A"}}"""), settings, format); + + Assert.Null(result.Document); + Assert.Contains(result.Diagnostic.Errors, error => + error.Message.Contains("Circular external reference-only chain", StringComparison.Ordinal)); + Assert.Equal(length, loader.Requests.Count); + Assert.Equal(length, loader.Requests.Distinct().Count()); + Assert.All(loader.Streams, stream => Assert.False(stream.CanRead)); + } + + [Fact] + public async Task StreamBaseUrlDoesNotPretendToIdentifyTheRootDocument() + { + var loader = new RecordingLoader(); + loader.Documents["http://example.test/a.yaml"] = """ + openapi: 3.0.0 + info: + title: External + version: 1.0.0 + paths: {} + components: + schemas: + A: + $ref: 'http://example.test/a.yaml#/components/schemas/A' + """; + var settings = Settings(loader); + settings.BaseUrl = new("http://example.test/a.yaml"); + var result = await Load(Document("""{"Main":{"$ref":"http://example.test/a.yaml#/components/schemas/A"}}"""), settings, "yaml"); + + Assert.Null(result.Document); + Assert.Single(loader.Requests); + Assert.Contains(result.Diagnostic.Errors, error => error.Message.Contains("Circular external", StringComparison.Ordinal)); + } + + [Theory] + [InlineData(2, 1, false)] + [InlineData(2, 2, true)] + [InlineData(2, 3, true)] + [InlineData(64, 65, true)] + [InlineData(65, 64, false)] + [InlineData(70, 80, true)] + public async Task ExternalDepthIsCheckedBeforeFetchingBeyondTheLimit(int length, int limit, bool succeeds) + { + var loader = Chain(length); + var settings = Settings(loader); + settings.MaxExternalReferenceDepth = limit; + var result = await Load(Document("""{"Main":{"$ref":"0.json#/components/schemas/A"}}"""), settings); + + Assert.Equal(succeeds, result.Document is not null); + Assert.Equal(Math.Min(length, limit), loader.Requests.Count); + if (succeeds) + { + Assert.Empty(result.Diagnostic.Errors); + var reference = Assert.IsType(result.Document.Components.Schemas["Main"]); + Assert.Equal(JsonSchemaType.Object, reference.RecursiveTarget.Type); + } + else + { + Assert.Contains(result.Diagnostic.Errors, error => + error.Message.Contains($"depth limit {limit}", StringComparison.Ordinal) && + error.Message.Contains($"{limit}.json", StringComparison.Ordinal)); + } + } + + [Fact] + public async Task DefaultDepthStopsAt64ExternalDocuments() + { + var loader = Chain(65); + var result = await Load(Document("""{"Main":{"$ref":"0.json#/components/schemas/A"}}"""), Settings(loader)); + Assert.Null(result.Document); + Assert.Equal(64, loader.Requests.Count); + Assert.Contains(result.Diagnostic.Errors, error => error.Message.Contains("depth limit 64", StringComparison.Ordinal)); + } + + [Fact] + public async Task SharedDependenciesAndEquivalentUrisAreFetchedOnce() + { + var loader = new RecordingLoader(); + loader.Documents["http://example.test/a.json"] = Document(""" + {"A":{"$ref":"shared.json#/components/schemas/A"}} + """); + loader.Documents["http://example.test/b.json"] = Document(""" + {"A":{"$ref":"./shared.json#/components/schemas/B"}} + """); + loader.Documents["http://example.test/shared.json"] = Document(""" + {"A":{"type":"string"},"B":{"type":"integer"}} + """); + var result = await Load(Document(""" + { + "A":{"$ref":"a.json#/components/schemas/A"}, + "B":{"$ref":"b.json#/components/schemas/A"}, + "Other":{"$ref":"http://example.test/sub/../shared.json#/components/schemas/A"} + } + """), Settings(loader)); + + Assert.NotNull(result.Document); + Assert.Empty(result.Diagnostic.Errors); + Assert.Equal(3, loader.Requests.Count); + Assert.Equal(JsonSchemaType.String, result.Document.Components.Schemas["A"].Type); + Assert.Equal(JsonSchemaType.Integer, result.Document.Components.Schemas["B"].Type); + } + + [Fact] + public async Task SameRelativeFilenameInDifferentDirectoriesResolvesIndependently() + { + var loader = new RecordingLoader(); + foreach (var directory in new[] { "left", "right" }) + { + loader.Documents[$"http://example.test/{directory}/a.json"] = + Document("""{"A":{"$ref":"shared.json#/components/schemas/A"}}"""); + loader.Documents[$"http://example.test/{directory}/shared.json"] = + Document($$$"""{"A":{"type":"{{{(directory == "left" ? "string" : "integer")}}}"}}"""); + } + var result = await Load(Document(""" + {"Left":{"$ref":"left/a.json#/components/schemas/A"},"Right":{"$ref":"right/a.json#/components/schemas/A"}} + """), Settings(loader)); + + Assert.NotNull(result.Document); + Assert.Empty(result.Diagnostic.Errors); + Assert.Equal(4, loader.Requests.Count); + Assert.Equal(JsonSchemaType.String, result.Document.Components.Schemas["Left"].Type); + Assert.Equal(JsonSchemaType.Integer, result.Document.Components.Schemas["Right"].Type); + } + + [Fact] + public async Task MultipleFragmentsAreAllCheckedForAliasCycles() + { + var loader = new RecordingLoader(); + loader.Documents["http://example.test/a.json"] = Document(""" + {"Valid":{"type":"string"},"Cycle":{"$ref":"a.json#/components/schemas/Cycle"}} + """); + var result = await Load(Document(""" + {"First":{"$ref":"a.json#/components/schemas/Valid"},"Second":{"$ref":"a.json#/components/schemas/Cycle"}} + """), Settings(loader)); + Assert.Null(result.Document); + Assert.Single(loader.Requests); + Assert.Contains(result.Diagnostic.Errors, error => error.Message.Contains("Cycle", StringComparison.Ordinal)); + } + + [Fact] + public async Task ValidRecursivePropertiesAcrossDocumentsArePreserved() + { + var loader = new RecordingLoader(); + loader.Documents["http://example.test/a.json"] = Document(""" + {"A":{"type":"object","properties":{"b":{"$ref":"b.json#/components/schemas/B"}}}} + """); + loader.Documents["http://example.test/b.json"] = Document(""" + {"B":{"type":"object","properties":{"a":{"$ref":"a.json#/components/schemas/A"}}}} + """); + var result = await Load(Document("""{"Main":{"$ref":"a.json#/components/schemas/A"}}"""), Settings(loader)); + + Assert.NotNull(result.Document); + Assert.Empty(result.Diagnostic.Errors); + Assert.Equal(2, loader.Requests.Count); + var schemaA = Assert.IsType(result.Document.Components.Schemas["Main"]).RecursiveTarget; + var schemaB = Assert.IsType(schemaA.Properties["b"]).RecursiveTarget; + Assert.Same(schemaA, Assert.IsType(schemaB.Properties["a"]).RecursiveTarget); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task HttpLoadingReusesTheKnownRootForValidRecursiveReferences(bool overrideBaseUrl) + { + var loader = new RecordingLoader(); + loader.Documents["http://example.test/main.json"] = Document(""" + {"Root":{"type":"object","properties":{"a":{"$ref":"http://example.test/a.json#/components/schemas/A"}}}} + """); + loader.Documents["http://example.test/a.json"] = Document(""" + {"A":{"type":"object","properties":{"root":{"$ref":"main.json#/components/schemas/Root"}}}} + """); + using var client = new HttpClient(new RecordingHandler(loader)); + var settings = new OpenApiReaderSettings { HttpClient = client, LoadExternalRefs = true }; + if (overrideBaseUrl) settings.BaseUrl = new("http://example.test/context/main.json"); + var result = await OpenApiDocument.LoadAsync("http://example.test/main.json", settings, TestContext.Current.CancellationToken); + + Assert.NotNull(result.Document); + Assert.Empty(result.Diagnostic.Errors); + Assert.Equal(2, loader.Requests.Count); + var root = result.Document.Components.Schemas["Root"]; + var schemaA = Assert.IsType(root.Properties["a"]).RecursiveTarget; + Assert.Same(root, Assert.IsType(schemaA.Properties["root"]).RecursiveTarget); + } + + [Fact] + public async Task ResponseReferenceOnlyCyclesAlsoFail() + { + var loader = new RecordingLoader(); + var document = """ + {"openapi":"3.0.0","info":{"title":"Test","version":"1.0"},"paths":{}, + "components":{"responses":{"A":{"$ref":"a.json#/components/responses/A"}}}} + """; + loader.Documents["http://example.test/a.json"] = document; + var result = await Load(document, Settings(loader)); + Assert.Null(result.Document); + Assert.Single(loader.Requests); + Assert.Contains(result.Diagnostic.Errors, error => error.Message.Contains("Circular external", StringComparison.Ordinal)); + } + + [Fact] + public async Task LocalOnlyAliasCyclesKeepTheirExistingWarnings() + { + var result = await Load(Document(""" + {"A":{"$ref":"#/components/schemas/B"},"B":{"$ref":"#/components/schemas/A"}} + """), Settings(new RecordingLoader())); + Assert.NotNull(result.Document); + Assert.Empty(result.Diagnostic.Errors); + Assert.Contains(result.Diagnostic.Warnings, warning => warning.Message.Contains("Circular reference", StringComparison.Ordinal)); + } + + [Fact] + public async Task UriPathsAndQueriesRemainDistinctDocumentIdentities() + { + var loader = new RecordingLoader(); + loader.Documents["http://example.test/A.json"] = Document("""{"A":{"type":"string"}}"""); + loader.Documents["http://example.test/a.json"] = Document("""{"A":{"type":"integer"}}"""); + loader.Documents["http://example.test/a.json?version=1"] = Document("""{"A":{"type":"boolean"}}"""); + var result = await Load(Document(""" + {"Upper":{"$ref":"A.json#/components/schemas/A"}, + "Lower":{"$ref":"a.json#/components/schemas/A"}, + "Query":{"$ref":"a.json?version=1#/components/schemas/A"}} + """), Settings(loader)); + Assert.NotNull(result.Document); + Assert.Empty(result.Diagnostic.Errors); + Assert.Equal(3, loader.Requests.Count); + Assert.Equal(JsonSchemaType.String, result.Document.Components.Schemas["Upper"].Type); + Assert.Equal(JsonSchemaType.Integer, result.Document.Components.Schemas["Lower"].Type); + Assert.Equal(JsonSchemaType.Boolean, result.Document.Components.Schemas["Query"].Type); + } + + [Theory] + [InlineData("3.1.0")] + [InlineData("3.2.0")] + public async Task AuthoredSchemaSiblingsArePreservedAndTheirReferencesAreLoaded(string version) + { + var loader = new RecordingLoader(); + loader.Documents["http://example.test/a.json"] = Document(""" + {"A":{"$ref":"a.json#/components/schemas/A","type":"object","properties":{"child":{"$ref":"child.json#/components/schemas/A"}}}} + """, version); + loader.Documents["http://example.test/child.json"] = Document("""{"A":{"type":"string"}}""", version); + var result = await Load(Document("""{"Main":{"$ref":"a.json#/components/schemas/A"}}""", version), Settings(loader)); + + Assert.NotNull(result.Document); + Assert.Empty(result.Diagnostic.Errors); + Assert.Equal(2, loader.Requests.Count); + Assert.Equal(JsonSchemaType.Object, result.Document.Components.Schemas["Main"].Type); + Assert.Equal(JsonSchemaType.String, result.Document.Components.Schemas["Main"].Properties["child"].Type); + } + + [Fact] + public async Task WideGraphsDoNotConsumeTheDepthBudgetAcrossSiblings() + { + var loader = new RecordingLoader(); + var schemas = new Dictionary(); + for (var i = 0; i < 70; i++) + { + loader.Documents[$"http://example.test/{i}.json"] = Document("""{"A":{"type":"object"}}"""); + schemas[$"Schema{i}"] = new Dictionary { ["$ref"] = $"{i}.json#/components/schemas/A" }; + } + var settings = Settings(loader); + settings.MaxExternalReferenceDepth = 1; + var result = await Load(Document(JsonSerializer.Serialize(schemas)), settings); + + Assert.NotNull(result.Document); + Assert.Empty(result.Diagnostic.Errors); + Assert.Equal(70, loader.Requests.Count); + } + + [Fact] + public async Task DisabledExternalLoadingMakesNoRequests() + { + var loader = new RecordingLoader(); + var settings = Settings(loader); + settings.LoadExternalRefs = false; + var result = await Load(Document("""{"Main":{"$ref":"a.json#/components/schemas/A"}}"""), settings); + Assert.NotNull(result.Document); + Assert.Empty(loader.Requests); + } + + [Fact] + public async Task ReusingSettingsDoesNotShareLoadedDocuments() + { + var loader = Chain(1); + var settings = Settings(loader); + var root = Document("""{"Main":{"$ref":"0.json#/components/schemas/A"}}"""); + var results = await Task.WhenAll(Load(root, settings), Load(root, settings)); + Assert.All(results, result => Assert.NotNull(result.Document)); + Assert.NotSame(results[0].Document.Workspace, results[1].Document.Workspace); + Assert.Equal(2, loader.Requests.Count); + } + + [Theory] + [InlineData(false, 1)] + [InlineData(true, 1)] + [InlineData(false, 2)] + [InlineData(true, 2)] + public async Task StreamsAreDisposedAccordingToOwnership(bool leaveOpen, int length) + { + var loader = Chain(length); + var settings = Settings(loader); + settings.MaxExternalReferenceDepth = 1; + settings.LeaveStreamOpen = leaveOpen; + using var root = new MemoryStream(Encoding.UTF8.GetBytes(Document("""{"Main":{"$ref":"0.json#/components/schemas/A"}}"""))); + var result = await OpenApiDocument.LoadAsync(root, "json", settings, TestContext.Current.CancellationToken); + + Assert.Equal(length == 1, result.Document is not null); + Assert.Equal(leaveOpen, root.CanRead); + Assert.All(loader.Streams, stream => Assert.False(stream.CanRead)); + } + + [Fact] + public async Task CancellationDisposesStreamsAndDoesNotBecomeADiagnostic() + { + var loader = Chain(2); + using var cancellation = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + loader.OnLoad = () => + { + if (loader.Requests.Count == 2) cancellation.Cancel(); + }; + using var root = new MemoryStream(Encoding.UTF8.GetBytes(Document("""{"Main":{"$ref":"0.json#/components/schemas/A"}}"""))); + await Assert.ThrowsAnyAsync(() => + OpenApiDocument.LoadAsync(root, "json", Settings(loader), cancellation.Token)); + Assert.False(root.CanRead); + Assert.All(loader.Streams, stream => Assert.False(stream.CanRead)); + } + + private static string Document(string schemas, string version = "3.0.0") => $$$""" + {"openapi":"{{{version}}}","info":{"title":"Test","version":"1.0"},"paths":{},"components":{"schemas":{{{schemas}}}}} + """; + + private static OpenApiReaderSettings Settings(RecordingLoader loader) + { + var settings = new OpenApiReaderSettings + { + LoadExternalRefs = true, + CustomExternalLoader = loader, + BaseUrl = new("http://example.test/main.json") + }; + settings.AddYamlReader(); + return settings; + } + + private static async Task Load(string document, OpenApiReaderSettings settings, string format = "json") + { + using var stream = new MemoryStream(Encoding.UTF8.GetBytes(document)); + return await OpenApiDocument.LoadAsync(stream, format, settings, TestContext.Current.CancellationToken); + } + + private static RecordingLoader Chain(int length) + { + var loader = new RecordingLoader(); + for (var i = 0; i < length; i++) + { + loader.Documents[$"http://example.test/{i}.json"] = Document(i + 1 < length ? + $$$"""{"A":{"$ref":"{{{i + 1}}}.json#/components/schemas/A"}}""" : """{"A":{"type":"object"}}"""); + } + return loader; + } + + private sealed class RecordingLoader : IStreamLoader + { + public Dictionary Documents { get; } = new(); + public List Requests { get; } = new(); + public List Streams { get; } = new(); + public Action OnLoad { get; set; } + + public Task LoadAsync(Uri baseUrl, Uri uri, CancellationToken cancellationToken = default) + { + lock (Requests) + { + if (Requests.Count >= 1000) throw new InvalidOperationException("Test request budget exceeded."); + var absolute = new Uri(baseUrl, uri); + Requests.Add(absolute); + OnLoad?.Invoke(); + cancellationToken.ThrowIfCancellationRequested(); + var stream = new MemoryStream(Encoding.UTF8.GetBytes(Documents[absolute.AbsoluteUri])); + Streams.Add(stream); + return Task.FromResult(stream); + } + } + } + + private sealed class RecordingHandler(RecordingLoader loader) : HttpMessageHandler + { + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + var stream = await loader.LoadAsync(new Uri("http://example.test/"), request.RequestUri, cancellationToken); + var response = new HttpResponseMessage(System.Net.HttpStatusCode.OK) { Content = new StreamContent(stream) }; + response.Content.Headers.ContentType = new("application/json"); + return response; + } + } + [Fact] public async Task LoadingDocumentWithResolveAllReferencesShouldLoadDocumentIntoWorkspaceAsync() { diff --git a/test/Microsoft.OpenApi.Tests/Reader/OpenApiReaderSettingsTests.cs b/test/Microsoft.OpenApi.Tests/Reader/OpenApiReaderSettingsTests.cs index fdc211b2c..d3f30f959 100644 --- a/test/Microsoft.OpenApi.Tests/Reader/OpenApiReaderSettingsTests.cs +++ b/test/Microsoft.OpenApi.Tests/Reader/OpenApiReaderSettingsTests.cs @@ -20,6 +20,15 @@ public void Defensive() Assert.Throws(() => settings.TryAddReader("json", null)); } + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void RejectsNonPositiveExternalReferenceDepth(int depth) + { + var settings = new OpenApiReaderSettings(); + Assert.Throws(() => settings.MaxExternalReferenceDepth = depth); + } + [Fact] public void Defaults() { From f70fc29cf143ba7f6f4a99ea911b14939543eff4 Mon Sep 17 00:00:00 2001 From: "Gavin Barron (from Dev Box)" Date: Fri, 6 Nov 2026 13:57:58 -0800 Subject: [PATCH 2/2] fix(reader): refine external ref file loading Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 357913a9-48cd-4007-897f-54956d3171aa --- .../Reader/Services/DefaultStreamLoader.cs | 2 +- .../OpenApiRemoteReferenceCollector.cs | 59 ++++++++++++++-- .../Reader/Services/OpenApiWorkspaceLoader.cs | 3 +- .../Services/OpenApiWorkspace.cs | 2 +- .../OpenApiWorkspaceStreamTests.cs | 70 ++++++++++++++++++- .../Reader/OpenApiModelFactoryTests.cs | 14 ++++ 6 files changed, 139 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.OpenApi/Reader/Services/DefaultStreamLoader.cs b/src/Microsoft.OpenApi/Reader/Services/DefaultStreamLoader.cs index f7bb00450..68915b518 100644 --- a/src/Microsoft.OpenApi/Reader/Services/DefaultStreamLoader.cs +++ b/src/Microsoft.OpenApi/Reader/Services/DefaultStreamLoader.cs @@ -47,7 +47,7 @@ internal static Uri ResolveUri(Uri baseUrl, Uri uri) { var resolved = uri.IsAbsoluteUri ? uri : baseUrl.AbsoluteUri.Equals(OpenApiConstants.BaseRegistryUri) ? - new Uri(Path.Combine(Directory.GetCurrentDirectory(), uri.ToString())) : + new Uri(new Uri(Directory.GetCurrentDirectory() + Path.DirectorySeparatorChar), uri) : new Uri(baseUrl, uri); return new UriBuilder(resolved) { Fragment = string.Empty }.Uri; } diff --git a/src/Microsoft.OpenApi/Reader/Services/OpenApiRemoteReferenceCollector.cs b/src/Microsoft.OpenApi/Reader/Services/OpenApiRemoteReferenceCollector.cs index d545e6e3d..af5f0d6ca 100644 --- a/src/Microsoft.OpenApi/Reader/Services/OpenApiRemoteReferenceCollector.cs +++ b/src/Microsoft.OpenApi/Reader/Services/OpenApiRemoteReferenceCollector.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. using System.Collections.Generic; +using System.Runtime.CompilerServices; namespace Microsoft.OpenApi.Reader { @@ -11,6 +12,7 @@ namespace Microsoft.OpenApi.Reader internal class OpenApiRemoteReferenceCollector : OpenApiVisitorBase { private readonly Dictionary _references = new(); + private readonly HashSet _schemas = new(SchemaIdentityComparer.Instance); internal List ReferenceHolders { get; } = new(); /// @@ -27,19 +29,55 @@ public IEnumerable References /// public override void Visit(IOpenApiReferenceHolder referenceHolder) { - ReferenceHolders.Add(referenceHolder); - var reference = GetReference(referenceHolder); - AddExternalReferences(reference); - if (reference is JsonSchemaReference schema) + if (referenceHolder is IOpenApiSchema schema) { - var walker = new OpenApiWalker(this); - foreach (var child in OpenApiWorkspace.EnumerateChildren(schema)) + Visit(schema); + return; + } + + AddReference(referenceHolder); + } + + /// + public override void Visit(IOpenApiSchema schema) + { + var pending = new Stack(); + pending.Push(schema); + while (pending.Count > 0) + { + var current = pending.Pop(); + if (!_schemas.Add(current)) + { + continue; + } + + IEnumerable children; + if (current is OpenApiSchemaReference reference) { - walker.Walk(child); + AddReference(reference); + children = OpenApiWorkspace.EnumerateChildren(reference.Reference); + } + else + { + children = OpenApiWorkspace.EnumerateChildren(current); + } + foreach (var child in children) + { + if (child is not null) + { + pending.Push(child); + } } } } + private void AddReference(IOpenApiReferenceHolder referenceHolder) + { + ReferenceHolders.Add(referenceHolder); + var reference = GetReference(referenceHolder); + AddExternalReferences(reference); + } + internal static BaseOpenApiReference GetReference(IOpenApiReferenceHolder referenceHolder) => referenceHolder switch { @@ -61,5 +99,12 @@ private void AddExternalReferences(BaseOpenApiReference? reference) _references.Add(externalResource, reference); } } + + private sealed class SchemaIdentityComparer : IEqualityComparer + { + public static SchemaIdentityComparer Instance { get; } = new(); + public bool Equals(IOpenApiSchema? x, IOpenApiSchema? y) => ReferenceEquals(x, y); + public int GetHashCode(IOpenApiSchema obj) => RuntimeHelpers.GetHashCode(obj); + } } } diff --git a/src/Microsoft.OpenApi/Reader/Services/OpenApiWorkspaceLoader.cs b/src/Microsoft.OpenApi/Reader/Services/OpenApiWorkspaceLoader.cs index 7385b683a..40c37bda5 100644 --- a/src/Microsoft.OpenApi/Reader/Services/OpenApiWorkspaceLoader.cs +++ b/src/Microsoft.OpenApi/Reader/Services/OpenApiWorkspaceLoader.cs @@ -144,7 +144,8 @@ private static bool ValidateReferenceChains(List refere target = reference.HostDocument?.ResolveReference(reference, reference.IsExternal, parentSchema); } } - catch (InvalidOperationException ex) + catch (Exception ex) when (ex is InvalidOperationException || + (ex is IndexOutOfRangeException or ArgumentOutOfRangeException) && chain.Any(static item => item.IsExternal)) { if (chain.Any(static item => item.IsExternal)) { diff --git a/src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs b/src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs index f7a2c47a2..96c478dd8 100644 --- a/src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs +++ b/src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs @@ -570,7 +570,7 @@ private void RegisterAnchorsRecursive(OpenApiDocument document, IOpenApiSchema? /// Enumerates all child schemas from an IOpenApiSchema, including properties from /// IOpenApiSchemaMissingProperties (Contains, If/Then/Else, etc.). /// - private static IEnumerable EnumerateChildren(IOpenApiSchema s) + internal static IEnumerable EnumerateChildren(IOpenApiSchema s) { if (s.Definitions is not null) foreach (var c in s.Definitions.Values) yield return c; diff --git a/test/Microsoft.OpenApi.Readers.Tests/OpenApiWorkspaceTests/OpenApiWorkspaceStreamTests.cs b/test/Microsoft.OpenApi.Readers.Tests/OpenApiWorkspaceTests/OpenApiWorkspaceStreamTests.cs index 024f122ed..53b582021 100644 --- a/test/Microsoft.OpenApi.Readers.Tests/OpenApiWorkspaceTests/OpenApiWorkspaceStreamTests.cs +++ b/test/Microsoft.OpenApi.Readers.Tests/OpenApiWorkspaceTests/OpenApiWorkspaceStreamTests.cs @@ -82,8 +82,10 @@ public async Task ExternalDepthIsCheckedBeforeFetchingBeyondTheLimit(int length, Assert.Equal(Math.Min(length, limit), loader.Requests.Count); if (succeeds) { + var document = result.Document; + Assert.NotNull(document); Assert.Empty(result.Diagnostic.Errors); - var reference = Assert.IsType(result.Document.Components.Schemas["Main"]); + var reference = Assert.IsType(document.Components.Schemas["Main"]); Assert.Equal(JsonSchemaType.Object, reference.RecursiveTarget.Type); } else @@ -279,6 +281,72 @@ public async Task AuthoredSchemaSiblingsArePreservedAndTheirReferencesAreLoaded( Assert.Equal(JsonSchemaType.String, result.Document.Components.Schemas["Main"].Properties["child"].Type); } + [Theory] + [InlineData("$defs", false)] + [InlineData("$defs", true)] + [InlineData("patternProperties", true)] + [InlineData("contains", true)] + public async Task ExternalReferencesBeneathConcreteSchemaChildrenAreLoaded(string keyword, bool hasReferenceSibling) + { + var loader = new RecordingLoader(); + var childReference = new Dictionary { ["$ref"] = "child.json#/components/schemas/A" }; + var wrapper = new Dictionary + { + [keyword] = keyword == "contains" ? childReference : + new Dictionary { ["Child"] = childReference } + }; + var schema = new Dictionary + { + ["type"] = "object", + ["properties"] = new Dictionary { ["wrapper"] = wrapper } + }; + if (hasReferenceSibling) + { + schema["$ref"] = "a.json#/components/schemas/A"; + } + loader.Documents["http://example.test/a.json"] = + Document(JsonSerializer.Serialize(new Dictionary { ["A"] = schema }), "3.1.0"); + loader.Documents["http://example.test/child.json"] = Document("""{"A":{"type":"string"}}""", "3.1.0"); + var result = await Load(Document("""{"Main":{"$ref":"a.json#/components/schemas/A"}}""", "3.1.0"), Settings(loader)); + + var document = result.Document; + Assert.NotNull(document); + Assert.Empty(result.Diagnostic.Errors); + Assert.Equal(2, loader.Requests.Count); + var wrapperSchema = document.Components.Schemas["Main"].Properties["wrapper"]; + var childSchema = keyword switch + { + "$defs" => wrapperSchema.Definitions["Child"], + "patternProperties" => wrapperSchema.PatternProperties["Child"], + _ => ((IOpenApiSchemaMissingProperties)wrapperSchema).Contains + }; + Assert.Equal(JsonSchemaType.String, Assert.IsType(childSchema).RecursiveTarget.Type); + } + + [Theory] + [InlineData("properties")] + [InlineData("allOf")] + [InlineData("allOf/-1")] + [InlineData("anyOf/-1")] + [InlineData("oneOf/-1")] + public async Task MalformedExternalSubschemaPointersReturnDiagnostics(string pointer) + { + var loader = new RecordingLoader(); + loader.Documents["http://example.test/a.json"] = Document(""" + {"A":{"type":"object","properties":{"name":{"type":"string"}}, + "allOf":[{"type":"object"}],"anyOf":[{"type":"object"}],"oneOf":[{"type":"object"}]}} + """, "3.1.0"); + var settings = Settings(loader); + settings.RuleSet = new ValidationRuleSet(); + var result = await Load(Document($$$"""{"Main":{"$ref":"a.json#/components/schemas/A/{{{pointer}}}"}}""", "3.1.0"), settings); + + Assert.Null(result.Document); + Assert.Single(loader.Requests); + Assert.Contains(result.Diagnostic.Errors, error => + error.Message.Contains("Could not resolve external reference chain", StringComparison.Ordinal) && + error.Message.Contains(pointer, StringComparison.Ordinal)); + } + [Fact] public async Task WideGraphsDoNotConsumeTheDepthBudgetAcrossSiblings() { diff --git a/test/Microsoft.OpenApi.Tests/Reader/OpenApiModelFactoryTests.cs b/test/Microsoft.OpenApi.Tests/Reader/OpenApiModelFactoryTests.cs index f7e9e0431..36916d26f 100644 --- a/test/Microsoft.OpenApi.Tests/Reader/OpenApiModelFactoryTests.cs +++ b/test/Microsoft.OpenApi.Tests/Reader/OpenApiModelFactoryTests.cs @@ -9,6 +9,20 @@ namespace Microsoft.OpenApi.Tests.Reader; public class OpenApiModelFactoryTests { + [Theory] + [InlineData("a.json")] + [InlineData("../a.json")] + [InlineData("/a.json")] + [InlineData("\\a.json")] + public void DefaultRegistryReferencesResolveAgainstCurrentDirectory(string resource) + { + var expected = new Uri(Path.GetFullPath(resource.Replace('\\', Path.DirectorySeparatorChar))); + var actual = DefaultStreamLoader.ResolveUri(new Uri(OpenApiConstants.BaseRegistryUri), + new Uri(resource, UriKind.RelativeOrAbsolute)); + + Assert.Equal(expected.AbsoluteUri, actual.AbsoluteUri); + } + [Fact] public async Task LoadDocumentWithCircularSchemaPropertyReferencesShouldSucceed() {