Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net8.0</TargetFramework>
<TargetFramework>net10.0</TargetFramework>
<PublishAot>true</PublishAot>
<EmitCompilerGeneratedFiles>true</EmitCompilerGeneratedFiles>
<Nullable>enable</Nullable>
Expand Down
3 changes: 2 additions & 1 deletion YamlDotNet.Fsharp.Test/YamlDotNet.Fsharp.Test.fsproj
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>net8.0;net47</TargetFrameworks>
<TargetFrameworks Condition="$([MSBuild]::IsOSPlatform('Windows'))">net10.0;net8.0;net4.7</TargetFrameworks>
<TargetFrameworks Condition="$([MSBuild]::IsOSPlatform('Linux'))">net10.0;net8.0</TargetFrameworks>
Comment thread
EdwardCooke marked this conversation as resolved.
Outdated
<IsPackable>false</IsPackable>
<AssemblyOriginatorKeyFile>..\YamlDotNet.snk</AssemblyOriginatorKeyFile>
<SignAssembly>true</SignAssembly>
Expand Down
3 changes: 2 additions & 1 deletion YamlDotNet.Samples.Fsharp/YamlDotNet.Samples.Fsharp.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net8.0</TargetFramework>
<TargetFrameworks Condition="$([MSBuild]::IsOSPlatform('Windows'))">net10.0;net8.0;net4.7</TargetFrameworks>
<TargetFrameworks Condition="$([MSBuild]::IsOSPlatform('Linux'))">net10.0;net8.0</TargetFrameworks>
Comment thread
EdwardCooke marked this conversation as resolved.
Outdated
<IsPackable>false</IsPackable>
<LangVersion></LangVersion>
<WarningLevel>5</WarningLevel>
Expand Down
2 changes: 1 addition & 1 deletion YamlDotNet.Samples/YamlDotNet.Samples.csproj
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net8.0</TargetFramework>
<TargetFramework>net10.0</TargetFramework>
<IsPackable>false</IsPackable>
</PropertyGroup>

Expand Down
82 changes: 82 additions & 0 deletions YamlDotNet.Test/Core/StringInterningTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// This file is part of YamlDotNet - A .NET library for YAML.
// Copyright (c) Antoine Aubry and contributors
//
// Permission is hereby granted, free of charge, to any person obtaining a copy of
// this software and associated documentation files (the "Software"), to deal in
// the Software without restriction, including without limitation the rights to
// use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
// of the Software, and to permit persons to whom the Software is furnished to do
// so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in all
// copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.

using System;
using Xunit;
using YamlDotNet.Core;
using YamlDotNet.Core.Events;

namespace YamlDotNet.Test.Core
{
public class StringInterningTests
{
[Fact]
public void AnchorNameDoesNotInternUniqueValues()
{
var value = UniqueValue("anchor");
Assert.Null(string.IsInterned(value));

var anchor = new AnchorName(value);

Assert.Equal(value, anchor.Value);
Assert.Null(string.IsInterned(value));
}

[Fact]
public void TagNameDoesNotInternUniqueValues()
{
var value = "!" + UniqueValue("tag");
Assert.Null(string.IsInterned(value));

var tag = new TagName(value);

Assert.Equal(value, tag.Value);
Assert.Null(string.IsInterned(value));
}

[Fact]
public void ScalarKeyDoesNotInternUniqueValues()
{
var value = UniqueValue("key");
Assert.Null(string.IsInterned(value));

var scalar = new Scalar(
AnchorName.Empty,
TagName.Empty,
value,
ScalarStyle.Plain,
isPlainImplicit: true,
isQuotedImplicit: false,
Mark.Empty,
Mark.Empty,
isKey: true);

Assert.True(scalar.IsKey);
Assert.Equal(value, scalar.Value);
Assert.Null(string.IsInterned(value));
}

private static string UniqueValue(string prefix)
{
return string.Concat(prefix, "_", Guid.NewGuid().ToString("N"));
}
}
}
134 changes: 134 additions & 0 deletions YamlDotNet.Test/Serialization/MergingParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,13 @@
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq.Expressions;
Comment thread
EdwardCooke marked this conversation as resolved.
Outdated
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using FluentAssertions;
using Xunit;
using YamlDotNet.Core;
Expand Down Expand Up @@ -151,5 +156,134 @@ public void MergingParserWithNestedSequence_ShouldNotThrowException()

new SerializerBuilder().Build().Serialize(yamlObject!).NormalizeNewLines().Should().Be(etalonMergedYaml);
}

[Fact]
public async Task MergingParserWithMergeKeyBomb_ShouldThrowExceptionWhenTooManyEvents()
{
// Timebox this test to avoid infinite loops in case of bugs.
// 30 seconds should be more than enough for this test to run even on a slow machine, and if it takes longer than that,
// it's likely that the merging parser is not correctly counting events and enforcing the limit.
var cancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(30));
cancellationTokenSource.Token.Register(() =>
{
throw new TimeoutException("The test took too long, likely due to an infinite loop in the merging parser.");
});

Comment thread
EdwardCooke marked this conversation as resolved.
await Task.Run(() =>
{
var sb = new StringBuilder();

// Base anchor
sb.AppendLine("a0: &a0");
sb.AppendLine(" x: 1");
sb.AppendLine();

// Each level merges the previous anchor TWICE (fanout=2), doubling event count
for (int i = 1; i <= 25; i++)
{
sb.AppendLine($"a{i}: &a{i}");
sb.AppendLine($" <<: *a{i - 1}"); // first merge
sb.AppendLine($" <<: *a{i - 1}"); // second merge
sb.AppendLine();
}

sb.AppendLine("final:");
sb.AppendLine(" <<: *a25");

var yaml = sb.ToString();
var parser = new Parser(new StringReader(yaml));
var mergingParser = new MergingParser(parser, 1000);
try
{
while (mergingParser.MoveNext())
{
//move through everything, we're in a timebox so if this takes too long, the cancellation token will trigger and fail the test
}
}
catch (YamlException ex) when (ex.Message.Contains("Too many events"))
{
// Expected exception, test passes
return;
}
catch (Exception ex)
{
throw new Exception($"Unexpected exception: {ex.Message}");
Comment thread
EdwardCooke marked this conversation as resolved.
Outdated
}
}, cancellationTokenSource.Token);
}

[Fact]
public async Task MergingParserWithManySmallMerges_ShouldThrowExceptionWhenCumulativeEventsExceedLimit()
{
// Timebox this test to avoid infinite loops in case of bugs.
// 30 seconds should be more than enough for this test to run even on a slow machine, and if it takes longer than that,
// it's likely that the merging parser is not correctly counting events and enforcing the limit.
var cancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(30));
cancellationTokenSource.Token.Register(() =>
{
throw new TimeoutException("The test took too long, likely due to an infinite loop in the merging parser.");
});

await Task.Run(() =>
{
var sb = new StringBuilder();
sb.AppendLine("base: &base");
for (var i = 0; i < 25; i++)
{
sb.AppendLine($" k{i}: v{i}");
}

sb.AppendLine();
for (var i = 0; i < 35; i++)
{
sb.AppendLine($"entry{i}:");
sb.AppendLine(" <<: *base");
sb.AppendLine();
}

var parser = new Parser(new StringReader(sb.ToString()));
var mergingParser = new MergingParser(parser, 1000);

Action parse = () =>
{
while (mergingParser.MoveNext())
{
}
};

parse.Should().Throw<YamlException>()
.Where(ex => ex.Message.Contains("Too many events"));
}, cancellationTokenSource.Token);
}

[Fact]
public void MergingParserWithDeepSingleChain_ShouldParseWithinLimit()
{
const int depth = 200;
var sb = new StringBuilder();

sb.AppendLine("a0: &a0");
sb.AppendLine(" root: value");
sb.AppendLine();

for (var i = 1; i <= depth; i++)
{
sb.AppendLine($"a{i}: &a{i}");
sb.AppendLine($" <<: *a{i - 1}");
sb.AppendLine($" level{i}: {i}");
sb.AppendLine();
}

sb.AppendLine("final:");
sb.AppendLine($" <<: *a{depth}");

var parser = new Parser(new StringReader(sb.ToString()));
var mergingParser = new MergingParser(parser, 50000);
var deserializer = new DeserializerBuilder().Build();

var yamlObject = deserializer.Deserialize<Dictionary<string, object>>(mergingParser);

yamlObject.Should().ContainKey("final");
}
}
}
3 changes: 2 additions & 1 deletion YamlDotNet.Test/YamlDotNet.Test.csproj
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>net10.0;net8.0;net47</TargetFrameworks>
<TargetFrameworks Condition="$([MSBuild]::IsOSPlatform('Windows'))">net10.0;net8.0;net4.7</TargetFrameworks>
<TargetFrameworks Condition="$([MSBuild]::IsOSPlatform('Linux'))">net10.0;net8.0</TargetFrameworks>
Comment thread
EdwardCooke marked this conversation as resolved.
Outdated
<IsPackable>false</IsPackable>
<AssemblyOriginatorKeyFile>..\YamlDotNet.snk</AssemblyOriginatorKeyFile>
<SignAssembly>true</SignAssembly>
Expand Down
2 changes: 1 addition & 1 deletion YamlDotNet/Core/AnchorName.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public AnchorName(string value)
$"disallowed characters: []{{}},\nThe value was '{value}'.", nameof(value));
}

this.value = string.Intern(value);
this.value = string.IsInterned(value) ?? value;
}

public override string ToString() => value ?? "[empty]";
Expand Down
5 changes: 1 addition & 4 deletions YamlDotNet/Core/Events/Scalar.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,8 @@ public sealed class Scalar : NodeEvent
public Scalar(AnchorName anchor, TagName tag, string value, ScalarStyle style, bool isPlainImplicit, bool isQuotedImplicit, Mark start, Mark end, bool isKey = false)
: base(anchor, tag, start, end)
{
// In a real client program, keys are likely to be
// interned already. Thus, IsInterned might work
// much better, but you can't really benchmark that.
this.Value = isKey ?
string.Intern(value) :
string.IsInterned(value) ?? value :
value;
this.Style = style;
this.IsPlainImplicit = isPlainImplicit;
Expand Down
23 changes: 19 additions & 4 deletions YamlDotNet/Core/MergingParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,15 @@ public sealed class MergingParser : IParser
private readonly IParser innerParser;
private IEnumerator<LinkedListNode<ParsingEvent>> iterator;
private bool merged;
private readonly int maxParsingEvents;

public MergingParser(IParser innerParser)
public MergingParser(IParser innerParser, int maxParsingEvents = 100000)
{
events = new ParsingEventCollection();
merged = false;
iterator = events.GetEnumerator();
this.innerParser = innerParser;
this.maxParsingEvents = maxParsingEvents;
}
Comment thread
EdwardCooke marked this conversation as resolved.
Outdated
Comment thread
EdwardCooke marked this conversation as resolved.
Outdated

public ParsingEvent? Current => iterator.Current?.Value;
Expand All @@ -64,7 +66,9 @@ private void Merge()
{
while (innerParser.MoveNext())
{
events.Add(innerParser.Current!);
var parsingEvent = innerParser.Current!;
events.Add(parsingEvent);
EnsureMaxParsingEventsNotExceeded(parsingEvent);
}

foreach (var node in events)
Expand Down Expand Up @@ -126,7 +130,7 @@ private bool HandleAnchorAlias(LinkedListNode<ParsingEvent> node, LinkedListNode
{
var mergedEvents = GetMappingEvents(anchorAlias.Value);

events.AddAfter(node, mergedEvents);
events.AddAfter(node, mergedEvents, EnsureMaxParsingEventsNotExceeded);
events.MarkDeleted(anchorNode);

return true;
Expand Down Expand Up @@ -165,6 +169,14 @@ private IEnumerable<ParsingEvent> GetMappingEvents(AnchorName anchor)
.Select(cloner.Clone);
}

private void EnsureMaxParsingEventsNotExceeded(ParsingEvent parsingEvent)
{
if (events.Count > maxParsingEvents)
{
throw new YamlException(parsingEvent.Start, parsingEvent.End, "Too many events, preventing a memory overflow and erroring out.");
Comment thread
EdwardCooke marked this conversation as resolved.
Outdated
}
}

private sealed class ParsingEventCollection : IEnumerable<LinkedListNode<ParsingEvent>>
{
private readonly LinkedList<ParsingEvent> events;
Expand All @@ -178,11 +190,14 @@ public ParsingEventCollection()
references = [];
}

public void AddAfter(LinkedListNode<ParsingEvent> node, IEnumerable<ParsingEvent> items)
public int Count => events.Count;

public void AddAfter(LinkedListNode<ParsingEvent> node, IEnumerable<ParsingEvent> items, Action<ParsingEvent> onItemAdded)
{
foreach (var item in items)
{
node = events.AddAfter(node, item);
onItemAdded(item);
}
}

Expand Down
2 changes: 1 addition & 1 deletion YamlDotNet/Core/TagName.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public TagName(string value)
throw new ArgumentException("Tag value must not be empty.", nameof(value));
}

this.value = string.Intern(value);
this.value = string.IsInterned(value) ?? value;

if (IsGlobal && !Uri.IsWellFormedUriString(value, UriKind.RelativeOrAbsolute))
{
Expand Down
Loading