Compression under HTTPS with dotnet watch and launchBrowser: true breaks some HTML responses

[roa-nyx]

Describe the bug

Apologies if this is the wrong repo for this, took my best guess.

When using compression under HTTPS, with dotnet watch run and a certain controller response with launchBrowser: true, Kestrel responds with a badly encoded compressed response.

I'm not sure exactly which conditions need to be met for the controller response to trigger this bad behaviour but repro case seems to trigger it reliably on my machine. As far as I can tell the response must be chunked in a specific way to trigger this.

To Reproduce

Please use my zipped isolated bad case.
badcase-isolate.zip

EDIT 2021-05-17 1:43 PM: You can now find the bad case example in this repo.

When it's uncompressed simply run dotnet watch run NOTE: This doesn't reproduce when running without watch.

Navigate to https://localhost:5001 and ignore SSL warning.

Exceptions (if any)

Chrome says: GET https://localhost:5001/ net::ERR_CONTENT_DECODING_FAILED 200 and displays nothing.

Looking at fiddler is slightly more informative.

• Firstly Fiddler claims "Response body is encoded." However attempting to decode in Fiddler results in the following error in the logs:

09:03:37:0882 !Cannot decode HTTP response using Encoding: br; error: System.Exception Brotli conversion failed
09:03:37:0882 !Cannot decode HTTP response using Content-Encoding: br

• Secondly looking at the raw response in fiddler shows some interesting stuff:
  
  • Specifically notice what looks like uncompressed plaintext injection of /_framework/aspnetcore-browser-refresh.js into the html body.

To me it looks like something in Kestrel or some built-in middleware in ASP.Net core, has decided to inject the browser link functionality, despite the fact that I never enabled or triggered this. It looks like whatever decides to do this is not aware or unable to resolve the fact that the response is compressed and therefore it cannot be simply injected as plaintext.

Further technical details

• ASP.NET Core version 3.1.15
• dotnet --info

.NET SDK (reflecting any global.json):
 Version:   5.0.201
 Commit:    a09bd5c86c

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19042
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\5.0.201\

Host (useful for support):
  Version: 5.0.4
  Commit:  f27d337295

.NET SDKs installed:
  2.1.812 [C:\Program Files\dotnet\sdk]
  3.1.202 [C:\Program Files\dotnet\sdk]
  3.1.300 [C:\Program Files\dotnet\sdk]
  3.1.409 [C:\Program Files\dotnet\sdk]
  5.0.102 [C:\Program Files\dotnet\sdk]
  5.0.201 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.18 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.24 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.26 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.18 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.24 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.26 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.10 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.18 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.19 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.24 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.26 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.10 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.15 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.4 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.13 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.15 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.4 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET runtimes or SDKs:
  https://aka.ms/dotnet-download

• Using the latest release VS Code 1.56.2

Please let me know what else I can do to assist further in diagnosing and resolving this issue.

EDIT 2021-05-17 1:43 PM: I replaced the badcase-isolate.zip file with a link to this repo

[mkArtakMSFT]

Thank you for filing this issue. In order for us to investigate this issue, please provide a minimalistic repro project (not as a .ZIP file please: ideally a GitHub repo) that illustrates the problem.

[roa-nyx]

@mkArtakMSFT I have now created a repo with the example of the bad case and updated my previous post to point to it.

[Tratcher]

@pranavkm the script injector forgot to check Content-Encoding.

[pranavkm]

@Tratcher the injector would look for a plain text html tag which would not appear in the body. We have issues with compression and watch, and we usually tell users either to disable response compression in Development or reference the script manually in Development. But I'm not sure how we could corrupt the response. Particularly in this case, there's a script block that's not injected by dotnet-watch:

Any guesses?

[Tratcher]

Ha, looks like you're not the only injector. The hits get for bundles/site.js are all npm, SPA stuff. @SteveSandersonMS?

[SteveSandersonMS]

The script tag for bundles/site.js doesn’t appear to be injected dynamically. It’s just here in the layout file code:

https://github.com/roa-nyx/dotnetcore-bad-compression-example/blob/main/Views/Shared/_Layout.cshtml#L192

[pranavkm]

Btw, I tried running their app, running it with dotnet-watch, and downloading the results thru curl, and I did not see this issue i.e. a mix of binary and markup. Of course watch can't do any of it's fun browser restarts. Maybe this was a one-off thing or perhaps there's more at play required to repro the issue?

[roa-nyx]

I have tried reproducing using other HTML, or HTML with an equivalent length. I'm not at all sure why but when I change the HTML significantly it starts working.

The best I can guess is it depends on exactly how it gets chunked post-compression? Some very strange edge-case somewhere?

The reason I say that is I haven't gotten it to break when the response is not chunked. Also I'm not sure what the exact logic is that determines if it will chunk the response or not.

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[rjgotten]

Getting this when running an existing .NET Core 3.1 project under VS 2022 with Kestrel.
Don't get this when running that same project under VS 2019 with Kestrel.

Obviously not related to purely dotnet watch run - unless that is what VS 2022 also happens to call out to under the hood.

The following two question arise:

1. Primarily: what additional junk does VS 2022 try to transparantly inject for the new hot reloading features and how would I get completely rid of them to ensure that those are not the thing causing problems here? (Again...)
2. If this is not specific to something VS 2022 is doing with injections: What does Kestrel shipping with VS 2022 do different from the one with VS 2019, that could cause this?

---

[EDIT]
Can confirm that the injection of the scripts for the hot reloading feature is what triggers this.
Had to rip it out completely to get a working solution back. I.e. go specifically into Tools -> Options -> Debugging ->.NET / C++ Hot reload and disable all the server-side C# hot reloading features altogether in addition to everything hot-reloading related under Tools -> Options -> Projects & Solutions -> ASP.NET Core.

So; for whatever reason the injection of the scripts that control reloading of pages and on-page content are not tied to the settings that control the reloading behavior of pages and on-page content. They're tied to the flags that enable or disable hot reloading at the C# runtime level, directly.

Now; I don't know about anyone else, but to me that sounds very, very, very wrong...

It's now basically impossible to use any of the hot-reloading / hot-patching abilities AND impossible to use the legacy 'Edit and Continue' behavior without screwing things over on the ASP.NET side with these broken injections. Moreover; if I am using an alternative framework that handles its own hot-reloading, like Webpack, then I probably would not want these scripts present - period. They only become a risk to the application's stability at that point.

[rjgotten]

@roa-nyx
I have tried reproducing using other HTML, or HTML with an equivalent length. I'm not at all sure why but when I change the HTML significantly it starts working.

The middleware that injects the script tags to load the hot reload instrumentation performs a naive string find and replace/insert.
If the response compression middleware produces a result which for whatever reason contains the token it is looking for (which iirc is a basic closing </body> tag) verbatim, which CAN easily 'randomly' happen considering it's just a sequence of bytes that is being interpreted as, probably UTF-8, encoded text -- then it does the swap and corrupts the compressed stream.

If it can't, then it doesn't -- meaning the response will remain valid and work, but the hot reload features won't.

@pranavkm
But I'm not sure how we could corrupt the response.

Here's your answer:
https://github.com/dotnet/sdk/blob/36621631f5982fb9d23644aec68222015ba337a2/src/BuiltInTools/BrowserRefresh/WebSocketScriptInjection.cs#L28

Just treating any old response body for responses that are of Content-Type: text/html will break randomly on compressed response streams that happen to contain a series of bytes that matches with the UTF-8 byte-encoding of the string "</body>".
And it will randomly break on any character encoding other than UTF-8 where the same series of bytes may mean something else entirely as well.

Nobody working on the development of this feature thought it was a bad idea to just treat 'whatever bytes' as always representing UTF-8 encoded text? Seriously? Nobody? My god...

[DamianEdwards]

@rjgotten while I appreciate you're frustrated, I'd ask that you keep your interactions here respectful and inline with our code of conduct.

In this case we could consider updating the script injection middleware to disable itself in the case that a compression encoding is observed in the response headers, although that might depend on middleware ordering. Or it might be possible to detect the presence of the response compression middleware in the pipeline and disable based on that instead.

It also seems you'd appreciate an option to disable just the script injection portion of the Hot Reload experience. Note you can disable Hot Reload at the project level using the hotReloadEnabled property in the applicable launch profile in "launchProfile.settings" or via the "Launch Profiles" property page in Visual Studio (image below). Doing so allows Edit and Continue to keep working but disables the auto-browser refresh infrastructure, including the middleware.

[rjgotten]

In this case we could consider updating the script injection middleware to disable itself in the case that a compression encoding is observed in the response headers, although that might depend on middleware ordering.

It's not just the use of compression.
Consider also responses that are intentionally sent in character set encodings other than UTF-8.
And what about requests that are being fetched cross-domain with CORS?
Those are other blind spot in the current implementation.

Other than straight-up incompatibility at the level of the data format, consider also the fact that not all responses are meant for consumption as a top-level page with hot reload functionality attached.

The current solution already contains some hacky bits abusing a security header to infer that a page is being requested as content for an iframe. But...
What about XmlHttpRequest or Fetch based requests that fetch HTML documents, e.g. to load them into a web-based WYSIWYG editor for interactive editing?
What about endpoints which are just meant to serve out pre-cooked HTML documents from some online storage for download and connecting / passing on to other systems? What if you have tests in place to validate the contents of these responses; where running with a debugger attached or not should obviously produce the same results - i.e. without the scripts injected?

There's no way to know about these scenarios beforehand from the context of just the request or response headers.
You can only 'assume' and you know what they say: "to assume, makes ..." etc etc.

The best solution to this conundrum is to simply require developers to explicitly opt in to or out of the client-side hot reload features at a level of abstraction a few tiers further up.

For an explicit opt-in, a razor tag helper that developers should include would be one way to set it up.
An explicit opt-out could instead consist of a simple flag passed along with the HttpContext. Introduce an IBrowserRefreshFeature attached to the request which stores the opt-out flag; and have an extension method to retrieve the feature and flip that flag. At that point the BrowserRefreshMiddleware could simply retrieve that same feature and honor the flag value it finds.

Doing so allows Edit and Continue to keep working but disables the auto-browser refresh infrastructure, including the middleware.

And would it disable the server-side Hot Reload features as well? Or would those keep working? I.e. does this toggle only affect the client-side injectors or affect the entire Hot Reload infrastructure?

And; how would that option work when one is using launchSettings.json instead, when working on e.g. an older .NET Core 3.1 project? I checked and the option is not offered in the launch settings UI.

Really; really poor discoverability and lack of documentation references, btw.

[EDIT]
Went looking: https://docs.microsoft.com/en-us/visualstudio/debugger/hot-reload?view=vs-2022#configure-hot-reload
Seems that it disables the Hot Reload features in their entirety as a project-level override and it's not at all clear whether it actually leaves things like Edit and Continue working.

[DamianEdwards]

RE the launch profile setting, my local testing on an ASP.NET Core 6 project in latest VS 2022 shows that setting "hotReloadEnabled": false in a launch profile disables Hot Reload but preserves the traditional Edit and Continue experience. The big difference there of course is the ability to apply changes without launching under the debugger. We're looking at improving the documentation around these options along with improving the VS UX to allow better discovery of issues and more control over the elements of the experience (e.g. ability to disable just the script injection).

As you point out, given the nature of the web, there are countless ways dynamic script injection can cause issues or fail to actually deliver the script intended. The way we've implemented this right now is indeed causing pain for more folks than perhaps we anticipated. Your suggestion of using a feature (and perhaps endpoint metadata) is a good one, and I imagine we'd auto-opt-in responses generated by Razor Pages/MVC. @pranavkm this is something we should consider for .NET 7.

[jtkech]

Just for info, I had the same issue and had to use the same workaround as suggested by @rjgotten #32767 (comment)

Can confirm that the injection of the scripts for the hot reloading feature is what triggers this.
Had to rip it out completely to get a working solution back. I.e. go specifically into Tools -> Options -> Debugging ->.NET / C++ Hot reload and disable all the server-side C# hot reloading features altogether in addition to everything hot-reloading related under Tools -> Options -> Projects & Solutions -> ASP.NET Core.

[EDIT]

As an additional info, we had the issue in a multitenant context, only for the homepage of a given tenant e.g. https://localhost:44300/tenant1 but if we add an ending slash https://localhost:44300/tenant1/ it works.

[jtkech]

So in my case was caused by enabling Projects and Solutions > ASP.NET Core > CSS Hot Reload

Upgrading to Visual Studio 2022 17.1.0 fixed the issue

[stonstad]

I can confirm this is an active issue in VS 2022 17.1.2. I'm seeing 'ERR_CONTENT_DECODING_FAILED' in the browser console depending on output length when compression is enabled. Specifically, I have a CSHTML view that fails to load unless I add a few large meaningless HTML comments to alter its compressed length.

I can resolve the behavior by removing app.UseResponseCompression OR by adding hotReloadEnabled:false to launch settings OR by adding the following bs comment in the view markup:

LARGE BLOCK OF COMPRESSABLE TEXT TO WORK AROUND ASPNETCORE BUG
LARGE BLOCK OF COMPRESSABLE TEXT TO WORK AROUND ASPNETCORE BUG
LARGE BLOCK OF COMPRESSABLE TEXT TO WORK AROUND ASPNETCORE BUG 

I'm seeing this in VS 2022 17.1.2 with .NET 6.0 and Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation 6.0.3.

[yugabe]

I don't understand why is it so hard to let us do the work required manually, especially in cases like these. I had no idea why Hot Reload wasn't working for me for weeks now, and at first glance (a deep glance, though) it was related to response compression. Once I removed that, there was an error for something trying to connect to an unsecure HTTP endpoint (not even my own).

Oh yes, I shouldn't have used CSP either? That's the wrong answer. Provide a TagHelper or docs for me to manually enable Hot Reload. I don't want my HTTP responses messed with that inject script tags into what either is or isn't HTML. What's worse is that there is no reference to either: the middleware doesn't log any errors, the script doesn't get included, and there's not much to go on. I had to literally make a new project and do bottom-up debugging what's different.

The problem for me wasn't that the response was jumbled. There seemingly were no errors whatsover in the browser or in Kestrel logs, just Hot Reload kept throwing a "General Exception" in Visual Studio.

I'm all for reducing boilerplate. What I'm not okay with is there being IDE-specific or ambient settings that can break the application or IDE that are not explicit in any way.

[stonstad]

@DamianEdwards @SteveSandersonMS Has this issue been resolved? We are still seeing this behavior and it looks like others are as well.

[ghost]

Thanks for contacting us.
We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[rjgotten]

Because it's not immediately obvious that this is a bug in our framework

I know it's just a canned response from a bot; but ... come on; that's friggin' hilarious.
It is more than evident that this is a bug in the framework, stemming from the fact that the current .NET 6 solution for injecting the hot reload scripts, is just treating whatever is in the response stream as uncompressed raw UTF-8 byte-encoded text.

[yugabe]

I would be more than happy to inject the script(s) manually, if there was official guidance on how to do so. Now it's pretty much black magic (albeit it doesn't work).

[DamianEdwards]

This was improved for .NET 7 as the script is no longer injected if the closing body tag cannot be located (e.g. because it's compressed) or the content type is not UTF8, and the script-injection now detects that the failure is likely caused by the response compression and logs a warning. I've confirmed this using the .NET 7 SDK with the originally posted 3.1 repro app.

You can manually add the script with a block like the following:

@* Render the browser reload script *@
@if (Environment.GetEnvironmentVariable("__ASPNETCORE_BROWSER_TOOLS") != null)
{
<script src="/_framework/aspnetcore-browser-refresh.js"></script>
}

[DamianEdwards]

We'll wait for feedback that these changes have resolved the reported issues here before closing this issue.

[rjgotten]

What are the odds of those fixes to not inject into compressed streams or non-UTF8 streams, being backported to .NET 6?
Considering here that .NET 6 is LTS and .NET 7 is not.

[DamianEdwards]

The changes are in the SDK, not the runtime, so the LTS consideration doesn't apply in the same way. Visual Studio 2022 has already updated to use .NET 7 SDK in the latest previews.

[yugabe]

Thanks @DamianEdwards! I guess the middleware will still inject the script tag when it's able to, so manually putting it to the response will essentially render the script twice (when not using compression, for example)? It's not a big deal, but something else to consider.

I feel this shouldn't be an ambient setting in Visual Studio (which in turn, I expect, injects the magic environment variable to the app's runtime context), but it should be configured in the app itself explicitly. Essentially there is no need for a middleware at all if all it does is it injects the script, it could easily be added to the templates (like .cshtml) themselves:

<environment include="Development">
  <script src="/_framework/aspnetcore-browser-refresh.js"></script>
</environment>

Other frameworks (like Blazor, or any JS framework) could deal with this their own way.

Although, I was always a fan of being explicit, especially regarding tooling. It still hurts the DeveloperExceptionPage middleware is ambiently added to the pipeline.

Now, we:

1. toggle a setting in VS,
2. which in turn sets a magic environment variable,
3. the app has a middleware that picks up the environment variable and injects a script tag to all HTML responses.

The alternative of explicitly including the script seems simpler for me. My app might also have other HTML responses which should never be mutated in any way whatsoever (maybe I'm serving HTML templates for consumption in my app for end users, I don't know).

[DamianEdwards]

I appreciate your feedback @yugabe but as you can imagine we are trying to balance the experiences of different types of users and sometimes that results in less-than-ideal side-effects for some subset. In this case, it seems we should still pursue the idea being able to disable just the script-injection so that apps that wish to manually configure the script can do so cleanly.

I propose we add a new launch profile property and/or environment variable that disables the script injection, that would be honored by dotnet watch and Visual Studio.

[yugabe]

Thank you @DamianEdwards once again. That seems like the best way forward. I appreciate you seriously listening.
🤜🤛

[nmg196]

We just wasted 2.5 hours trying to resolve this problem in a .NET 6 project (it was made more difficult to find due to anti virus software changing the error message to "ERR_HTTP2_PROTOCOL_ERROR" instead of the ones mentioned in this thread).

Please consider backporting this fix to .NET 6, given that it's an LTS release and .NET 7 is not.

[DamianEdwards]

@nmg196 as I stated here the fix was made in the SDK, not in the runtime, so it's included automatically in supported versions of Visual Studio 2022 17.4 and above. It's only if you're using VS 17.2.x that you wouldn't get this fix? Is that the case?