Skip to content

Commit 368c0c0

Browse files
authored
Fix aligned buffer write (#11861)
* Fix aligned buffer write This is a particularly subtle bug that requires a chunk (or remaining bit of a chunk) and the destination buffer being _exactly_ the same size. When this happens, the following sequence occurs: 1. `source.Length` and `destination.Length` are exactly aligned. This means that we do not enter the `if` on line 450, and `endOfChunkWritten` will be true. 2. We slice `destination` by `source.Length`. This leaves us with a `destination` that has 0 elements, and `IsEmpty` is true. 3. Because `endOfChunkWritten` is true, we enter the `if` on line 466. This increments us to the next chunk. 4. On line 472 (in the before diff), `destination.IsEmpty` is true, so we enter the `if`, and break out of the loop, never reseting `charIndex` to 0. 5. 4 happens again, this time on line 480 (in the before diff). Again, we do not reset `charIndex` to 0. To fix this, we want to unconditionally reset `charIndex` when we increment a chunk, not wait until after the destination has been checked for empty. * Ensure that no more than `count` characters are read into the destination buffer. * Add tests. * Simplify code around chunk tracking * .NET Framework build fixes * .NET FRAMEWORK!!!!!!!!!!!!!!!!!!!!!!!!1 * Feedback
1 parent c971e1c commit 368c0c0

File tree

2 files changed

+59
-19
lines changed

2 files changed

+59
-19
lines changed

src/Compiler/Microsoft.AspNetCore.Razor.Language/test/CodeGeneration/CSharpCodeWriterTest.cs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System;
77
using System.Collections.Generic;
88
using Microsoft.AspNetCore.Razor.Language.Intermediate;
9+
using Roslyn.Test.Utilities;
910
using Xunit;
1011

1112
namespace Microsoft.AspNetCore.Razor.Language.CodeGeneration;
@@ -439,4 +440,46 @@ class C
439440
440441
""", output);
441442
}
443+
444+
[Fact, WorkItem("https://github.com/dotnet/core/issues/9885")]
445+
public void AlignedPages_WritesCorrectlyWhenPageAndBufferAreAligned()
446+
{
447+
var pages = new LinkedList<ReadOnlyMemory<char>[]>();
448+
449+
const string FirstLine = "First Line";
450+
pages.AddLast([(FirstLine + FirstLine).AsMemory(), "Second".AsMemory()]);
451+
452+
var testReader = CodeWriter.GetTestTextReader(pages);
453+
var output = new char[FirstLine.Length];
454+
455+
testReader.Read(output, 0, output.Length);
456+
Assert.Equal(FirstLine, string.Join("", output));
457+
Array.Clear(output, 0, output.Length);
458+
459+
testReader.Read(output, 0, output.Length);
460+
Assert.Equal(FirstLine, string.Join("", output));
461+
Array.Clear(output, 0, output.Length);
462+
463+
testReader.Read(output, 0, output.Length);
464+
Assert.Equal("Second\0\0\0\0", string.Join("", output));
465+
}
466+
467+
[Fact]
468+
public void ReaderOnlyReadsAsMuchAsRequested()
469+
{
470+
var pages = new LinkedList<ReadOnlyMemory<char>[]>();
471+
472+
const string FirstLine = "First Line";
473+
pages.AddLast([FirstLine.AsMemory()]);
474+
475+
var testReader = CodeWriter.GetTestTextReader(pages);
476+
var output = new char[FirstLine.Length];
477+
478+
testReader.Read(output, 0, 2);
479+
Assert.Equal("Fi\0\0\0\0\0\0\0\0", string.Join("", output));
480+
Array.Clear(output, 0, output.Length);
481+
482+
testReader.Read(output, 0, output.Length);
483+
Assert.Equal("rst Line\0\0", string.Join("", output));
484+
}
442485
}

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/CodeGeneration/CodeWriter.cs

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -323,14 +323,20 @@ public CodeWriter WriteLine([InterpolatedStringHandlerArgument("")] ref WriteInt
323323

324324
public SourceText GetText()
325325
{
326-
using var reader = new Reader(this);
326+
using var reader = new Reader(_pages, Length);
327327
return SourceText.From(reader, Length, Encoding.UTF8);
328328
}
329329

330-
private sealed class Reader(CodeWriter codeWriter) : TextReader
330+
// Internal for testing
331+
internal static TextReader GetTestTextReader(LinkedList<ReadOnlyMemory<char>[]> pages)
331332
{
332-
private LinkedListNode<ReadOnlyMemory<char>[]>? _page = codeWriter._pages.First;
333-
private int _remainingLength = codeWriter.Length;
333+
return new Reader(pages, pages.Count);
334+
}
335+
336+
private sealed class Reader(LinkedList<ReadOnlyMemory<char>[]> pages, int length) : TextReader
337+
{
338+
private LinkedListNode<ReadOnlyMemory<char>[]>? _page = pages.First;
339+
private int _remainingLength = length;
334340
private int _chunkIndex;
335341
private int _charIndex;
336342

@@ -413,7 +419,7 @@ public override int Read(char[] buffer, int index, int count)
413419
return -1;
414420
}
415421

416-
var destination = buffer.AsSpan(index);
422+
var destination = buffer.AsSpan(index, count);
417423
var charsWritten = 0;
418424

419425
var page = _page;
@@ -443,38 +449,29 @@ public override int Read(char[] buffer, int index, int count)
443449
}
444450
}
445451

446-
var endOfChunkWritten = true;
447-
448452
// Are we about to write past the end of the buffer? If so, adjust source.
449453
// This will be the last chunk we write, so be sure to update charIndex.
450454
if (source.Length > destination.Length)
451455
{
452456
source = source[..destination.Length];
453457
charIndex += source.Length;
454-
455-
// There's more to this chunk to write! Note this so that we don't update
456-
// chunkIndex later.
457-
endOfChunkWritten = false;
458+
}
459+
else
460+
{
461+
chunkIndex++;
462+
charIndex = 0;
458463
}
459464

460465
source.CopyTo(destination);
461466
destination = destination[source.Length..];
462467

463468
charsWritten += source.Length;
464469

465-
// Be careful not to increment chunkIndex unless we actually wrote to the end of the chunk.
466-
if (endOfChunkWritten)
467-
{
468-
chunkIndex++;
469-
}
470-
471470
// Break if we are done writing. chunkIndex and charIndex should have their correct values at this point.
472471
if (destination.IsEmpty)
473472
{
474473
break;
475474
}
476-
477-
charIndex = 0;
478475
}
479476

480477
if (destination.IsEmpty)

0 commit comments

Comments
 (0)