Skip to content

Commit e9b980c

Browse files
[release/7.0.2xx] Revert hot reload script injection response buffering (#29152)
## Summary Fixes a regression in memory consumption caused by large responses when debugging with hot reload enabled. ## Description As one of the changes introduced in .NET 7 by #27537, the hot reload script injection middleware now buffers the entire response before performing script injection. This was done to improve script injection reliability, but we recently began receiving reports from customers that were confused by the increase in server memory usage for large responses when debugging with hot reload is enabled. This PR reverts the buffering behavior but keeps the logging improvements that help customers identify the cause of script injection failures and take steps to fix them. For .NET 8, we will assess an alternative approach to improve script injection reliability that doesn't involve buffering each response. ## Validation Manual validation + existing automated tests for this scenario. ## Risk Low. This PR simply reverts the offending commit and adds back some of the improved logging behavior with no other changes to functionality. Fixes dotnet/aspnetcore#45037
1 parent ee5e311 commit e9b980c

File tree

6 files changed

+269
-176
lines changed

6 files changed

+269
-176
lines changed

src/BuiltInTools/BrowserRefresh/BrowserRefreshMiddleware.cs

Lines changed: 11 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33

44
using System;
55
using System.Collections.Generic;
6-
using System.Diagnostics;
7-
using System.IO;
86
using System.Threading.Tasks;
97
using Microsoft.AspNetCore.Http;
108
using Microsoft.AspNetCore.Http.Features;
@@ -28,10 +26,10 @@ public async Task InvokeAsync(HttpContext context)
2826
// We only need to support this for requests that could be initiated by a browser.
2927
if (IsBrowserDocumentRequest(context))
3028
{
31-
// Use a custom stream to buffer the response body for rewriting.
32-
using var memoryStream = new MemoryStream();
29+
// Use a custom StreamWrapper to rewrite output on Write/WriteAsync
30+
using var responseStreamWrapper = new ResponseStreamWrapper(context, _logger);
3331
var originalBodyFeature = context.Features.Get<IHttpResponseBodyFeature>();
34-
context.Features.Set<IHttpResponseBodyFeature>(new StreamResponseBodyFeature(memoryStream));
32+
context.Features.Set<IHttpResponseBodyFeature>(new StreamResponseBodyFeature(responseStreamWrapper));
3533

3634
try
3735
{
@@ -42,35 +40,19 @@ public async Task InvokeAsync(HttpContext context)
4240
context.Features.Set(originalBodyFeature);
4341
}
4442

45-
if (memoryStream.TryGetBuffer(out var buffer) && buffer.Count > 0)
43+
if (responseStreamWrapper.IsHtmlResponse)
4644
{
47-
var response = context.Response;
48-
var baseStream = response.Body;
49-
50-
if (IsHtmlResponse(response))
45+
if (responseStreamWrapper.ScriptInjectionPerformed)
46+
{
47+
Log.BrowserConfiguredForRefreshes(_logger);
48+
}
49+
else if (context.Response.Headers.TryGetValue(HeaderNames.ContentEncoding, out var contentEncodings))
5150
{
52-
Log.SetupResponseForBrowserRefresh(_logger);
53-
54-
// Since we're changing the markup content, reset the content-length
55-
response.Headers.ContentLength = null;
56-
57-
var scriptInjectionPerformed = await WebSocketScriptInjection.TryInjectLiveReloadScriptAsync(baseStream, buffer);
58-
if (scriptInjectionPerformed)
59-
{
60-
Log.BrowserConfiguredForRefreshes(_logger);
61-
}
62-
else if (response.Headers.TryGetValue(HeaderNames.ContentEncoding, out var contentEncodings))
63-
{
64-
Log.ResponseCompressionDetected(_logger, contentEncodings);
65-
}
66-
else
67-
{
68-
Log.FailedToConfiguredForRefreshes(_logger);
69-
}
51+
Log.ResponseCompressionDetected(_logger, contentEncodings);
7052
}
7153
else
7254
{
73-
await baseStream.WriteAsync(buffer);
55+
Log.FailedToConfiguredForRefreshes(_logger);
7456
}
7557
}
7658
}
@@ -114,12 +96,6 @@ internal static bool IsBrowserDocumentRequest(HttpContext context)
11496
return false;
11597
}
11698

117-
private bool IsHtmlResponse(HttpResponse response)
118-
=> (response.StatusCode == StatusCodes.Status200OK || response.StatusCode == StatusCodes.Status500InternalServerError) &&
119-
MediaTypeHeaderValue.TryParse(response.ContentType, out var mediaType) &&
120-
mediaType.IsSubsetOf(_textHtmlMediaType) &&
121-
(!mediaType.Charset.HasValue || mediaType.Charset.Equals("utf-8", StringComparison.OrdinalIgnoreCase));
122-
12399
internal static class Log
124100
{
125101
private static readonly Action<ILogger, Exception?> _setupResponseForBrowserRefresh = LoggerMessage.Define(

src/BuiltInTools/BrowserRefresh/ReadOnlySpanOfByteExtensions.cs

Lines changed: 0 additions & 38 deletions
This file was deleted.
Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
// Based on https://github.com/RickStrahl/Westwind.AspnetCore.LiveReload/blob/128b5f524e86954e997f2c453e7e5c1dcc3db746/Westwind.AspnetCore.LiveReload/ResponseStreamWrapper.cs
5+
6+
using System;
7+
using System.IO;
8+
using System.Threading;
9+
using System.Threading.Tasks;
10+
using Microsoft.AspNetCore.Http;
11+
using Microsoft.Extensions.Logging;
12+
using Microsoft.Net.Http.Headers;
13+
14+
namespace Microsoft.AspNetCore.Watch.BrowserRefresh
15+
{
16+
/// <summary>
17+
/// Wraps the Response Stream to inject the WebSocket HTML into
18+
/// an HTML Page.
19+
/// </summary>
20+
public class ResponseStreamWrapper : Stream
21+
{
22+
private static readonly MediaTypeHeaderValue _textHtmlMediaType = new MediaTypeHeaderValue("text/html");
23+
private readonly Stream _baseStream;
24+
private readonly HttpContext _context;
25+
private readonly ILogger _logger;
26+
private bool? _isHtmlResponse;
27+
28+
public ResponseStreamWrapper(HttpContext context, ILogger logger)
29+
{
30+
_context = context;
31+
_baseStream = context.Response.Body;
32+
_logger = logger;
33+
}
34+
35+
public override bool CanRead => false;
36+
public override bool CanSeek => false;
37+
public override bool CanWrite => true;
38+
public override long Length { get; }
39+
public override long Position { get; set; }
40+
public bool ScriptInjectionPerformed { get; private set; }
41+
42+
public bool IsHtmlResponse => _isHtmlResponse ?? false;
43+
44+
public override void Flush()
45+
{
46+
OnWrite();
47+
_baseStream.Flush();
48+
}
49+
50+
public override Task FlushAsync(CancellationToken cancellationToken)
51+
{
52+
OnWrite();
53+
return _baseStream.FlushAsync(cancellationToken);
54+
}
55+
56+
public override void Write(ReadOnlySpan<byte> buffer)
57+
{
58+
OnWrite();
59+
if (IsHtmlResponse && !ScriptInjectionPerformed)
60+
{
61+
ScriptInjectionPerformed = WebSocketScriptInjection.TryInjectLiveReloadScript(_baseStream, buffer);
62+
}
63+
else
64+
{
65+
_baseStream.Write(buffer);
66+
}
67+
}
68+
69+
public override void WriteByte(byte value)
70+
{
71+
OnWrite();
72+
_baseStream.WriteByte(value);
73+
}
74+
75+
public override void Write(byte[] buffer, int offset, int count)
76+
{
77+
OnWrite();
78+
79+
if (IsHtmlResponse && !ScriptInjectionPerformed)
80+
{
81+
ScriptInjectionPerformed = WebSocketScriptInjection.TryInjectLiveReloadScript(_baseStream, buffer.AsSpan(offset, count));
82+
}
83+
else
84+
{
85+
_baseStream.Write(buffer, offset, count);
86+
}
87+
}
88+
89+
public override async Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
90+
{
91+
OnWrite();
92+
93+
if (IsHtmlResponse && !ScriptInjectionPerformed)
94+
{
95+
ScriptInjectionPerformed = await WebSocketScriptInjection.TryInjectLiveReloadScriptAsync(_baseStream, buffer.AsMemory(offset, count), cancellationToken);
96+
}
97+
else
98+
{
99+
await _baseStream.WriteAsync(buffer, offset, count, cancellationToken);
100+
}
101+
}
102+
103+
public override async ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken = default)
104+
{
105+
OnWrite();
106+
107+
if (IsHtmlResponse && !ScriptInjectionPerformed)
108+
{
109+
ScriptInjectionPerformed = await WebSocketScriptInjection.TryInjectLiveReloadScriptAsync(_baseStream, buffer, cancellationToken);
110+
}
111+
else
112+
{
113+
await _baseStream.WriteAsync(buffer, cancellationToken);
114+
}
115+
}
116+
117+
private void OnWrite()
118+
{
119+
if (_isHtmlResponse.HasValue)
120+
{
121+
return;
122+
}
123+
124+
var response = _context.Response;
125+
126+
_isHtmlResponse =
127+
(response.StatusCode == StatusCodes.Status200OK || response.StatusCode == StatusCodes.Status500InternalServerError) &&
128+
MediaTypeHeaderValue.TryParse(response.ContentType, out var mediaType) &&
129+
mediaType.IsSubsetOf(_textHtmlMediaType) &&
130+
(!mediaType.Charset.HasValue || mediaType.Charset.Equals("utf-8", StringComparison.OrdinalIgnoreCase));
131+
132+
if (_isHtmlResponse.Value)
133+
{
134+
BrowserRefreshMiddleware.Log.SetupResponseForBrowserRefresh(_logger);
135+
136+
// Since we're changing the markup content, reset the content-length
137+
response.Headers.ContentLength = null;
138+
}
139+
}
140+
141+
public override int Read(byte[] buffer, int offset, int count) => throw new NotSupportedException();
142+
143+
public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException();
144+
145+
public override Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
146+
=> throw new NotSupportedException();
147+
148+
public override ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken = default)
149+
=> throw new NotSupportedException();
150+
151+
public override void SetLength(long value) => throw new NotSupportedException();
152+
}
153+
}

src/BuiltInTools/BrowserRefresh/WebSocketScriptInjection.cs

Lines changed: 27 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -15,81 +15,59 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh
1515
/// </summary>
1616
public static class WebSocketScriptInjection
1717
{
18-
private static readonly byte[] s_closingTagStartIndicator = "</"u8.ToArray();
19-
private static readonly byte[] s_bodyElementName = "body"u8.ToArray();
20-
private static readonly byte[] s_closingTagEndIndicator = ">"u8.ToArray();
18+
private const string BodyMarker = "</body>";
19+
20+
private static readonly byte[] _bodyBytes = Encoding.UTF8.GetBytes(BodyMarker);
2121

2222
internal static string InjectedScript { get; } = $"<script src=\"{ApplicationPaths.BrowserRefreshJS}\"></script>";
2323

24-
private static readonly byte[] s_injectedScriptBytes = Encoding.UTF8.GetBytes(InjectedScript);
24+
private static readonly byte[] _injectedScriptBytes = Encoding.UTF8.GetBytes(InjectedScript);
2525

26-
public static async ValueTask<bool> TryInjectLiveReloadScriptAsync(Stream baseStream, ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken = default)
26+
public static bool TryInjectLiveReloadScript(Stream baseStream, ReadOnlySpan<byte> buffer)
2727
{
28-
var index = LastIndexOfClosingBodyTag(buffer.Span);
28+
var index = buffer.LastIndexOf(_bodyBytes);
2929
if (index == -1)
3030
{
31-
await baseStream.WriteAsync(buffer, cancellationToken);
31+
baseStream.Write(buffer);
3232
return false;
3333
}
3434

3535
if (index > 0)
3636
{
37-
await baseStream.WriteAsync(buffer.Slice(0, index), cancellationToken);
37+
baseStream.Write(buffer.Slice(0, index));
3838
buffer = buffer[index..];
3939
}
4040

4141
// Write the injected script
42-
await baseStream.WriteAsync(s_injectedScriptBytes, cancellationToken);
42+
baseStream.Write(_injectedScriptBytes);
4343

4444
// Write the rest of the buffer/HTML doc
45-
await baseStream.WriteAsync(buffer, cancellationToken);
45+
baseStream.Write(buffer);
4646
return true;
4747
}
4848

49-
private static int LastIndexOfClosingBodyTag(ReadOnlySpan<byte> buffer)
49+
public static async ValueTask<bool> TryInjectLiveReloadScriptAsync(Stream baseStream, ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken = default)
5050
{
51-
while (true)
51+
var index = buffer.Span.LastIndexOf(_bodyBytes);
52+
if (index == -1)
5253
{
53-
// Find the character sequence for the end of the closing tag.
54-
var index = buffer.LastIndexOf(s_closingTagEndIndicator);
55-
if (index == -1)
56-
{
57-
return -1;
58-
}
59-
buffer = buffer[..index];
60-
61-
// Find the first non-whitespace character inside the tag.
62-
index = buffer.LastIndexOfNonWhiteSpace();
63-
if (index == -1)
64-
{
65-
return -1;
66-
}
67-
buffer = buffer[..(index + 1)];
68-
69-
// Determine if the characters inside the tag match "body".
70-
if (!buffer.EndsWithIgnoreCase(s_bodyElementName))
71-
{
72-
continue;
73-
}
74-
buffer = buffer[..^s_bodyElementName.Length];
54+
await baseStream.WriteAsync(buffer, cancellationToken);
55+
return false;
56+
}
7557

76-
// Find the first non-whitespace character before the tag name.
77-
index = buffer.LastIndexOfNonWhiteSpace();
78-
if (index == -1)
79-
{
80-
return -1;
81-
}
82-
buffer = buffer[..(index + 1)];
58+
if (index > 0)
59+
{
60+
await baseStream.WriteAsync(buffer.Slice(0, index), cancellationToken);
61+
buffer = buffer[index..];
62+
}
8363

84-
// Determine if the characters preceding tag name match the closing tag start indicator.
85-
if (!buffer.EndsWith(s_closingTagStartIndicator))
86-
{
87-
continue;
88-
}
89-
buffer = buffer[..^s_closingTagStartIndicator.Length];
64+
// Write the injected script
65+
await baseStream.WriteAsync(_injectedScriptBytes, cancellationToken);
9066

91-
return buffer.Length;
92-
}
67+
// Write the rest of the buffer/HTML doc
68+
await baseStream.WriteAsync(buffer, cancellationToken);
69+
return true;
9370
}
71+
9472
}
9573
}

0 commit comments

Comments
 (0)