Skip to content

Commit

Permalink
Prevent synchronous writes when using Razor (#9395)
Browse files Browse the repository at this point in the history 
* Do not perform synchronous writes to the Response TextWriter after a Razor FlushAsync
* Use ViewBuffer to perform async writes to the response when using ViewComponentResult

Related to #6397 

Fixes #4885
  • Loading branch information
@pranavkm
pranavkm authored Apr 16, 2019
1 parent fb7e8a6 commit 2be8052
Show file tree
Hide file tree
Showing 13 changed files with 223 additions and 267 deletions.
11 changes: 5 additions & 6 deletions src/Mvc/Mvc.Razor/src/RazorPageBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,12 +370,7 @@ public virtual void Write(object value)
var encoder = HtmlEncoder;
if (value is IHtmlContent htmlContent)
{
var bufferedWriter = writer as ViewBufferTextWriter;
if (bufferedWriter == null || !bufferedWriter.IsBuffering)
{
htmlContent.WriteTo(writer, encoder);
}
else
if (writer is ViewBufferTextWriter bufferedWriter)
{
if (value is IHtmlContentContainer htmlContentContainer)
{
Expand All @@ -389,6 +384,10 @@ public virtual void Write(object value)
bufferedWriter.Buffer.AppendHtml(htmlContent);
}
}
else
{
htmlContent.WriteTo(writer, encoder);
}

return;
}
Expand Down
33 changes: 15 additions & 18 deletions src/Mvc/Mvc.Razor/src/RazorView.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ private async Task RenderLayoutAsync(
// (including the layout page we just rendered).
while (!string.IsNullOrEmpty(previousPage.Layout))
{
if (!bodyWriter.IsBuffering)
if (bodyWriter.Flushed)
{
// Once a call to RazorPage.FlushAsync is made, we can no longer render Layout pages - content has
// already been written to the client and the layout content would be appended rather than surround
Expand Down Expand Up @@ -274,25 +274,22 @@ private async Task RenderLayoutAsync(
layoutPage.EnsureRenderedBodyOrSections();
}

if (bodyWriter.IsBuffering)
// We've got a bunch of content in the view buffer. How to best deal with it
// really depends on whether or not we're writing directly to the output or if we're writing to
// another buffer.
if (context.Writer is ViewBufferTextWriter viewBufferTextWriter)
{
// If IsBuffering - then we've got a bunch of content in the view buffer. How to best deal with it
// really depends on whether or not we're writing directly to the output or if we're writing to
// another buffer.
var viewBufferTextWriter = context.Writer as ViewBufferTextWriter;
if (viewBufferTextWriter == null || !viewBufferTextWriter.IsBuffering)
{
// This means we're writing to a 'real' writer, probably to the actual output stream.
// We're using PagedBufferedTextWriter here to 'smooth' synchronous writes of IHtmlContent values.
using (var writer = _bufferScope.CreateWriter(context.Writer))
{
await bodyWriter.Buffer.WriteToAsync(writer, _htmlEncoder);
}
}
else
// This means we're writing to another buffer. Use MoveTo to combine them.
bodyWriter.Buffer.MoveTo(viewBufferTextWriter.Buffer);
}
else
{
// This means we're writing to a 'real' writer, probably to the actual output stream.
// We're using PagedBufferedTextWriter here to 'smooth' synchronous writes of IHtmlContent values.
using (var writer = _bufferScope.CreateWriter(context.Writer))
{
// This means we're writing to another buffer. Use MoveTo to combine them.
bodyWriter.Buffer.MoveTo(viewBufferTextWriter.Buffer);
await bodyWriter.Buffer.WriteToAsync(writer, _htmlEncoder);
await writer.FlushAsync();
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/Mvc/Mvc.ViewFeatures/ref/Microsoft.AspNetCore.Mvc.ViewFeatures.netcoreapp3.0.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1204,7 +1204,9 @@ public virtual void AddAndTrackValidationAttributes(Microsoft.AspNetCore.Mvc.Ren
}
public partial class ViewComponentResultExecutor : Microsoft.AspNetCore.Mvc.Infrastructure.IActionResultExecutor<Microsoft.AspNetCore.Mvc.ViewComponentResult>
{
[System.ObsoleteAttribute("This constructor is obsolete and will be removed in a future version.")]
public ViewComponentResultExecutor(Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Mvc.MvcViewOptions> mvcHelperOptions, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory, System.Text.Encodings.Web.HtmlEncoder htmlEncoder, Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider modelMetadataProvider, Microsoft.AspNetCore.Mvc.ViewFeatures.ITempDataDictionaryFactory tempDataDictionaryFactory) { }
public ViewComponentResultExecutor(Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Mvc.MvcViewOptions> mvcHelperOptions, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory, System.Text.Encodings.Web.HtmlEncoder htmlEncoder, Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider modelMetadataProvider, Microsoft.AspNetCore.Mvc.ViewFeatures.ITempDataDictionaryFactory tempDataDictionaryFactory, Microsoft.AspNetCore.Mvc.Infrastructure.IHttpResponseStreamWriterFactory writerFactory) { }
[System.Diagnostics.DebuggerStepThroughAttribute]
public virtual System.Threading.Tasks.Task ExecuteAsync(Microsoft.AspNetCore.Mvc.ActionContext context, Microsoft.AspNetCore.Mvc.ViewComponentResult result) { throw null; }
}
Expand Down
181 changes: 39 additions & 142 deletions src/Mvc/Mvc.ViewFeatures/src/Buffers/ViewBufferTextWriter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,25 +86,20 @@ public ViewBufferTextWriter(ViewBuffer buffer, Encoding encoding, HtmlEncoder ht
/// <inheritdoc />
public override Encoding Encoding { get; }

/// <inheritdoc />
public bool IsBuffering { get; private set; } = true;

/// <summary>
/// Gets the <see cref="ViewBuffer"/>.
/// </summary>
public ViewBuffer Buffer { get; }

/// <summary>
/// Gets a value that indiciates if <see cref="Flush"/> or <see cref="FlushAsync" /> was invoked.
/// </summary>
public bool Flushed { get; private set; }

/// <inheritdoc />
public override void Write(char value)
{
if (IsBuffering)
{
Buffer.AppendHtml(value.ToString());
}
else
{
_inner.Write(value);
}
Buffer.AppendHtml(value.ToString());
}

/// <inheritdoc />
Expand All @@ -125,14 +120,7 @@ public override void Write(char[] buffer, int index, int count)
throw new ArgumentOutOfRangeException(nameof(count));
}

if (IsBuffering)
{
Buffer.AppendHtml(new string(buffer, index, count));
}
else
{
_inner.Write(buffer, index, count);
}
Buffer.AppendHtml(new string(buffer, index, count));
}

/// <inheritdoc />
Expand All @@ -143,14 +131,7 @@ public override void Write(string value)
return;
}

if (IsBuffering)
{
Buffer.AppendHtml(value);
}
else
{
_inner.Write(value);
}
Buffer.AppendHtml(value);
}

/// <inheritdoc />
Expand Down Expand Up @@ -186,14 +167,7 @@ public void Write(IHtmlContent value)
return;
}

if (IsBuffering)
{
Buffer.AppendHtml(value);
}
else
{
value.WriteTo(_inner, _htmlEncoder);
}
Buffer.AppendHtml(value);
}

/// <summary>
Expand All @@ -207,14 +181,7 @@ public void Write(IHtmlContentContainer value)
return;
}

if (IsBuffering)
{
value.MoveTo(Buffer);
}
else
{
value.WriteTo(_inner, _htmlEncoder);
}
value.MoveTo(Buffer);
}

/// <inheritdoc />
Expand Down Expand Up @@ -245,15 +212,8 @@ public override void WriteLine(object value)
/// <inheritdoc />
public override Task WriteAsync(char value)
{
if (IsBuffering)
{
Buffer.AppendHtml(value.ToString());
return Task.CompletedTask;
}
else
{
return _inner.WriteAsync(value);
}
Buffer.AppendHtml(value.ToString());
return Task.CompletedTask;
}

/// <inheritdoc />
Expand All @@ -273,121 +233,64 @@ public override Task WriteAsync(char[] buffer, int index, int count)
throw new ArgumentOutOfRangeException(nameof(count));
}

if (IsBuffering)
{
Buffer.AppendHtml(new string(buffer, index, count));
return Task.CompletedTask;
}
else
{
return _inner.WriteAsync(buffer, index, count);
}
Buffer.AppendHtml(new string(buffer, index, count));
return Task.CompletedTask;
}

/// <inheritdoc />
public override Task WriteAsync(string value)
{
if (IsBuffering)
{
Buffer.AppendHtml(value);
return Task.CompletedTask;
}
else
{
return _inner.WriteAsync(value);
}
Buffer.AppendHtml(value);
return Task.CompletedTask;
}

/// <inheritdoc />
public override void WriteLine()
{
if (IsBuffering)
{
Buffer.AppendHtml(NewLine);
}
else
{
_inner.WriteLine();
}
Buffer.AppendHtml(NewLine);
}

/// <inheritdoc />
public override void WriteLine(string value)
{
if (IsBuffering)
{
Buffer.AppendHtml(value);
Buffer.AppendHtml(NewLine);
}
else
{
_inner.WriteLine(value);
}
Buffer.AppendHtml(value);
Buffer.AppendHtml(NewLine);
}

/// <inheritdoc />
public override Task WriteLineAsync(char value)
{
if (IsBuffering)
{
Buffer.AppendHtml(value.ToString());
Buffer.AppendHtml(NewLine);
return Task.CompletedTask;
}
else
{
return _inner.WriteLineAsync(value);
}
Buffer.AppendHtml(value.ToString());
Buffer.AppendHtml(NewLine);
return Task.CompletedTask;
}

/// <inheritdoc />
public override Task WriteLineAsync(char[] value, int start, int offset)
{
if (IsBuffering)
{
Buffer.AppendHtml(new string(value, start, offset));
Buffer.AppendHtml(NewLine);
return Task.CompletedTask;
}
else
{
return _inner.WriteLineAsync(value, start, offset);
}
Buffer.AppendHtml(new string(value, start, offset));
Buffer.AppendHtml(NewLine);
return Task.CompletedTask;

}

/// <inheritdoc />
public override Task WriteLineAsync(string value)
{
if (IsBuffering)
{
Buffer.AppendHtml(value);
Buffer.AppendHtml(NewLine);
return Task.CompletedTask;
}
else
{
return _inner.WriteLineAsync(value);
}
Buffer.AppendHtml(value);
Buffer.AppendHtml(NewLine);
return Task.CompletedTask;
}

/// <inheritdoc />
public override Task WriteLineAsync()
{
if (IsBuffering)
{
Buffer.AppendHtml(NewLine);
return Task.CompletedTask;
}
else
{
return _inner.WriteLineAsync();
}
Buffer.AppendHtml(NewLine);
return Task.CompletedTask;
}

/// <summary>
/// Copies the buffered content to the unbuffered writer and invokes flush on it.
/// Additionally causes this instance to no longer buffer and direct all write operations
/// to the unbuffered writer.
/// </summary>
public override void Flush()
{
Expand All @@ -396,20 +299,16 @@ public override void Flush()
return;
}

if (IsBuffering)
{
IsBuffering = false;
Buffer.WriteTo(_inner, _htmlEncoder);
Buffer.Clear();
}
Flushed = true;

Buffer.WriteTo(_inner, _htmlEncoder);
Buffer.Clear();

_inner.Flush();
}

/// <summary>
/// Copies the buffered content to the unbuffered writer and invokes flush on it.
/// Additionally causes this instance to no longer buffer and direct all write operations
/// to the unbuffered writer.
/// </summary>
/// <returns>A <see cref="Task"/> that represents the asynchronous copy and flush operations.</returns>
public override async Task FlushAsync()
Expand All @@ -419,12 +318,10 @@ public override async Task FlushAsync()
return;
}

if (IsBuffering)
{
IsBuffering = false;
await Buffer.WriteToAsync(_inner, _htmlEncoder);
Buffer.Clear();
}
Flushed = true;

await Buffer.WriteToAsync(_inner, _htmlEncoder);
Buffer.Clear();

await _inner.FlushAsync();
}
Expand Down
Loading

0 comments on commit 2be8052

Please sign in to comment.