Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If-None-Match and If-Modified-Since headers not being process by .NET 6 on VS 2022 #36291

Closed
vgmello opened this issue Sep 8, 2021 · 11 comments
Assignees
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions investigate

Comments

@vgmello
Copy link

vgmello commented Sep 8, 2021

Describe the bug

I'm not sure if this issue should be reported here or as a VS 2022 bug, however, for some reason whenever an aspnet app is started within VS 2022 even without the debugger attached, the app can not read the If-None-Match and If-Modified-Since request headers, there are never included in header collection. I'm experiencing this behavior in both Kestrel and IIS Express, and with minimal APIs and normal controllers. Everything works fine if I target docker as launch profile.

image

To Reproduce

Dummy code

using Microsoft.Net.Http.Headers;

var builder = WebApplication.CreateBuilder(args);
var app = builder.Build();

if (app.Environment.IsDevelopment())
{
    app.UseDeveloperExceptionPage();
}

app.MapGet("/", (HttpRequest request) => {
    var handler = new HttpClientHandler()
    {
        ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator
    };

    var httpClient = new HttpClient(handler);
    httpClient.DefaultRequestHeaders.TryAddWithoutValidation(HeaderNames.IfNoneMatch, "\"abc\"");
    httpClient.DefaultRequestHeaders.TryAddWithoutValidation("TestHeader", "abc");

    return httpClient.GetStringAsync($"{request.Scheme}://localhost:{request.HttpContext.Connection.LocalPort}/test");
});
app.MapGet("/test", (HttpRequest request) => $"{PrintHeaders(request.Headers)}");

string PrintHeaders(IHeaderDictionary headers) =>
    headers.Aggregate(string.Empty, (result, header) => result += $"{header.Key}: {header.Value}\r\n");

app.Run();

Exceptions (if any)

No exceptions were thrown

Further technical details

Framework: Tests on .NET 5 and .NET 6

Environment

.NET SDK (reflecting any global.json):
 Version:   6.0.100-preview.7.21379.14
 Commit:    22d70b47bc

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19043
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\6.0.100-preview.7.21379.14\

Host (useful for support):
  Version: 6.0.0-preview.7.21377.19
  Commit:  91ba01788d

.NET SDKs installed:
  5.0.400 [C:\Program Files\dotnet\sdk]
  6.0.100-preview.5.21302.13 [C:\Program Files\dotnet\sdk]
  6.0.100-preview.7.21379.14 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.29 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.29 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.18 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.0-preview.5.21301.17 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.0-preview.7.21378.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.29 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.18 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.0-preview.5.21301.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.0-preview.7.21377.19 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.18 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.9 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.0-preview.5.21301.4 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.0-preview.7.21378.9 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

IDE:

Microsoft Visual Studio Professional 2022 Preview
Version 17.0.0 Preview 3.1
VisualStudio.17.Preview/17.0.0-pre.3.1+31612.314
Microsoft .NET Framework
Version 4.8.04084
@mrtluke
Copy link

mrtluke commented Sep 22, 2021

having the same issue myself, and i could be wrong, but for my case it seems to be Microsoft.WebTools.BrowserLink.Net.BrowserLinkMiddleWareUtil.RemoveETagAndTimeStamp that is removing it, and I don't have browser link enabled in the aspnetcore website or in VS.

@vgmello
Copy link
Author

vgmello commented Sep 24, 2021

@mrtluke this is very interesting, the browser link is also disabled on my VS2022. However, disabling the "Hot Reload CSS Changes" fixed the issue with etag.

image

@sebastienros
Copy link
Member

sebastienros commented Oct 7, 2021

I can reproduce the issue with If-Modified-Since and If-None-Match headers. There might be others. Etag is not filtered in my case.

Setting the option Hot Reload CSS changes to True triggers the issue.

image

Simpler code to repro:

var builder = WebApplication.CreateBuilder(args);
var app = builder.Build();

app.MapGet("/", async context =>
{ 
    await context.Response.WriteAsync(context.Request.Headers
        .Aggregate(string.Empty, (result, header) => result += $"{header.Key}: {header.Value}\r\n"));
});

app.Run();

Repro on VS 17.0.0 Preview 4.1 and .NET 6.0 RC1

@sebastienros
Copy link
Member

@jodavis Could you help with this issue?

@jodavis
Copy link

jodavis commented Oct 7, 2021

@vgmello @mrtluke The "Hot Reload CSS changes" setting is removing those headers. It needs to inject a script to make the feature work. Assuming the browser's cached copy won't contain the script (or won't contain the right URL, since the port can change) it clears those headers to break the cache.

It's smarter with ETags. It modifies the ETag in a way that includes the URL, so the browser's cached content can be used as long as the URL doesn't change.

@mrtluke
Copy link

mrtluke commented Oct 8, 2021

@jodavis thanks for the reply.

It needs to inject a script to make the feature work. Assuming the browser's cached copy won't contain the script (or won't contain the right URL, since the port can change) it clears those headers to break the cache.

in my case, this is affecting a custom json/object controller that is trying to handle these headers (and produces the etag itself, for its own requests).
is this something that always needs to run for non html/mvc requests since no script will ever be present?

the hot reloading in general is a great thing to have for the UI, so it completely makes sense for that scenario, but not really for a custom controller thats trying to handle the client/requests. maybe im not on the same page or im forgetting other scenarios, so just my two cents from my view.

@jodavis
Copy link

jodavis commented Oct 9, 2021

@mrtluke Good point, we have a filter further in the codepath that decides whether or not to inject the script, and it wouldn't inject in the case of non-HTML responses. We should avoid affecting these headers in that case. I'll file an issue to fix this in Visual Studio.

@mrtluke
Copy link

mrtluke commented Oct 10, 2021

@jodavis sounds great, thank you.

@aviita
Copy link

aviita commented Jan 14, 2022

Hmm. Seems I was looking into wrong repo. Created a duplicate in: #39516.

@rjgotten
Copy link

rjgotten commented Jan 14, 2022

It's smarter with ETags. It modifies the ETag in a way that includes the URL, so the browser's cached content can be used as long as the URL doesn't change.

You know what would be even smarter? Writing a hot reloader which gets sent hot patches via a different URL altogether which contains a cache busting token so you never have to mess with headers that result in dumb situations such as poorly written code stripping out perfectly valid cache headers on resources that have zero to do with CSS under the purview of hot reloading.

Microsoft should have a little look at prior art which gets this right, like Webpack. And then spend some time ripping out their botched job and redoing it properly.

Arrived here from #39516 and I'm sorry to say: at the moment it doesn't surprise me in the slightest anymore that, once again, this turns out to be a problem with the atrocious implementation of hot reloading for ASP.NET that is injected when using VS 2022.

Just like the asinine problem where it can completely corrupt any content sent with GZip or Brotli compression because the middleware that injects the hot reloading scripts is dumb as bricks and will gleefully attempt injection directly into a compressed stream of content, wherever it finds what it (very, very falsely) believes to be the marker for the insertion point.
One of those other beauties that can send developers down an 8 hour rabbit hole.

@sebastienros
Copy link
Member

Can I close this issue since there is no action to do in aspnet?

@ghost ghost locked as resolved and limited conversation to collaborators Jun 27, 2022
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions investigate
Projects
None yet
Development

No branches or pull requests

8 participants