Skip to content

Commit 9fa157e

Browse files
committed
fix(serialization): potential infinite loop when inlining references
1 parent 00b5675 commit 9fa157e

9 files changed

Lines changed: 277 additions & 4 deletions

File tree

‎src/Microsoft.OpenApi/Extensions/OpenAPIWriterExtensions.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,13 @@ internal static OpenApiWriterSettings GetSettings(this IOpenApiWriter openApiWri
1515
}
1616
return new();
1717
}
18+
19+
internal static void CountInlineReference(this IOpenApiWriter openApiWriter)
20+
{
21+
if (openApiWriter is OpenApiWriterBase @base)
22+
{
23+
@base.CountInlineReference();
24+
}
25+
}
1826
}
1927
}

‎src/Microsoft.OpenApi/Models/References/BaseOpenApiReferenceHolder.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ protected BaseOpenApiReferenceHolder(string referenceId, OpenApiDocument? hostDo
207207
/// <inheritdoc/>
208208
public virtual void SerializeAsV3(IOpenApiWriter writer)
209209
{
210-
if (!writer.GetSettings().ShouldInlineReference(Reference)
210+
if (!writer.GetSettings().ShouldInlineReference(Reference)
211211
|| Reference.Type == ReferenceType.Tag) // tags are held as references need to drop in.
212212
{
213213
Reference.SerializeAsV3(writer);
@@ -267,6 +267,7 @@ private protected void SerializeInternal(IOpenApiWriter writer,
267267
Action<IOpenApiWriter, U> action)
268268
{
269269
Utils.CheckArgumentNull(writer);
270+
writer.CountInlineReference();
270271
ApplyToTarget(element => action(writer, element));
271272
}
272273
}

‎src/Microsoft.OpenApi/Models/References/OpenApiSchemaReference.cs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,8 +256,14 @@ private void SerializeAsWithoutLoops(IOpenApiWriter writer, Action<IOpenApiWrite
256256
}
257257
else
258258
{
259-
SerializeInternal(writer, (w, element) => action(w, element));
260-
writer.GetSettings().LoopDetector.PopLoop<IOpenApiSchema>();
259+
try
260+
{
261+
SerializeInternal(writer, (w, element) => action(w, element));
262+
}
263+
finally
264+
{
265+
writer.GetSettings().LoopDetector.PopLoop<IOpenApiSchema>();
266+
}
261267
}
262268

263269
}

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

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/Microsoft.OpenApi/Properties/SRResource.resx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,9 @@
132132
<data name="IndentationLevelInvalid" xml:space="preserve">
133133
<value>Indentation level cannot be lower than 0.</value>
134134
</data>
135+
<data name="MaximumInlineReferenceCountExceeded" xml:space="preserve">
136+
<value>The maximum number of inline references ({0}) has been exceeded.</value>
137+
</data>
135138
<data name="InputItemShouldBeType" xml:space="preserve">
136139
<value>The input item should be in type of '{0}'.</value>
137140
</data>
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
11
#nullable enable
22
Microsoft.OpenApi.OpenApiDocument.GetOperationById(string! operationId) -> Microsoft.OpenApi.OpenApiOperation?
3+
Microsoft.OpenApi.OpenApiWriterSettings.MaxInlineReferenceCount.get -> int
4+
Microsoft.OpenApi.OpenApiWriterSettings.MaxInlineReferenceCount.set -> void

‎src/Microsoft.OpenApi/Writers/OpenApiWriterBase.cs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ public abstract class OpenApiWriterBase : IOpenApiWriter
3535
/// Number which specifies the level of indentation.
3636
/// </summary>
3737
private int _indentLevel;
38+
private int _inlineReferenceCount;
3839

3940
/// <summary>
4041
/// Initializes a new instance of the <see cref="OpenApiWriterBase"/> class.
@@ -70,6 +71,20 @@ protected OpenApiWriterBase(TextWriter textWriter, OpenApiWriterSettings? settin
7071
/// </summary>
7172
protected TextWriter Writer { get; }
7273

74+
internal void CountInlineReference()
75+
{
76+
if (_inlineReferenceCount >= Settings.MaxInlineReferenceCount)
77+
{
78+
throw new OpenApiWriterException(
79+
string.Format(
80+
CultureInfo.InvariantCulture,
81+
SRResource.MaximumInlineReferenceCountExceeded,
82+
Settings.MaxInlineReferenceCount));
83+
}
84+
85+
_inlineReferenceCount++;
86+
}
87+
7388
/// <summary>
7489
/// Write start object.
7590
/// </summary>
@@ -319,7 +334,7 @@ public virtual void WriteIndentation()
319334
Writer.Write(IndentationString);
320335
}
321336
}
322-
337+
323338
/// <summary>
324339
/// Get current scope.
325340
/// </summary>

‎src/Microsoft.OpenApi/Writers/OpenApiWriterSettings.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55
/// </summary>
66
public class OpenApiWriterSettings
77
{
8+
private const int DefaultMaxInlineReferenceCount = 10_000;
9+
810
internal LoopDetector LoopDetector { get; } = new();
11+
912
/// <summary>
1013
/// Indicates if local references should be rendered as an inline object
1114
/// </summary>
@@ -16,6 +19,12 @@ public class OpenApiWriterSettings
1619
/// </summary>
1720
public bool InlineExternalReferences { get; set; }
1821

22+
/// <summary>
23+
/// Gets or sets the maximum number of references that can be expanded inline by a writer.
24+
/// </summary>
25+
/// <remarks>The default is 10,000.</remarks>
26+
public int MaxInlineReferenceCount { get; set; } = DefaultMaxInlineReferenceCount;
27+
1928
internal bool ShouldInlineReference(BaseOpenApiReference reference)
2029
{
2130
return (reference.IsLocal && InlineLocalReferences)
Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT license.
3+
4+
using System;
5+
using System.Globalization;
6+
using System.IO;
7+
using System.Text;
8+
using Xunit;
9+
10+
namespace Microsoft.OpenApi.Tests.Writers
11+
{
12+
public class OpenApiWriterInlineReferenceTests
13+
{
14+
private const int InlineReferenceLimit = 128;
15+
private const int OutputCharacterLimit = 100_000;
16+
17+
[Theory]
18+
[InlineData(OpenApiSpecVersion.OpenApi2_0, false)]
19+
[InlineData(OpenApiSpecVersion.OpenApi2_0, true)]
20+
[InlineData(OpenApiSpecVersion.OpenApi3_0, false)]
21+
[InlineData(OpenApiSpecVersion.OpenApi3_0, true)]
22+
[InlineData(OpenApiSpecVersion.OpenApi3_1, false)]
23+
[InlineData(OpenApiSpecVersion.OpenApi3_1, true)]
24+
[InlineData(OpenApiSpecVersion.OpenApi3_2, false)]
25+
[InlineData(OpenApiSpecVersion.OpenApi3_2, true)]
26+
public void SerializeWithRepeatedInlineReferencesStopsBeforeWriterLimit(
27+
OpenApiSpecVersion specVersion,
28+
bool useYaml)
29+
{
30+
var readResult = OpenApiDocument.Parse(CreateDocumentWithRepeatedReferences(18), "json");
31+
Assert.Empty(readResult.Diagnostic.Errors);
32+
Assert.NotNull(readResult.Document);
33+
34+
using var textWriter = new CharacterLimitedTextWriter(OutputCharacterLimit);
35+
IOpenApiWriter writer = useYaml
36+
? new OpenApiYamlWriter(textWriter, CreateWriterSettings(InlineReferenceLimit))
37+
: new OpenApiJsonWriter(textWriter, CreateWriterSettings(InlineReferenceLimit));
38+
39+
var exception = Assert.Throws<OpenApiWriterException>(
40+
() => Serialize(readResult.Document, writer, specVersion));
41+
Assert.Contains(InlineReferenceLimit.ToString(CultureInfo.InvariantCulture), exception.Message);
42+
}
43+
44+
[Theory]
45+
[InlineData(OpenApiSpecVersion.OpenApi2_0)]
46+
[InlineData(OpenApiSpecVersion.OpenApi3_0)]
47+
[InlineData(OpenApiSpecVersion.OpenApi3_1)]
48+
[InlineData(OpenApiSpecVersion.OpenApi3_2)]
49+
public void SerializeWithInlineReferencesBelowLimitWorks(OpenApiSpecVersion specVersion)
50+
{
51+
var readResult = OpenApiDocument.Parse(CreateDocumentWithRepeatedReferences(4), "json");
52+
Assert.Empty(readResult.Diagnostic.Errors);
53+
Assert.NotNull(readResult.Document);
54+
55+
using var textWriter = new StringWriter(CultureInfo.InvariantCulture);
56+
var writer = new OpenApiJsonWriter(textWriter, CreateWriterSettings(32));
57+
58+
Serialize(readResult.Document, writer, specVersion);
59+
60+
Assert.NotEmpty(textWriter.ToString());
61+
}
62+
63+
[Fact]
64+
public void ReusingSettingsWithNewWriterStartsANewInlineReferenceCount()
65+
{
66+
var largeReadResult = OpenApiDocument.Parse(CreateDocumentWithRepeatedReferences(6), "json");
67+
Assert.Empty(largeReadResult.Diagnostic.Errors);
68+
Assert.NotNull(largeReadResult.Document);
69+
var smallReadResult = OpenApiDocument.Parse(CreateDocumentWithRepeatedReferences(4), "json");
70+
Assert.Empty(smallReadResult.Diagnostic.Errors);
71+
Assert.NotNull(smallReadResult.Document);
72+
var settings = CreateWriterSettings(32);
73+
74+
using var firstOutput = new StringWriter(CultureInfo.InvariantCulture);
75+
Assert.Throws<OpenApiWriterException>(
76+
() => Serialize(
77+
largeReadResult.Document,
78+
new OpenApiJsonWriter(firstOutput, settings),
79+
OpenApiSpecVersion.OpenApi3_0));
80+
81+
using var secondOutput = new StringWriter(CultureInfo.InvariantCulture);
82+
Serialize(
83+
smallReadResult.Document,
84+
new OpenApiJsonWriter(secondOutput, settings),
85+
OpenApiSpecVersion.OpenApi3_0);
86+
87+
Assert.DoesNotContain("$ref", secondOutput.ToString());
88+
}
89+
90+
private static string CreateDocumentWithRepeatedReferences(int levels)
91+
{
92+
var schemas = new StringBuilder();
93+
for (var i = 0; i < levels; i++)
94+
{
95+
if (i > 0)
96+
{
97+
schemas.Append(',');
98+
}
99+
100+
schemas
101+
.Append("\"s")
102+
.Append(i.ToString(CultureInfo.InvariantCulture))
103+
.Append("\":{\"allOf\":[{\"$ref\":\"#/components/schemas/s")
104+
.Append((i + 1).ToString(CultureInfo.InvariantCulture))
105+
.Append("\"},{\"$ref\":\"#/components/schemas/s")
106+
.Append((i + 1).ToString(CultureInfo.InvariantCulture))
107+
.Append("\"}]}");
108+
}
109+
110+
schemas
111+
.Append(",\"s")
112+
.Append(levels.ToString(CultureInfo.InvariantCulture))
113+
.Append("\":{\"type\":\"string\"}");
114+
115+
return $$"""
116+
{
117+
"openapi": "3.0.0",
118+
"info": {
119+
"title": "Repeated references",
120+
"version": "1.0.0"
121+
},
122+
"paths": {
123+
"/test": {
124+
"get": {
125+
"responses": {
126+
"200": {
127+
"description": "OK",
128+
"content": {
129+
"application/json": {
130+
"schema": {
131+
"$ref": "#/components/schemas/s0"
132+
}
133+
}
134+
}
135+
}
136+
}
137+
}
138+
}
139+
},
140+
"components": {
141+
"schemas": {
142+
{{schemas}}
143+
}
144+
}
145+
}
146+
""";
147+
}
148+
149+
private static void Serialize(
150+
OpenApiDocument document,
151+
IOpenApiWriter writer,
152+
OpenApiSpecVersion specVersion)
153+
{
154+
switch (specVersion)
155+
{
156+
case OpenApiSpecVersion.OpenApi2_0:
157+
document.SerializeAsV2(writer);
158+
break;
159+
case OpenApiSpecVersion.OpenApi3_0:
160+
document.SerializeAsV3(writer);
161+
break;
162+
case OpenApiSpecVersion.OpenApi3_1:
163+
document.SerializeAsV31(writer);
164+
break;
165+
case OpenApiSpecVersion.OpenApi3_2:
166+
document.SerializeAsV32(writer);
167+
break;
168+
default:
169+
throw new ArgumentOutOfRangeException(nameof(specVersion), specVersion, null);
170+
}
171+
}
172+
173+
private static OpenApiWriterSettings CreateWriterSettings(int maxInlineReferenceCount)
174+
{
175+
return new OpenApiWriterSettings
176+
{
177+
InlineLocalReferences = true,
178+
MaxInlineReferenceCount = maxInlineReferenceCount
179+
};
180+
}
181+
182+
private sealed class CharacterLimitedTextWriter(int characterLimit) : TextWriter
183+
{
184+
private int _characterCount;
185+
186+
public override Encoding Encoding => Encoding.UTF8;
187+
188+
public override void Write(char value)
189+
{
190+
CountCharacters(1);
191+
}
192+
193+
public override void Write(string value)
194+
{
195+
CountCharacters(value?.Length ?? 0);
196+
}
197+
198+
public override void Write(char[] buffer, int index, int count)
199+
{
200+
CountCharacters(count);
201+
}
202+
203+
public override void Write(ReadOnlySpan<char> buffer)
204+
{
205+
CountCharacters(buffer.Length);
206+
}
207+
208+
private void CountCharacters(int count)
209+
{
210+
_characterCount += count;
211+
if (_characterCount > characterLimit)
212+
{
213+
throw new CharacterLimitExceededException();
214+
}
215+
}
216+
}
217+
218+
private sealed class CharacterLimitExceededException : Exception;
219+
}
220+
}

0 commit comments

Comments
 (0)