Skip to content

Commit a3acaf4

Browse files
fix: yaml anchor preprocessor bug for mapping and sequence objects (#628)
* too many objects has been read from the anchor if followed by sequence / mapping
1 parent 9a38014 commit a3acaf4

2 files changed

Lines changed: 129 additions & 8 deletions

File tree

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
using GitHub.DistributedTask.Expressions2;
2+
using GitHub.DistributedTask.ObjectTemplating;
3+
using GitHub.DistributedTask.Pipelines.ContextData;
4+
using GitHub.DistributedTask.Pipelines.ObjectTemplating;
5+
6+
namespace Runner.Server
7+
{
8+
public class ActionsTests
9+
{
10+
private static TemplateContext CreateTemplateContext(GitHub.DistributedTask.ObjectTemplating.ITraceWriter traceWriter) {
11+
ExpressionFlags flags = ExpressionFlags.None;
12+
13+
var templateContext = new TemplateContext() {
14+
Flags = flags,
15+
CancellationToken = CancellationToken.None,
16+
Errors = new TemplateValidationErrors(10, 500),
17+
Memory = new TemplateMemory(
18+
maxDepth: 100,
19+
maxEvents: 1000000,
20+
maxBytes: 10 * 1024 * 1024),
21+
TraceWriter = traceWriter,
22+
Schema = PipelineTemplateSchemaFactory.GetSchema()
23+
};
24+
return templateContext;
25+
}
26+
27+
[Fact]
28+
public void TestParseYamlAnchorsComplex()
29+
{
30+
using var content = new StringReader(@"
31+
name: 'Test Workflow'
32+
on: [push, pull_request]
33+
jobs:
34+
build:
35+
runs-on: &name ubuntu-latest
36+
steps:
37+
- &anchor
38+
name: Checkout
39+
uses: actions/checkout@v4
40+
- name: Run Tests
41+
run: echo 'Running tests...'
42+
build2:
43+
runs-on: ubuntu-latest
44+
steps:
45+
- *anchor
46+
- *anchor
47+
- *anchor
48+
- name: Run Tests
49+
run: echo 'Running tests...'
50+
*name:
51+
runs-on: ubuntu-latest
52+
steps:
53+
- *anchor
54+
- *anchor
55+
- *anchor
56+
");
57+
58+
YamlObjectReader reader = new YamlObjectReader(null, content, true);
59+
var ctx = CreateTemplateContext(new EmptyTraceWriter());
60+
var result = TemplateReader.Read(ctx, "workflow-root", reader, null, out _);
61+
Assert.Equal(0, ctx.Errors.Count);
62+
Assert.NotNull(result);
63+
string f = result.ToContextData().ToJToken().ToString();
64+
Console.WriteLine(f);
65+
}
66+
67+
[Fact]
68+
public void TestParseYamlAnchorsSimple()
69+
{
70+
using var content = new StringReader(@"
71+
- &anchor
72+
key: value
73+
- *anchor
74+
");
75+
76+
YamlObjectReader reader = new YamlObjectReader(null, content, true);
77+
var ctx = CreateTemplateContext(new EmptyTraceWriter());
78+
var result = TemplateReader.Read(ctx, "any", reader, null, out _);
79+
Assert.Equal(0, ctx.Errors.Count);
80+
Assert.NotNull(result);
81+
string f = result.ToContextData().ToJToken().ToString();
82+
Console.WriteLine(f);
83+
}
84+
85+
[Fact]
86+
public void TestParseYamlAnchorsSimpleSplitByMapping()
87+
{
88+
using var content = new StringReader(@"
89+
a:
90+
- &anchor
91+
key: value
92+
b:
93+
- *anchor
94+
");
95+
96+
YamlObjectReader reader = new YamlObjectReader(null, content, true);
97+
var ctx = CreateTemplateContext(new EmptyTraceWriter());
98+
var result = TemplateReader.Read(ctx, "any", reader, null, out _);
99+
Assert.Equal(0, ctx.Errors.Count);
100+
Assert.NotNull(result);
101+
string f = result.ToContextData().ToJToken().ToString();
102+
Console.WriteLine(f);
103+
}
104+
}
105+
}

src/Sdk/DTPipelines/Pipelines/ObjectTemplating/YamlAnchorParser.cs

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ private void Merge()
7070
events.Add(cevent);
7171
}
7272

73-
if(yamlMerge) {
73+
if (yamlMerge)
74+
{
7475
// Disable Merge Operator by default
7576
foreach (var node in events)
7677
{
@@ -95,7 +96,10 @@ private void Merge()
9596
}
9697
}
9798
}
99+
// Broken here already
100+
DumpState();
98101
events.CleanMarked();
102+
DumpState();
99103
foreach (var node in events)
100104
{
101105
var m_current = node.Value;
@@ -120,10 +124,20 @@ private void Merge()
120124
// Verify not using achors
121125
if (sequenceStart.Anchor != null)
122126
{
123-
node.Value = new SequenceStart(null, sequenceStart.Tag, sequenceStart.IsImplicit, sequenceStart.Style, sequenceStart.Start, sequenceStart.End);
127+
node.Value = new SequenceStart(null, sequenceStart.Tag, sequenceStart.IsImplicit, sequenceStart.Style, sequenceStart.Start, sequenceStart.End);
124128
}
125129
}
126130
}
131+
DumpState();
132+
}
133+
134+
private void DumpState()
135+
{
136+
List<string> evs = new List<string>();
137+
foreach (var reference in events)
138+
{
139+
evs.Add(reference.Value.ToString());
140+
}
127141
}
128142

129143
private bool HandleMerge(LinkedListNode<ParsingEvent> node)
@@ -181,6 +195,7 @@ private bool HandleAnchorAlias(LinkedListNode<ParsingEvent> node, LinkedListNode
181195
private bool HandleAnchorAlias2(LinkedListNode<ParsingEvent> node, LinkedListNode<ParsingEvent> anchorNode, AnchorAlias anchorAlias)
182196
{
183197
var mergedEvents = GetEvents(anchorAlias.Value).ToArray();
198+
// Broken here already
184199

185200
events.AddAfter(node, mergedEvents);
186201
events.MarkDeleted(anchorNode);
@@ -225,17 +240,18 @@ private IEnumerable<ParsingEvent> GetEvents(AnchorName anchor)
225240
{
226241
var cloner = new ParsingEventCloner();
227242
var nesting = 0;
228-
bool prev = true;
243+
bool shouldStop = false;
229244

230245
return events.FromAnchor(anchor)
231246
.Select(e => e.Value)
232247
.TakeWhile(e => {
233-
var now = (nesting += e.NestingIncrease) >= 1;
234-
if(!now && prev) {
235-
prev = false;
236-
return true;
248+
if (shouldStop)
249+
{
250+
return false;
237251
}
238-
return now;
252+
// We want to stop after the closing pair
253+
shouldStop |= (nesting += e.NestingIncrease) <= 0;
254+
return true;
239255
})
240256
.Select(e => cloner.Clone(e));
241257
}

0 commit comments

Comments
 (0)