huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix bounds checking in `PagedCharBuffer` and related code 

- #5347
- inconsistent bounds checking caused problems after `ArrayPool<char>` fell back to `new char[2048]`
 - would fail a `Debug` assertion in Debug builds and loop endlessly in Release builds
- change to `CacheTagHelper+CharBufferHtmlContent` is for correctness only
 - always uses a `CharArrayBufferSource` and that returns arrays of the exact size requested
This commit is contained in:
Doug Bunting 2016-10-02 19:47:54 -07:00
parent 7481151086
commit 4e6fd823cf
5 changed files with 341 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/Microsoft.AspNetCore.Mvc.TagHelpers/CacheTagHelper.cs
View File

@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using System.Diagnostics;
using System.IO;
using System.Text;
using System.Text.Encodings.Web;
@ -228,10 +229,13 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
for (var i = 0; i < _buffer.Pages.Count; i++)
{
var pageLength = Math.Min(length, PagedCharBuffer.PageSize);
writer.Write(_buffer.Pages[i], 0, pageLength);
var page = _buffer.Pages[i];
var pageLength = Math.Min(length, page.Length);
writer.Write(page, index: 0, count: pageLength);
length -= pageLength;
}
Debug.Assert(length == 0);
}
}
}

8
src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/PagedBufferedTextWriter.cs
View File

@ -3,6 +3,7 @@
using System;
using System.Buffers;
using System.Diagnostics;
using System.IO;
using System.Text;
using System.Threading.Tasks;
@ -39,15 +40,16 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
for (var i = 0; i < pages.Count; i++)
{
var page = pages[i];
var pageLength = Math.Min(length, PagedCharBuffer.PageSize);
var pageLength = Math.Min(length, page.Length);
if (pageLength != 0)
{
await _inner.WriteAsync(page, 0, pageLength);
await _inner.WriteAsync(page, index: 0, count: pageLength);
}
length -= pageLength;
}
Debug.Assert(length == 0);
_charBuffer.Clear();
}

7
src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/PagedCharBuffer.cs
View File

@ -26,9 +26,9 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
get
{
var length = _charIndex;
if (Pages.Count > 1)
for (var i = 0; i < Pages.Count - 1; i++)
{
length += PageSize * (Pages.Count - 1);
length += Pages[i].Length;
}
return length;
@ -120,8 +120,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
private char[] GetCurrentPage()
{
if (CurrentPage == null ||
_charIndex == PageSize)
if (CurrentPage == null || _charIndex == CurrentPage.Length)
{
CurrentPage = NewPage();
_charIndex = 0;

77
test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/PagedBufferedTextWriterTest.cs
View File

@ -265,6 +265,66 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
Assert.Equal(3, pool.Returned.Count);
}
[Fact]
public async Task FlushAsync_FlushesContent()
{
// Arrange
var pool = new TestArrayPool();
var inner = new StringWriter();
var writer = new PagedBufferedTextWriter(pool, inner);
for (var i = 0; i < Content.Length; i++)
{
writer.Write(Content[i]);
}
// Act
await writer.FlushAsync();
// Assert
Assert.Equal<char>(Content, inner.ToString().ToCharArray());
}
[Fact]
public async Task FlushAsync_WritesContentToInner()
{
// Arrange
var pool = new TestArrayPool();
var inner = new StringWriter();
var writer = new PagedBufferedTextWriter(pool, inner);
for (var i = 0; i < Content.Length; i++)
{
writer.Write(Content[i]);
}
// Act
await writer.FlushAsync();
// Assert
Assert.Equal<char>(Content, inner.ToString().ToCharArray());
}
[Fact]
public async Task FlushAsync_WritesContentToInner_WithLargeArrays()
{
// Arrange
var pool = new RentMoreArrayPool();
var inner = new StringWriter();
var writer = new PagedBufferedTextWriter(pool, inner);
for (var i = 0; i < Content.Length; i++)
{
writer.Write(Content[i]);
}
// Act
await writer.FlushAsync();
// Assert
Assert.Equal<char>(Content, inner.ToString().ToCharArray());
}
private class TestArrayPool : ArrayPool<char>
{
public IList<char[]> Returned { get; } = new List<char[]>();
@ -279,5 +339,20 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
Returned.Add(buffer);
}
}
private class RentMoreArrayPool : ArrayPool<char>
{
public IList<char[]> Returned { get; } = new List<char[]>();
public override char[] Rent(int minimumLength)
{
return new char[2 * minimumLength];
}
public override void Return(char[] buffer, bool clearArray = false)
{
Returned.Add(buffer);
}
}
}
}
}

253
test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/PagedCharBufferTest.cs
View File

@ -26,7 +26,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
}
[Fact]
public void AppendWithChar_AddsCharacterToTheLastPage()
public void AppendWithChar_AddsCharacterToNewPage()
{
// Arrange
var stringToAppend = new string('a', PagedCharBuffer.PageSize);
@ -62,6 +62,75 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
Assert.Equal(4, buffer.Length);
}
[Fact]
public void AppendWithChar_AppendsLastCharacterToTheCurrentPage()
{
// Arrange
var stringToAppend = new string('a', PagedCharBuffer.PageSize - 1);
var charToAppend = 't';
var buffer = new PagedCharBuffer(new CharArrayBufferSource());
// Act
buffer.Append(stringToAppend);
buffer.Append(charToAppend);
// Assert
Assert.Equal(PagedCharBuffer.PageSize, buffer.Length);
var page = Assert.Single(buffer.Pages);
Assert.Equal(stringToAppend.ToCharArray(), page.Take(PagedCharBuffer.PageSize - 1));
Assert.Equal('t', page[PagedCharBuffer.PageSize - 1]);
}
[Fact]
public void AppendWithChar_AddsCharacterToNewPage_AfterLengthFallback()
{
// Arrange
// Imitate ArrayPool<char>.Shared after it falls back from its default char[1024] Bucket to the next one.
var bufferSource = new Mock<ICharBufferSource>();
bufferSource
.Setup(s => s.Rent(PagedCharBuffer.PageSize))
.Returns(() => new char[2 * PagedCharBuffer.PageSize]);
var buffer = new PagedCharBuffer(bufferSource.Object);
var stringToAppend = new string('a', 2 * PagedCharBuffer.PageSize);
var charToAppend = 't';
// Act
buffer.Append(stringToAppend);
buffer.Append(charToAppend);
// Assert
Assert.Equal(2 * PagedCharBuffer.PageSize + 1, buffer.Length);
Assert.Collection(buffer.Pages,
page => Assert.Equal(stringToAppend.ToCharArray(), page),
page => Assert.Equal(charToAppend, page[0]));
}
[Fact]
public void AppendWithChar_AppendsLastCharacterToTheCurrentPage_AfterLengthFallback()
{
// Arrange
// Imitate ArrayPool<char>.Shared after it falls back from its default char[1024] Bucket to the next one.
var bufferSource = new Mock<ICharBufferSource>();
bufferSource
.Setup(s => s.Rent(PagedCharBuffer.PageSize))
.Returns(new char[2 * PagedCharBuffer.PageSize]);
var buffer = new PagedCharBuffer(bufferSource.Object);
var stringToAppend = new string('a', 2 * PagedCharBuffer.PageSize - 1);
var charToAppend = 't';
// Act
buffer.Append(stringToAppend);
buffer.Append(charToAppend);
// Assert
Assert.Equal(2 * PagedCharBuffer.PageSize, buffer.Length);
var page = Assert.Single(buffer.Pages);
Assert.Equal(stringToAppend.ToCharArray(), page.Take(2 * PagedCharBuffer.PageSize - 1));
Assert.Equal('t', page[2 * PagedCharBuffer.PageSize - 1]);
}
[Fact]
public void AppendWithString_AppendsToPage()
{
@ -79,7 +148,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
}
[Fact]
public void AppendWithString_AcrossMultiplePages()
public void AppendWithString_AppendsToMultiplePages()
{
// Arrange
var length = 2 * PagedCharBuffer.PageSize + 1;
@ -99,6 +168,96 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
page => Assert.Equal('d', page[0]));
}
[Fact]
public void AppendWithString_AppendsToMultiplePages_AsLengthFallsBack()
{
// Arrange
// Imitate ArrayPool<char>.Shared as it falls back from its default char[1024] Bucket to the next one.
var bufferSource = new Mock<ICharBufferSource>();
bufferSource
.SetupSequence(s => s.Rent(PagedCharBuffer.PageSize))
.Returns(new char[PagedCharBuffer.PageSize])
.Returns(new char[2 * PagedCharBuffer.PageSize]);
var buffer = new PagedCharBuffer(bufferSource.Object);
// Request enough space that transition should occur.
var length = 2 * PagedCharBuffer.PageSize + 1;
var expected1 = Enumerable.Repeat('d', PagedCharBuffer.PageSize);
var expected2 = Enumerable.Repeat('d', PagedCharBuffer.PageSize + 1);
var laterString = new string('d', PagedCharBuffer.PageSize);
// Act (loop within first Append(string) call).
buffer.Append('d');
buffer.Append(laterString);
buffer.Append(laterString);
// Assert
Assert.Equal(length, buffer.Length);
Assert.Collection(
buffer.Pages,
page => Assert.Equal(expected1, page),
page => Assert.Equal(expected2, page.Take(PagedCharBuffer.PageSize + 1)));
}
[Fact]
public void AppendWithString_AppendsToMultiplePages_AsLengthReturnsToNormal()
{
// Arrange
// Imitate ArrayPool<char>.Shared just after an entry in its default char[1024] Bucket is returned.
var bufferSource = new Mock<ICharBufferSource>();
bufferSource
.SetupSequence(s => s.Rent(PagedCharBuffer.PageSize))
.Returns(new char[2 * PagedCharBuffer.PageSize])
.Returns(new char[PagedCharBuffer.PageSize]);
var buffer = new PagedCharBuffer(bufferSource.Object);
// Request enough space that transition should occur.
var length = 2 * PagedCharBuffer.PageSize + 1;
var expected = Enumerable.Repeat('d', 2 * PagedCharBuffer.PageSize);
var laterString = new string('d', PagedCharBuffer.PageSize);
// Act (loop within second Append(string) call).
buffer.Append('d');
buffer.Append(laterString);
buffer.Append(laterString);
// Assert
Assert.Equal(length, buffer.Length);
Assert.Collection(
buffer.Pages,
page => Assert.Equal(expected, page),
page => Assert.Equal('d', page[0]));
}
[Fact]
public void AppendWithString_AppendsToMultiplePages_AfterLengthFallback()
{
// Arrange
// Imitate ArrayPool<char>.Shared after it falls back from its default char[1024] Bucket to the next one.
var bufferSource = new Mock<ICharBufferSource>();
bufferSource
.Setup(s => s.Rent(PagedCharBuffer.PageSize))
.Returns(() => new char[2 * PagedCharBuffer.PageSize]);
var buffer = new PagedCharBuffer(bufferSource.Object);
// Request enough space that transition should occur.
var length = 2 * PagedCharBuffer.PageSize + 1;
var expected = Enumerable.Repeat('d', 2 * PagedCharBuffer.PageSize);
var laterString = new string('d', PagedCharBuffer.PageSize);
// Act (loop within second Append(string) call).
buffer.Append('d');
buffer.Append(laterString);
buffer.Append(laterString);
// Assert
Assert.Equal(length, buffer.Length);
Assert.Collection(
buffer.Pages,
page => Assert.Equal(expected, page),
page => Assert.Equal('d', page[0]));
}
[Fact]
public void AppendWithString_AppendsToTheCurrentPageIfItIsNotEmpty()
{
@ -159,6 +318,96 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
});
}
[Fact]
public void AppendWithCharArray_AppendsToMultiplePages_AsLengthFallsBack()
{
// Arrange
// Imitate ArrayPool<char>.Shared as it falls back from its default char[1024] Bucket to the next one.
var bufferSource = new Mock<ICharBufferSource>();
bufferSource
.SetupSequence(s => s.Rent(PagedCharBuffer.PageSize))
.Returns(new char[PagedCharBuffer.PageSize])
.Returns(new char[2 * PagedCharBuffer.PageSize]);
var buffer = new PagedCharBuffer(bufferSource.Object);
// Request enough space that transition should occur.
var length = 2 * PagedCharBuffer.PageSize + 1;
var expected1 = Enumerable.Repeat('d', PagedCharBuffer.PageSize);
var expected2 = Enumerable.Repeat('d', PagedCharBuffer.PageSize + 1);
var laterChars = new string('d', PagedCharBuffer.PageSize).ToCharArray();
// Act (loop within first Append(char[]) call).
buffer.Append('d');
buffer.Append(laterChars, 0, laterChars.Length);
buffer.Append(laterChars, 0, laterChars.Length);
// Assert
Assert.Equal(length, buffer.Length);
Assert.Collection(
buffer.Pages,
page => Assert.Equal(expected1, page),
page => Assert.Equal(expected2, page.Take(PagedCharBuffer.PageSize + 1)));
}
[Fact]
public void AppendWithCharArray_AppendsToMultiplePages_AsLengthReturnsToNormal()
{
// Arrange
// Imitate ArrayPool<char>.Shared just after an entry in its default char[1024] Bucket is returned.
var bufferSource = new Mock<ICharBufferSource>();
bufferSource
.SetupSequence(s => s.Rent(PagedCharBuffer.PageSize))
.Returns(new char[2 * PagedCharBuffer.PageSize])
.Returns(new char[PagedCharBuffer.PageSize]);
var buffer = new PagedCharBuffer(bufferSource.Object);
// Request enough space that transition should occur.
var length = 2 * PagedCharBuffer.PageSize + 1;
var expected = Enumerable.Repeat('d', 2 * PagedCharBuffer.PageSize);
var laterChars = new string('d', PagedCharBuffer.PageSize).ToCharArray();
// Act (loop within second Append(char[]) call).
buffer.Append('d');
buffer.Append(laterChars, 0, laterChars.Length);
buffer.Append(laterChars, 0, laterChars.Length);
// Assert
Assert.Equal(length, buffer.Length);
Assert.Collection(
buffer.Pages,
page => Assert.Equal(expected, page),
page => Assert.Equal('d', page[0]));
}
[Fact]
public void AppendWithCharArray_AppendsToMultiplePages_AfterLengthFallback()
{
// Arrange
// Imitate ArrayPool<char>.Shared after it falls back from its default char[1024] Bucket to the next one.
var bufferSource = new Mock<ICharBufferSource>();
bufferSource
.Setup(s => s.Rent(PagedCharBuffer.PageSize))
.Returns(() => new char[2 * PagedCharBuffer.PageSize]);
var buffer = new PagedCharBuffer(bufferSource.Object);
// Request enough space that transition should occur.
var length = 2 * PagedCharBuffer.PageSize + 1;
var expected = Enumerable.Repeat('d', 2 * PagedCharBuffer.PageSize);
var laterChars = new string('d', PagedCharBuffer.PageSize).ToCharArray();
// Act (loop within second Append(char[]) call).
buffer.Append('d');
buffer.Append(laterChars, 0, laterChars.Length);
buffer.Append(laterChars, 0, laterChars.Length);
// Assert
Assert.Equal(length, buffer.Length);
Assert.Collection(
buffer.Pages,
page => Assert.Equal(expected, page),
page => Assert.Equal('d', page[0]));
}
[Fact]
public void AppendWithCharArray_AppendsToCurrentPage()
{