Prevent browser refresh script from injecting into iframes #21954
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
added
the
|
|
||
| if (request.Headers.TryGetValue("Sec-Fetch-Dest", out var values) && | ||
| !StringValues.IsNullOrEmpty(values) && | ||
| !string.Equals(values[0], "document", StringComparison.OrdinalIgnoreCase)) |
There was a problem hiding this comment.
The spec doesn't talk about any other casing than document so strictly speaking I think this could be StringComparison.Ordinal. However I can't think of any real drawback to doing a case-insensitive check.
SteveSandersonMS
approved these changes
Oct 11, 2021
|
I'll file a VS issue and follow up with @jodavis about getting it resolved there. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The aspnet-browser-refresh.js script is injected in to documents that appear to destined to be displayed in the browser. As part of dotnet/aspnetcore#37326, we realized the script was being added to an iframe that appears in the background. (We only found this because the particular page being served uses CSP that is designed to prevent modifications). Even if amending the script had succeeded in this case, VS which uses this code currently does not support having multiple instances of the script calling back.
The fix is to avoid injecting the scripts in responses that are clearly not destined for a visible browser document.
Partly fixes dotnet/aspnetcore#37326
Customer impact
Warnings in the browser console when running F5 / Ctrl-F5 on a React or Angular project template
Testing
Risk
Low