Skip to content

Commit 52ccf5f

Browse files
gavinbarrongavinbarron
committed
fixes based on review comments
1 parent 9384eef commit 52ccf5f

8 files changed

Lines changed: 83 additions & 341 deletions

File tree

‎src/Microsoft.OpenApi.Readers/BoundedYamlDocumentParser.cs‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,35 @@ internal sealed class BoundedYamlDocumentParser
2828
private YamlNode _root;
2929

3030
public BoundedYamlDocumentParser(OpenApiReaderSettings settings)
31-
{
32-
_budget = new(
31+
: this(
3332
settings.MaxDepth,
3433
settings.MaxNodeCount,
35-
settings.MaxAliasExpansionNodeCount);
36-
_maxScalarLength = settings.MaxScalarLength;
37-
_maxDepth = settings.MaxDepth;
34+
settings.MaxAliasExpansionNodeCount,
35+
settings.MaxScalarLength)
36+
{
37+
}
38+
39+
public BoundedYamlDocumentParser(ParsingContext context)
40+
: this(
41+
context.MaxDepth,
42+
context.MaxNodeCount,
43+
context.MaxAliasExpansionNodeCount,
44+
context.MaxScalarLength)
45+
{
46+
}
47+
48+
private BoundedYamlDocumentParser(
49+
uint maxDepth,
50+
uint maxNodeCount,
51+
uint maxAliasExpansionNodeCount,
52+
uint maxScalarLength)
53+
{
54+
_budget = new(
55+
maxDepth,
56+
maxNodeCount,
57+
maxAliasExpansionNodeCount);
58+
_maxScalarLength = maxScalarLength;
59+
_maxDepth = maxDepth;
3860
}
3961

4062
public YamlDocument Parse(

‎src/Microsoft.OpenApi.Readers/InputLimitTextReader.cs‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@ namespace Microsoft.OpenApi.Readers
1111
/// </summary>
1212
internal sealed class InputLimitTextReader : TextReader
1313
{
14+
private const char MaxOneByteUtf8Value = '\u007F';
15+
private const char MaxTwoByteUtf8Value = '\u07FF';
16+
private const uint OneByteUtf8Length = 1;
17+
private const uint TwoByteUtf8Length = 2;
18+
private const uint ThreeByteUtf8Length = 3;
19+
private const uint SurrogatePairUtf8Length = 4;
20+
1421
private readonly TextReader _inner;
1522
private readonly uint _maxByteCount;
1623
private ulong _byteCount;
@@ -58,18 +65,23 @@ public override int Read(char[] buffer, int index, int count)
5865
return charsRead;
5966
}
6067

68+
/// <summary>
69+
/// Charges the UTF-8 encoded length of one UTF-16 code unit. Valid surrogate pairs are
70+
/// charged as one four-byte code point; unpaired surrogates use the three-byte UTF-8
71+
/// replacement-character length used by the default .NET encoder fallback.
72+
/// </summary>
6173
private void Charge(char value)
6274
{
6375
if (_hasPendingHighSurrogate)
6476
{
6577
if (char.IsLowSurrogate(value))
6678
{
67-
AddBytes(4);
79+
AddBytes(SurrogatePairUtf8Length);
6880
_hasPendingHighSurrogate = false;
6981
return;
7082
}
7183

72-
AddBytes(3);
84+
AddBytes(ThreeByteUtf8Length);
7385
_hasPendingHighSurrogate = false;
7486
}
7587

@@ -79,19 +91,19 @@ private void Charge(char value)
7991
}
8092
else if (char.IsLowSurrogate(value))
8193
{
82-
AddBytes(3);
94+
AddBytes(ThreeByteUtf8Length);
8395
}
84-
else if (value <= 0x7F)
96+
else if (value <= MaxOneByteUtf8Value)
8597
{
86-
AddBytes(1);
98+
AddBytes(OneByteUtf8Length);
8799
}
88-
else if (value <= 0x7FF)
100+
else if (value <= MaxTwoByteUtf8Value)
89101
{
90-
AddBytes(2);
102+
AddBytes(TwoByteUtf8Length);
91103
}
92104
else
93105
{
94-
AddBytes(3);
106+
AddBytes(ThreeByteUtf8Length);
95107
}
96108
}
97109

@@ -105,7 +117,7 @@ private void ChargeEndOfInput()
105117
_endCharged = true;
106118
if (_hasPendingHighSurrogate)
107119
{
108-
AddBytes(3);
120+
AddBytes(ThreeByteUtf8Length);
109121
_hasPendingHighSurrogate = false;
110122
}
111123
}

‎src/Microsoft.OpenApi.Readers/OpenApiYamlDocumentReader.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ public OpenApiDocument Read(YamlDocument input, out OpenApiDiagnostic diagnostic
5151
MaxDepth = _settings.MaxDepth,
5252
MaxNodeCount = _settings.MaxNodeCount,
5353
MaxAliasExpansionNodeCount = _settings.MaxAliasExpansionNodeCount,
54+
MaxInputByteCount = _settings.MaxInputByteCount,
5455
MaxScalarLength = _settings.MaxScalarLength
5556
};
5657

@@ -99,6 +100,7 @@ public async Task<ReadResult> ReadAsync(YamlDocument input, CancellationToken ca
99100
MaxDepth = _settings.MaxDepth,
100101
MaxNodeCount = _settings.MaxNodeCount,
101102
MaxAliasExpansionNodeCount = _settings.MaxAliasExpansionNodeCount,
103+
MaxInputByteCount = _settings.MaxInputByteCount,
102104
MaxScalarLength = _settings.MaxScalarLength
103105
};
104106

@@ -196,6 +198,7 @@ public T ReadFragment<T>(YamlDocument input, OpenApiSpecVersion version, out Ope
196198
MaxDepth = _settings.MaxDepth,
197199
MaxNodeCount = _settings.MaxNodeCount,
198200
MaxAliasExpansionNodeCount = _settings.MaxAliasExpansionNodeCount,
201+
MaxInputByteCount = _settings.MaxInputByteCount,
199202
MaxScalarLength = _settings.MaxScalarLength
200203
};
201204

‎src/Microsoft.OpenApi.Readers/ParsingContext.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ public class ParsingContext
2929
internal uint MaxDepth { get; set; } = OpenApiReaderSettings.DefaultMaxDepth;
3030
internal uint MaxNodeCount { get; set; } = OpenApiReaderSettings.DefaultMaxNodeCount;
3131
internal uint MaxAliasExpansionNodeCount { get; set; } = OpenApiReaderSettings.DefaultMaxAliasExpansionNodeCount;
32+
internal uint MaxInputByteCount { get; set; } = OpenApiReaderSettings.DefaultMaxInputByteCount;
3233
internal uint MaxScalarLength { get; set; } = OpenApiReaderSettings.DefaultMaxScalarLength;
3334
internal Dictionary<string, Func<IOpenApiAny, OpenApiSpecVersion, IOpenApiExtension>> ExtensionParsers { get; set; } = new();
3435
internal RootNode RootNode { get; set; }

‎src/Microsoft.OpenApi.Readers/YamlHelper.cs‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,11 @@ public static string GetScalarValue(
3131

3232
public static YamlNode ParseYamlString(
3333
string yamlString,
34-
ParsingContext context = null)
34+
ParsingContext context)
3535
{
36-
var settings = new OpenApiReaderSettings();
37-
if (context != null)
38-
{
39-
settings.MaxDepth = context.MaxDepth;
40-
settings.MaxNodeCount = context.MaxNodeCount;
41-
settings.MaxAliasExpansionNodeCount = context.MaxAliasExpansionNodeCount;
42-
settings.MaxScalarLength = context.MaxScalarLength;
43-
}
44-
4536
using var reader = new StringReader(yamlString);
46-
return new BoundedYamlDocumentParser(settings)
47-
.Parse(reader, settings.MaxInputByteCount, CancellationToken.None)
37+
return new BoundedYamlDocumentParser(context)
38+
.Parse(reader, context.MaxInputByteCount, CancellationToken.None)
4839
.RootNode;
4940
}
5041
}

‎src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs‎

Lines changed: 21 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
using Microsoft.OpenApi.Models;
55
using Microsoft.OpenApi.Properties;
66
using System.Collections.Generic;
7-
using System.Linq;
8-
using System.Runtime.CompilerServices;
97

108
namespace Microsoft.OpenApi.Validations.Rules
119
{
@@ -91,17 +89,28 @@ public static class OpenApiSchemaRules
9189
/// between other schemas which may satisfy the payload description.</param>
9290
public static bool ValidateChildSchemaAgainstDiscriminator(OpenApiSchema schema, string discriminatorName)
9391
{
94-
if (discriminatorName == null)
92+
if (!schema.Required?.Contains(discriminatorName) ?? false)
9593
{
96-
return false;
94+
// recursively check nested schema.OneOf, schema.AnyOf or schema.AllOf and their required fields for the discriminator
95+
if (schema.OneOf.Count != 0)
96+
{
97+
return TraverseSchemaElements(discriminatorName, schema.OneOf);
98+
}
99+
if (schema.AnyOf.Count != 0)
100+
{
101+
return TraverseSchemaElements(discriminatorName, schema.AnyOf);
102+
}
103+
if (schema.AllOf.Count != 0)
104+
{
105+
return TraverseSchemaElements(discriminatorName, schema.AllOf);
106+
}
97107
}
98-
99-
if (schema.Required?.Contains(discriminatorName) == true)
108+
else
100109
{
101110
return true;
102111
}
103112

104-
return TraverseSchemaElementsIterative(discriminatorName, GetSchemaCombinators(schema));
113+
return false;
105114
}
106115

107116
/// <summary>
@@ -112,88 +121,21 @@ public static bool ValidateChildSchemaAgainstDiscriminator(OpenApiSchema schema,
112121
/// <param name="childSchema">The child schema.</param>
113122
/// <returns></returns>
114123
public static bool TraverseSchemaElements(string discriminatorName, IList<OpenApiSchema> childSchema)
115-
=> TraverseSchemaElementsIterative(discriminatorName, childSchema);
116-
117-
private static bool TraverseSchemaElementsIterative(
118-
string discriminatorName,
119-
IEnumerable<OpenApiSchema> childSchemas)
120124
{
121-
if (childSchemas == null)
122-
{
123-
return false;
124-
}
125-
126-
var schemasToVisit = new Queue<OpenApiSchema>();
127-
var visitedSchemas = new HashSet<OpenApiSchema>(SchemaReferenceEqualityComparer.Instance);
128-
129-
EnqueueSchemas(schemasToVisit, childSchemas);
130-
131-
while (schemasToVisit.Count > 0)
125+
foreach (var childItem in childSchema)
132126
{
133-
var childItem = schemasToVisit.Dequeue();
134-
if (!visitedSchemas.Add(childItem))
127+
if ((!childItem.Properties?.ContainsKey(discriminatorName) ?? false) &&
128+
(!childItem.Required?.Contains(discriminatorName) ?? false))
135129
{
136-
continue;
130+
return ValidateChildSchemaAgainstDiscriminator(childItem, discriminatorName);
137131
}
138-
139-
if (childItem.Properties?.ContainsKey(discriminatorName) == true ||
140-
childItem.Required?.Contains(discriminatorName) == true)
132+
else
141133
{
142134
return true;
143135
}
144-
145-
EnqueueSchemas(schemasToVisit, GetSchemaCombinators(childItem));
146136
}
147137

148138
return false;
149139
}
150-
151-
private static IEnumerable<OpenApiSchema> GetSchemaCombinators(OpenApiSchema schema)
152-
{
153-
if (schema.OneOf != null)
154-
{
155-
foreach (var childSchema in schema.OneOf)
156-
{
157-
yield return childSchema;
158-
}
159-
}
160-
161-
if (schema.AnyOf != null)
162-
{
163-
foreach (var childSchema in schema.AnyOf)
164-
{
165-
yield return childSchema;
166-
}
167-
}
168-
169-
if (schema.AllOf != null)
170-
{
171-
foreach (var childSchema in schema.AllOf)
172-
{
173-
yield return childSchema;
174-
}
175-
}
176-
}
177-
178-
private static void EnqueueSchemas(
179-
Queue<OpenApiSchema> schemasToVisit,
180-
IEnumerable<OpenApiSchema> childSchemas)
181-
{
182-
foreach (var childSchema in childSchemas.Where(childSchema => childSchema != null))
183-
{
184-
schemasToVisit.Enqueue(childSchema);
185-
}
186-
}
187-
188-
private sealed class SchemaReferenceEqualityComparer : IEqualityComparer<OpenApiSchema>
189-
{
190-
internal static SchemaReferenceEqualityComparer Instance { get; } = new();
191-
192-
public bool Equals(OpenApiSchema x, OpenApiSchema y)
193-
=> ReferenceEquals(x, y);
194-
195-
public int GetHashCode(OpenApiSchema obj)
196-
=> RuntimeHelpers.GetHashCode(obj);
197-
}
198140
}
199141
}

‎test/Microsoft.OpenApi.Readers.Tests/YamlAliasExpansionTests.cs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ public class YamlAliasExpansionTests
3333
[Fact]
3434
public void ExponentialAliasExpansionIsRejected()
3535
{
36-
Assert.Throws<OpenApiReaderException>(() => YamlHelper.ParseYamlString(YamlBomb));
36+
var context = new ParsingContext(new());
37+
38+
Assert.Throws<OpenApiReaderException>(() => YamlHelper.ParseYamlString(YamlBomb, context));
3739
}
3840

3941
[Fact]
@@ -43,8 +45,9 @@ public void ExcessiveNestingDepthIsRejected()
4345
// converter from stack exhaustion.
4446
const int depth = 70;
4547
var deeplyNested = new string('[', depth) + new string(']', depth);
48+
var context = new ParsingContext(new());
4649

47-
Assert.Throws<SharpYaml.YamlException>(() => YamlHelper.ParseYamlString(deeplyNested));
50+
Assert.Throws<SharpYaml.YamlException>(() => YamlHelper.ParseYamlString(deeplyNested, context));
4851
}
4952

5053
[Fact]
@@ -77,8 +80,9 @@ public void LegitimateAliasesStillConvert()
7780
a: &val hello
7881
b: *val
7982
""";
83+
var context = new ParsingContext(new());
8084

81-
var node = ParseNode.Create(new(new()), YamlHelper.ParseYamlString(input));
85+
var node = ParseNode.Create(context, YamlHelper.ParseYamlString(input, context));
8286

8387
var anyObject = Assert.IsType<OpenApiObject>(node.CreateAny());
8488
Assert.Equal("hello", ((OpenApiString)anyObject["a"]).Value);

0 commit comments

Comments
 (0)