Skip to content

Commit f70fc29

Browse files
gavinbarronCopilot
gavinbarron
and
Copilot
committed
fix(reader): refine external ref file loading
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 357913a9-48cd-4007-897f-54956d3171aa
1 parent 5f13024 commit f70fc29

6 files changed

Lines changed: 139 additions & 11 deletions

File tree

‎src/Microsoft.OpenApi/Reader/Services/DefaultStreamLoader.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ internal static Uri ResolveUri(Uri baseUrl, Uri uri)
4747
{
4848
var resolved = uri.IsAbsoluteUri ? uri :
4949
baseUrl.AbsoluteUri.Equals(OpenApiConstants.BaseRegistryUri) ?
50-
new Uri(Path.Combine(Directory.GetCurrentDirectory(), uri.ToString())) :
50+
new Uri(new Uri(Directory.GetCurrentDirectory() + Path.DirectorySeparatorChar), uri) :
5151
new Uri(baseUrl, uri);
5252
return new UriBuilder(resolved) { Fragment = string.Empty }.Uri;
5353
}

‎src/Microsoft.OpenApi/Reader/Services/OpenApiRemoteReferenceCollector.cs‎

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT license.
33

44
using System.Collections.Generic;
5+
using System.Runtime.CompilerServices;
56

67
namespace Microsoft.OpenApi.Reader
78
{
@@ -11,6 +12,7 @@ namespace Microsoft.OpenApi.Reader
1112
internal class OpenApiRemoteReferenceCollector : OpenApiVisitorBase
1213
{
1314
private readonly Dictionary<string, BaseOpenApiReference> _references = new();
15+
private readonly HashSet<IOpenApiSchema> _schemas = new(SchemaIdentityComparer.Instance);
1416
internal List<IOpenApiReferenceHolder> ReferenceHolders { get; } = new();
1517

1618
/// <summary>
@@ -27,19 +29,55 @@ public IEnumerable<BaseOpenApiReference> References
2729
/// <inheritdoc/>
2830
public override void Visit(IOpenApiReferenceHolder referenceHolder)
2931
{
30-
ReferenceHolders.Add(referenceHolder);
31-
var reference = GetReference(referenceHolder);
32-
AddExternalReferences(reference);
33-
if (reference is JsonSchemaReference schema)
32+
if (referenceHolder is IOpenApiSchema schema)
3433
{
35-
var walker = new OpenApiWalker(this);
36-
foreach (var child in OpenApiWorkspace.EnumerateChildren(schema))
34+
Visit(schema);
35+
return;
36+
}
37+
38+
AddReference(referenceHolder);
39+
}
40+
41+
/// <inheritdoc/>
42+
public override void Visit(IOpenApiSchema schema)
43+
{
44+
var pending = new Stack<IOpenApiSchema>();
45+
pending.Push(schema);
46+
while (pending.Count > 0)
47+
{
48+
var current = pending.Pop();
49+
if (!_schemas.Add(current))
50+
{
51+
continue;
52+
}
53+
54+
IEnumerable<IOpenApiSchema> children;
55+
if (current is OpenApiSchemaReference reference)
3756
{
38-
walker.Walk(child);
57+
AddReference(reference);
58+
children = OpenApiWorkspace.EnumerateChildren(reference.Reference);
59+
}
60+
else
61+
{
62+
children = OpenApiWorkspace.EnumerateChildren(current);
63+
}
64+
foreach (var child in children)
65+
{
66+
if (child is not null)
67+
{
68+
pending.Push(child);
69+
}
3970
}
4071
}
4172
}
4273

74+
private void AddReference(IOpenApiReferenceHolder referenceHolder)
75+
{
76+
ReferenceHolders.Add(referenceHolder);
77+
var reference = GetReference(referenceHolder);
78+
AddExternalReferences(reference);
79+
}
80+
4381
internal static BaseOpenApiReference GetReference(IOpenApiReferenceHolder referenceHolder)
4482
=> referenceHolder switch
4583
{
@@ -61,5 +99,12 @@ private void AddExternalReferences(BaseOpenApiReference? reference)
6199
_references.Add(externalResource, reference);
62100
}
63101
}
102+
103+
private sealed class SchemaIdentityComparer : IEqualityComparer<IOpenApiSchema>
104+
{
105+
public static SchemaIdentityComparer Instance { get; } = new();
106+
public bool Equals(IOpenApiSchema? x, IOpenApiSchema? y) => ReferenceEquals(x, y);
107+
public int GetHashCode(IOpenApiSchema obj) => RuntimeHelpers.GetHashCode(obj);
108+
}
64109
}
65110
}

‎src/Microsoft.OpenApi/Reader/Services/OpenApiWorkspaceLoader.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,8 @@ private static bool ValidateReferenceChains(List<IOpenApiReferenceHolder> refere
144144
target = reference.HostDocument?.ResolveReference(reference, reference.IsExternal, parentSchema);
145145
}
146146
}
147-
catch (InvalidOperationException ex)
147+
catch (Exception ex) when (ex is InvalidOperationException ||
148+
(ex is IndexOutOfRangeException or ArgumentOutOfRangeException) && chain.Any(static item => item.IsExternal))
148149
{
149150
if (chain.Any(static item => item.IsExternal))
150151
{

‎src/Microsoft.OpenApi/Services/OpenApiWorkspace.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,7 @@ private void RegisterAnchorsRecursive(OpenApiDocument document, IOpenApiSchema?
570570
/// Enumerates all child schemas from an IOpenApiSchema, including properties from
571571
/// IOpenApiSchemaMissingProperties (Contains, If/Then/Else, etc.).
572572
/// </summary>
573-
private static IEnumerable<IOpenApiSchema> EnumerateChildren(IOpenApiSchema s)
573+
internal static IEnumerable<IOpenApiSchema> EnumerateChildren(IOpenApiSchema s)
574574
{
575575
if (s.Definitions is not null)
576576
foreach (var c in s.Definitions.Values) yield return c;

‎test/Microsoft.OpenApi.Readers.Tests/OpenApiWorkspaceTests/OpenApiWorkspaceStreamTests.cs‎

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,10 @@ public async Task ExternalDepthIsCheckedBeforeFetchingBeyondTheLimit(int length,
8282
Assert.Equal(Math.Min(length, limit), loader.Requests.Count);
8383
if (succeeds)
8484
{
85+
var document = result.Document;
86+
Assert.NotNull(document);
8587
Assert.Empty(result.Diagnostic.Errors);
86-
var reference = Assert.IsType<OpenApiSchemaReference>(result.Document.Components.Schemas["Main"]);
88+
var reference = Assert.IsType<OpenApiSchemaReference>(document.Components.Schemas["Main"]);
8789
Assert.Equal(JsonSchemaType.Object, reference.RecursiveTarget.Type);
8890
}
8991
else
@@ -279,6 +281,72 @@ public async Task AuthoredSchemaSiblingsArePreservedAndTheirReferencesAreLoaded(
279281
Assert.Equal(JsonSchemaType.String, result.Document.Components.Schemas["Main"].Properties["child"].Type);
280282
}
281283

284+
[Theory]
285+
[InlineData("$defs", false)]
286+
[InlineData("$defs", true)]
287+
[InlineData("patternProperties", true)]
288+
[InlineData("contains", true)]
289+
public async Task ExternalReferencesBeneathConcreteSchemaChildrenAreLoaded(string keyword, bool hasReferenceSibling)
290+
{
291+
var loader = new RecordingLoader();
292+
var childReference = new Dictionary<string, string> { ["$ref"] = "child.json#/components/schemas/A" };
293+
var wrapper = new Dictionary<string, object>
294+
{
295+
[keyword] = keyword == "contains" ? childReference :
296+
new Dictionary<string, object> { ["Child"] = childReference }
297+
};
298+
var schema = new Dictionary<string, object>
299+
{
300+
["type"] = "object",
301+
["properties"] = new Dictionary<string, object> { ["wrapper"] = wrapper }
302+
};
303+
if (hasReferenceSibling)
304+
{
305+
schema["$ref"] = "a.json#/components/schemas/A";
306+
}
307+
loader.Documents["http://example.test/a.json"] =
308+
Document(JsonSerializer.Serialize(new Dictionary<string, object> { ["A"] = schema }), "3.1.0");
309+
loader.Documents["http://example.test/child.json"] = Document("""{"A":{"type":"string"}}""", "3.1.0");
310+
var result = await Load(Document("""{"Main":{"$ref":"a.json#/components/schemas/A"}}""", "3.1.0"), Settings(loader));
311+
312+
var document = result.Document;
313+
Assert.NotNull(document);
314+
Assert.Empty(result.Diagnostic.Errors);
315+
Assert.Equal(2, loader.Requests.Count);
316+
var wrapperSchema = document.Components.Schemas["Main"].Properties["wrapper"];
317+
var childSchema = keyword switch
318+
{
319+
"$defs" => wrapperSchema.Definitions["Child"],
320+
"patternProperties" => wrapperSchema.PatternProperties["Child"],
321+
_ => ((IOpenApiSchemaMissingProperties)wrapperSchema).Contains
322+
};
323+
Assert.Equal(JsonSchemaType.String, Assert.IsType<OpenApiSchemaReference>(childSchema).RecursiveTarget.Type);
324+
}
325+
326+
[Theory]
327+
[InlineData("properties")]
328+
[InlineData("allOf")]
329+
[InlineData("allOf/-1")]
330+
[InlineData("anyOf/-1")]
331+
[InlineData("oneOf/-1")]
332+
public async Task MalformedExternalSubschemaPointersReturnDiagnostics(string pointer)
333+
{
334+
var loader = new RecordingLoader();
335+
loader.Documents["http://example.test/a.json"] = Document("""
336+
{"A":{"type":"object","properties":{"name":{"type":"string"}},
337+
"allOf":[{"type":"object"}],"anyOf":[{"type":"object"}],"oneOf":[{"type":"object"}]}}
338+
""", "3.1.0");
339+
var settings = Settings(loader);
340+
settings.RuleSet = new ValidationRuleSet();
341+
var result = await Load(Document($$$"""{"Main":{"$ref":"a.json#/components/schemas/A/{{{pointer}}}"}}""", "3.1.0"), settings);
342+
343+
Assert.Null(result.Document);
344+
Assert.Single(loader.Requests);
345+
Assert.Contains(result.Diagnostic.Errors, error =>
346+
error.Message.Contains("Could not resolve external reference chain", StringComparison.Ordinal) &&
347+
error.Message.Contains(pointer, StringComparison.Ordinal));
348+
}
349+
282350
[Fact]
283351
public async Task WideGraphsDoNotConsumeTheDepthBudgetAcrossSiblings()
284352
{

‎test/Microsoft.OpenApi.Tests/Reader/OpenApiModelFactoryTests.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,20 @@ namespace Microsoft.OpenApi.Tests.Reader;
99

1010
public class OpenApiModelFactoryTests
1111
{
12+
[Theory]
13+
[InlineData("a.json")]
14+
[InlineData("../a.json")]
15+
[InlineData("/a.json")]
16+
[InlineData("\\a.json")]
17+
public void DefaultRegistryReferencesResolveAgainstCurrentDirectory(string resource)
18+
{
19+
var expected = new Uri(Path.GetFullPath(resource.Replace('\\', Path.DirectorySeparatorChar)));
20+
var actual = DefaultStreamLoader.ResolveUri(new Uri(OpenApiConstants.BaseRegistryUri),
21+
new Uri(resource, UriKind.RelativeOrAbsolute));
22+
23+
Assert.Equal(expected.AbsoluteUri, actual.AbsoluteUri);
24+
}
25+
1226
[Fact]
1327
public async Task LoadDocumentWithCircularSchemaPropertyReferencesShouldSucceed()
1428
{

0 commit comments

Comments
 (0)