From b34c5ddd5f0f548610302a31c2247889ab837473 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Fri, 13 Nov 2020 14:25:47 -0800 Subject: [PATCH] Prevent dotnet-watch script injection staleness (#27778) The browser refresh mechansim used by dotnet-watch and VS modifies HTML content. The modified content includes a script block that has a WebSocket url that changes for each new execution of dotnet watch run (not rebuilds, but watch itself). HTML content can come from views or static html files on disk. For the latter, ASP.NET Core participates in browser caching by sending (and invalidating) etag headers. One way to fix this problem is remove or modify the etag headers. The risk here is that might cause differences in behavior in development users may come to rely on that are unavailable in production. This change instead modifies the HTML content so the output is always consistent and consequently safe to cache. The dynamic content is served separately by the injected middleware. This change fixes the issue of multiple instances of dotnet-watch. While this issue may crop up if you alternate between dotnet run and dotnet watch run but we haven't seen this being an issue as yet. Fixes #27548 Summary Running dotnet watch run multiple times in Blazor WASM apps (or any app that serves static html files) can produce console errors and prevent the browser refresh feature from working. Given that we've been telling our users to use dotnet watch run as their primary way to work outside of VS, it's likely more users would run in this. Customer impact A hard browser refresh (Ctrl + R) is needed to get the refresh behavior to work. Regression No. This has existed since the feature was introduced we did not get reports of it Risk Low. The fix is isolated to dotnet-watch and VS's browser refresh mechanism which is in preview. The change was tested locally, but if there's a regression or if the change interferes with user's workflow, users have the ability to disable this feature. --- .../src/BrowserScriptMiddleware.cs | 51 ++++++++++++++ .../BrowserRefresh/src/HostingFilter.cs | 2 + .../src/ResponseStreamWrapper.cs | 8 +-- .../src/WebSocketScriptInjection.cs | 30 +++------ .../test/BrowserScriptMiddlewareTest.cs | 66 +++++++++++++++++++ ...pNetCore.Watch.BrowserRefresh.Tests.csproj | 1 + .../test/ResponseStreamWrapperTest.cs | 12 ++-- .../test/WebSockerScriptInjectionTest.cs | 50 +++++--------- 8 files changed, 157 insertions(+), 63 deletions(-) create mode 100644 src/Tools/dotnet-watch/BrowserRefresh/src/BrowserScriptMiddleware.cs create mode 100644 src/Tools/dotnet-watch/BrowserRefresh/test/BrowserScriptMiddlewareTest.cs diff --git a/src/Tools/dotnet-watch/BrowserRefresh/src/BrowserScriptMiddleware.cs b/src/Tools/dotnet-watch/BrowserRefresh/src/BrowserScriptMiddleware.cs new file mode 100644 index 0000000000..b90ae7591d --- /dev/null +++ b/src/Tools/dotnet-watch/BrowserRefresh/src/BrowserScriptMiddleware.cs @@ -0,0 +1,51 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Globalization; +using System.IO; +using System.Text; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Watch.BrowserRefresh +{ + /// + /// Responds with the contennts of WebSocketScriptInjection.js with the stub WebSocket url replaced by the + /// one specified by the launching app. + /// + public sealed class BrowserScriptMiddleware + { + private readonly byte[] _scriptBytes; + private readonly string _contentLength; + + public BrowserScriptMiddleware(RequestDelegate next) + : this(Environment.GetEnvironmentVariable("ASPNETCORE_AUTO_RELOAD_WS_ENDPOINT")!) + { + } + + internal BrowserScriptMiddleware(string webSocketUrl) + { + _scriptBytes = GetWebSocketClientJavaScript(webSocketUrl); + _contentLength = _scriptBytes.Length.ToString(CultureInfo.InvariantCulture); + } + + public async Task InvokeAsync(HttpContext context) + { + context.Response.Headers["Cache-Control"] = "no-store"; + context.Response.Headers["Content-Length"] = _contentLength; + context.Response.Headers["Content-Type"] = "application/javascript; charset=utf-8"; + + await context.Response.Body.WriteAsync(_scriptBytes.AsMemory(), context.RequestAborted); + } + + internal static byte[] GetWebSocketClientJavaScript(string hostString) + { + var jsFileName = "Microsoft.AspNetCore.Watch.BrowserRefresh.WebSocketScriptInjection.js"; + using var reader = new StreamReader(typeof(WebSocketScriptInjection).Assembly.GetManifestResourceStream(jsFileName)!); + var script = reader.ReadToEnd().Replace("{{hostString}}", hostString); + + return Encoding.UTF8.GetBytes(script); + } + } +} diff --git a/src/Tools/dotnet-watch/BrowserRefresh/src/HostingFilter.cs b/src/Tools/dotnet-watch/BrowserRefresh/src/HostingFilter.cs index 02135dca0d..68f32444b7 100644 --- a/src/Tools/dotnet-watch/BrowserRefresh/src/HostingFilter.cs +++ b/src/Tools/dotnet-watch/BrowserRefresh/src/HostingFilter.cs @@ -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.Globalization; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.DependencyInjection; @@ -22,6 +23,7 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh { return app => { + app.Map(WebSocketScriptInjection.WebSocketScriptUrl, app1 => app1.UseMiddleware()); app.UseMiddleware(); next(app); }; diff --git a/src/Tools/dotnet-watch/BrowserRefresh/src/ResponseStreamWrapper.cs b/src/Tools/dotnet-watch/BrowserRefresh/src/ResponseStreamWrapper.cs index dfc5d46503..ae5fcc6f2c 100644 --- a/src/Tools/dotnet-watch/BrowserRefresh/src/ResponseStreamWrapper.cs +++ b/src/Tools/dotnet-watch/BrowserRefresh/src/ResponseStreamWrapper.cs @@ -58,7 +58,7 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh OnWrite(); if (IsHtmlResponse && !ScriptInjectionPerformed) { - ScriptInjectionPerformed = WebSocketScriptInjection.Instance.TryInjectLiveReloadScript(_baseStream, buffer); + ScriptInjectionPerformed = WebSocketScriptInjection.TryInjectLiveReloadScript(_baseStream, buffer); } else { @@ -78,7 +78,7 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh if (IsHtmlResponse && !ScriptInjectionPerformed) { - ScriptInjectionPerformed = WebSocketScriptInjection.Instance.TryInjectLiveReloadScript(_baseStream, buffer.AsSpan(offset, count)); + ScriptInjectionPerformed = WebSocketScriptInjection.TryInjectLiveReloadScript(_baseStream, buffer.AsSpan(offset, count)); } else { @@ -92,7 +92,7 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh if (IsHtmlResponse && !ScriptInjectionPerformed) { - ScriptInjectionPerformed = await WebSocketScriptInjection.Instance.TryInjectLiveReloadScriptAsync(_baseStream, buffer.AsMemory(offset, count), cancellationToken); + ScriptInjectionPerformed = await WebSocketScriptInjection.TryInjectLiveReloadScriptAsync(_baseStream, buffer.AsMemory(offset, count), cancellationToken); } else { @@ -106,7 +106,7 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh if (IsHtmlResponse && !ScriptInjectionPerformed) { - ScriptInjectionPerformed = await WebSocketScriptInjection.Instance.TryInjectLiveReloadScriptAsync(_baseStream, buffer, cancellationToken); + ScriptInjectionPerformed = await WebSocketScriptInjection.TryInjectLiveReloadScriptAsync(_baseStream, buffer, cancellationToken); } else { diff --git a/src/Tools/dotnet-watch/BrowserRefresh/src/WebSocketScriptInjection.cs b/src/Tools/dotnet-watch/BrowserRefresh/src/WebSocketScriptInjection.cs index f0e85e2fa8..43cb43c50e 100644 --- a/src/Tools/dotnet-watch/BrowserRefresh/src/WebSocketScriptInjection.cs +++ b/src/Tools/dotnet-watch/BrowserRefresh/src/WebSocketScriptInjection.cs @@ -13,22 +13,18 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh /// Helper class that handles the HTML injection into /// a string or byte array. /// - public class WebSocketScriptInjection + public static class WebSocketScriptInjection { private const string BodyMarker = ""; + internal const string WebSocketScriptUrl = "/_framework/aspnetcore-browser-refresh.js"; - private readonly byte[] _bodyBytes = Encoding.UTF8.GetBytes(BodyMarker); - private readonly byte[] _scriptInjectionBytes; + private static readonly byte[] _bodyBytes = Encoding.UTF8.GetBytes(BodyMarker); - public static WebSocketScriptInjection Instance { get; } = new WebSocketScriptInjection( - GetWebSocketClientJavaScript(Environment.GetEnvironmentVariable("ASPNETCORE_AUTO_RELOAD_WS_ENDPOINT"))); + internal static string InjectedScript { get; } = $""; - public WebSocketScriptInjection(string clientScript) - { - _scriptInjectionBytes = Encoding.UTF8.GetBytes(clientScript); - } + private static readonly byte[] _injectedScriptBytes = Encoding.UTF8.GetBytes(InjectedScript); - public bool TryInjectLiveReloadScript(Stream baseStream, ReadOnlySpan buffer) + public static bool TryInjectLiveReloadScript(Stream baseStream, ReadOnlySpan buffer) { var index = buffer.LastIndexOf(_bodyBytes); if (index == -1) @@ -44,14 +40,14 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh } // Write the injected script - baseStream.Write(_scriptInjectionBytes); + baseStream.Write(_injectedScriptBytes); // Write the rest of the buffer/HTML doc baseStream.Write(buffer); return true; } - public async ValueTask TryInjectLiveReloadScriptAsync(Stream baseStream, ReadOnlyMemory buffer, CancellationToken cancellationToken = default) + public static async ValueTask TryInjectLiveReloadScriptAsync(Stream baseStream, ReadOnlyMemory buffer, CancellationToken cancellationToken = default) { var index = buffer.Span.LastIndexOf(_bodyBytes); if (index == -1) @@ -67,20 +63,12 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh } // Write the injected script - await baseStream.WriteAsync(_scriptInjectionBytes, cancellationToken); + await baseStream.WriteAsync(_injectedScriptBytes, cancellationToken); // Write the rest of the buffer/HTML doc await baseStream.WriteAsync(buffer, cancellationToken); return true; } - internal static string GetWebSocketClientJavaScript(string? hostString) - { - var jsFileName = "Microsoft.AspNetCore.Watch.BrowserRefresh.WebSocketScriptInjection.js"; - using var reader = new StreamReader(typeof(WebSocketScriptInjection).Assembly.GetManifestResourceStream(jsFileName)!); - var script = reader.ReadToEnd().Replace("{{hostString}}", hostString); - - return $""; - } } } diff --git a/src/Tools/dotnet-watch/BrowserRefresh/test/BrowserScriptMiddlewareTest.cs b/src/Tools/dotnet-watch/BrowserRefresh/test/BrowserScriptMiddlewareTest.cs new file mode 100644 index 0000000000..92931158e0 --- /dev/null +++ b/src/Tools/dotnet-watch/BrowserRefresh/test/BrowserScriptMiddlewareTest.cs @@ -0,0 +1,66 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.IO; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Xunit; + +namespace Microsoft.AspNetCore.Watch.BrowserRefresh +{ + public class BrowserScriptMiddlewareTest + { + [Fact] + public async Task InvokeAsync_ReturnsScript() + { + // Arrange + var context = new DefaultHttpContext(); + var stream = new MemoryStream(); + context.Response.Body = stream; + var middleware = new BrowserScriptMiddleware("some-host"); + + // Act + await middleware.InvokeAsync(context); + + // Assert + stream.Position = 0; + var script = Encoding.UTF8.GetString(stream.ToArray()); + Assert.Contains("// dotnet-watch browser reload script", script); + Assert.Contains("'some-host'", script); + } + + [Fact] + public async Task InvokeAsync_ConfiguresHeaders() + { + // Arrange + var context = new DefaultHttpContext(); + context.Response.Body = new MemoryStream(); + var middleware = new BrowserScriptMiddleware("some-host"); + + // Act + await middleware.InvokeAsync(context); + + // Assert + var response = context.Response; + Assert.Collection( + response.Headers.OrderBy(h => h.Key), + kvp => + { + Assert.Equal("Cache-Control", kvp.Key); + Assert.Equal("no-store", kvp.Value); + }, + kvp => + { + Assert.Equal("Content-Length", kvp.Key); + Assert.NotEmpty(kvp.Value); + }, + kvp => + { + Assert.Equal("Content-Type", kvp.Key); + Assert.Equal("application/javascript; charset=utf-8", kvp.Value); + }); + } + } +} diff --git a/src/Tools/dotnet-watch/BrowserRefresh/test/Microsoft.AspNetCore.Watch.BrowserRefresh.Tests.csproj b/src/Tools/dotnet-watch/BrowserRefresh/test/Microsoft.AspNetCore.Watch.BrowserRefresh.Tests.csproj index 1769c1cf2c..5b3938e426 100644 --- a/src/Tools/dotnet-watch/BrowserRefresh/test/Microsoft.AspNetCore.Watch.BrowserRefresh.Tests.csproj +++ b/src/Tools/dotnet-watch/BrowserRefresh/test/Microsoft.AspNetCore.Watch.BrowserRefresh.Tests.csproj @@ -18,6 +18,7 @@ We'll simply test against it as source. --> + diff --git a/src/Tools/dotnet-watch/BrowserRefresh/test/ResponseStreamWrapperTest.cs b/src/Tools/dotnet-watch/BrowserRefresh/test/ResponseStreamWrapperTest.cs index a62d00ab6a..f8c48ba879 100644 --- a/src/Tools/dotnet-watch/BrowserRefresh/test/ResponseStreamWrapperTest.cs +++ b/src/Tools/dotnet-watch/BrowserRefresh/test/ResponseStreamWrapperTest.cs @@ -34,7 +34,7 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh.Tests response.EnsureSuccessStatusCode(); var content = await response.Content.ReadAsStringAsync(); - Assert.Contains("dotnet-watch browser reload script", content); + Assert.Contains(WebSocketScriptInjection.InjectedScript, content); } [Fact] @@ -50,7 +50,7 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh.Tests Assert.False(response.Content.Headers.TryGetValues("Content-Length", out _), "We shouldn't send a Content-Length header."); var content = await response.Content.ReadAsStringAsync(); - Assert.Contains("dotnet-watch browser reload script", content); + Assert.Contains(WebSocketScriptInjection.InjectedScript, content); } [Fact] @@ -73,7 +73,7 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh.Tests Assert.False(response.Content.Headers.TryGetValues("Content-Length", out _), "We shouldn't send a Content-Length header."); var content = await response.Content.ReadAsStringAsync(); - Assert.Contains("dotnet-watch browser reload script", content); + Assert.Contains(WebSocketScriptInjection.InjectedScript, content); } [Fact] @@ -98,7 +98,7 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh.Tests Assert.False(response.Content.Headers.TryGetValues("Content-Length", out _), "We shouldn't send a Content-Length header."); var content = await response.Content.ReadAsStringAsync(); - Assert.Contains("dotnet-watch browser reload script", content); + Assert.Contains(WebSocketScriptInjection.InjectedScript, content); } [Fact] @@ -131,7 +131,7 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh.Tests Assert.False(response.Content.Headers.TryGetValues("Content-Length", out _), "We shouldn't send a Content-Length header."); var content = await response.Content.ReadAsStringAsync(); - Assert.Contains("dotnet-watch browser reload script", content); + Assert.Contains(WebSocketScriptInjection.InjectedScript, content); } [Fact] @@ -164,7 +164,7 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh.Tests Assert.False(response.Content.Headers.TryGetValues("Content-Length", out _), "We shouldn't send a Content-Length header."); var content = await response.Content.ReadAsStringAsync(); - Assert.Contains("dotnet-watch browser reload script", content); + Assert.Contains(WebSocketScriptInjection.InjectedScript, content); } [Fact] diff --git a/src/Tools/dotnet-watch/BrowserRefresh/test/WebSockerScriptInjectionTest.cs b/src/Tools/dotnet-watch/BrowserRefresh/test/WebSockerScriptInjectionTest.cs index ceb53404d1..6ad204ae4a 100644 --- a/src/Tools/dotnet-watch/BrowserRefresh/test/WebSockerScriptInjectionTest.cs +++ b/src/Tools/dotnet-watch/BrowserRefresh/test/WebSockerScriptInjectionTest.cs @@ -11,9 +11,6 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh { public class WebSockerScriptInjectionTest { - private const string ClientScript = ""; - private readonly WebSocketScriptInjection ScriptInjection = new WebSocketScriptInjection(ClientScript); - [Fact] public async Task TryInjectLiveReloadScriptAsync_DoesNotInjectMarkup_IfInputDoesNotContainBodyTag() { @@ -22,7 +19,7 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh var input = Encoding.UTF8.GetBytes("
this is not a real body tag.
"); // Act - var result = await ScriptInjection.TryInjectLiveReloadScriptAsync(stream, input); + var result = await WebSocketScriptInjection.TryInjectLiveReloadScriptAsync(stream, input); // Assert Assert.False(result); @@ -37,7 +34,7 @@ namespace Microsoft.AspNetCore.Watch.BrowserRefresh $@"
This is the footer
-{ClientScript} +{WebSocketScriptInjection.InjectedScript} "; var stream = new MemoryStream(); var input = Encoding.UTF8.GetBytes( @@ -48,7 +45,7 @@ $@"