Skip to content

Commit eb73b70

Browse files
authored
[Blazor] Emit action attribute when not explicit. (#51130)
This change updates the server rendering logic to always emit an absolute URL for a form action when none is specified explicitly. ## Description When as part of rendering a form, the developer does not explicitly add an `action` attribute, the framework will automatically generate one with the value of the current request URL. Fixes #51118 ## Customer Impact When enhanced navigation is active, the URL might change while an update to the page is in progress. In such situations, if the user clicks a button and submits a form, the form might incorrectly post to the wrong URL, as the page URL might have already been updated. Given that when the URL updates on the document is not a behavior, we have control over, we have to account for this, and to prevent it, we make sure that we always generate forms with an action attribute, which is unambiguous. ## Regression? - [ ] Yes - [X] No [If yes, specify the version the behavior has regressed from] ## Risk - [ ] High - [ ] Medium - [X] Low The fix is to detect forms without an `action` attribute and emit it to the current URL. This is correct in all cases and is unambiguous. ## Verification - [x] Manual (required) - [X] Automated ## Packaging changes reviewed? - [ ] Yes - [ ] No - [x] N/A ---- ## When servicing release/2.1 - [ ] Make necessary changes in eng/PatchConfig.props
1 parent aae9a79 commit eb73b70

File tree

9 files changed

+326
-122
lines changed

9 files changed

+326
-122
lines changed

src/Components/Samples/BlazorUnitedApp/Program.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
app.UseHttpsRedirection();
2525

2626
app.UseStaticFiles();
27+
app.UseAntiforgery();
2728

2829
app.MapRazorComponents<App>();
2930

src/Components/Web.JS/dist/Release/blazor.web.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Components/Web.JS/src/Services/NavigationEnhancement.ts

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ let currentEnhancedNavigationAbortController: AbortController | null;
3535
let navigationEnhancementCallbacks: NavigationEnhancementCallbacks;
3636
let performingEnhancedPageLoad: boolean;
3737

38+
// This gets initialized to the current URL when we load.
39+
// After that, it gets updated every time we successfully complete a navigation.
40+
let currentContentUrl = location.href;
41+
3842
export interface NavigationEnhancementCallbacks {
3943
documentUpdated: () => void;
4044
enhancedNavigationCompleted: () => void;
@@ -217,10 +221,25 @@ export async function performEnhancedPageLoad(internalDestinationHref: string, f
217221
return;
218222
}
219223

220-
if (!response.redirected && !isGetRequest && response.url !== location.href && isSuccessResponse) {
221-
isNonRedirectedPostToADifferentUrlMessage = `Cannot perform enhanced form submission that changes the URL (except via a redirection), because then back/forward would not work. Either remove this form\'s \'action\' attribute, or change its method to \'get\', or do not mark it as enhanced.\nOld URL: ${location.href}\nNew URL: ${response.url}`;
224+
if (!response.redirected && !isGetRequest && isSuccessResponse) {
225+
// If this is the result of a form post that didn't trigger a redirection.
226+
if (!isForSamePath(response)) {
227+
// In this case we don't want to push the currentContentUrl to the history stack because we don't know if this is a location
228+
// we can navigate back to (as we don't know if the location supports GET) and we are not able to replicate the Resubmit form?
229+
// browser behavior.
230+
// The only case where this is acceptable is when the last content URL, is the same as the URL for the form we posted to.
231+
isNonRedirectedPostToADifferentUrlMessage = `Cannot perform enhanced form submission that changes the URL (except via a redirection), because then back/forward would not work. Either remove this form\'s \'action\' attribute, or change its method to \'get\', or do not mark it as enhanced.\nOld URL: ${location.href}\nNew URL: ${response.url}`;
232+
} else {
233+
if (location.href !== currentContentUrl) {
234+
// The url on the browser might be out of data, so push an entry to the stack to update the url in place.
235+
history.pushState(null, '', currentContentUrl);
236+
}
237+
}
222238
}
223239

240+
// Set the currentContentUrl to the location of the last completed navigation.
241+
currentContentUrl = response.url;
242+
224243
const responseContentType = response.headers.get('content-type');
225244
if (responseContentType?.startsWith('text/html') && initialContent) {
226245
// For HTML responses, regardless of the status code, display it
@@ -277,6 +296,20 @@ export async function performEnhancedPageLoad(internalDestinationHref: string, f
277296
throw new Error(isNonRedirectedPostToADifferentUrlMessage);
278297
}
279298
}
299+
300+
function isForSamePath(response: Response) {
301+
// We are trying to determine if the response URL is compatible with the last content URL that was successfully loaded on to
302+
// the page.
303+
// We are going to use the scheme, host, port and path to determine if they are compatible. We do not account for the query string
304+
// as we want to allow for the query string to change. (Blazor doesn't use the query string for routing purposes).
305+
306+
const responseUrl = new URL(response.url);
307+
const currentContentUrlParsed = new URL(currentContentUrl!);
308+
return responseUrl.protocol === currentContentUrlParsed.protocol
309+
&& responseUrl.host === currentContentUrlParsed.host
310+
&& responseUrl.port === currentContentUrlParsed.port
311+
&& responseUrl.pathname === currentContentUrlParsed.pathname;
312+
}
280313
}
281314

282315
async function getResponsePartsWithFraming(responsePromise: Promise<Response>, abortSignal: AbortSignal, onInitialDocument: (response: Response, initialDocumentText: string) => void, onStreamingElement: (streamingElementMarkup) => void) {
@@ -369,7 +402,7 @@ function enhancedNavigationIsEnabledForLink(element: HTMLAnchorElement): boolean
369402
function enhancedNavigationIsEnabledForForm(form: HTMLFormElement): boolean {
370403
// For forms, they default *not* to being enhanced, and must be enabled explicitly on the form element itself (not an ancestor).
371404
const attributeValue = form.getAttribute('data-enhance');
372-
return typeof(attributeValue) === 'string'
405+
return typeof (attributeValue) === 'string'
373406
&& attributeValue === '' || attributeValue?.toLowerCase() === 'true';
374407
}
375408

src/Components/Web/src/HtmlRendering/StaticHtmlRenderer.HtmlWriting.cs

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,9 @@ private int RenderElement(int componentId, TextWriter output, ArrayRange<RenderT
108108
output.Write(frame.ElementName);
109109
int afterElement;
110110
var isTextArea = string.Equals(frame.ElementName, "textarea", StringComparison.OrdinalIgnoreCase);
111+
var isForm = string.Equals(frame.ElementName, "form", StringComparison.OrdinalIgnoreCase);
111112
// We don't want to include value attribute of textarea element.
112-
var afterAttributes = RenderAttributes(output, frames, position + 1, frame.ElementSubtreeLength - 1, !isTextArea, out var capturedValueAttribute);
113+
var afterAttributes = RenderAttributes(output, frames, position + 1, frame.ElementSubtreeLength - 1, !isTextArea, isForm: isForm, out var capturedValueAttribute);
113114

114115
// When we see an <option> as a descendant of a <select>, and the option's "value" attribute matches the
115116
// "value" attribute on the <select>, then we auto-add the "selected" attribute to that option. This is
@@ -270,15 +271,23 @@ private static bool TryFindEnclosingElementFrame(ArrayRange<RenderTreeFrame> fra
270271
}
271272

272273
private int RenderAttributes(
273-
TextWriter output, ArrayRange<RenderTreeFrame> frames, int position, int maxElements, bool includeValueAttribute, out string? capturedValueAttribute)
274+
TextWriter output,
275+
ArrayRange<RenderTreeFrame> frames,
276+
int position,
277+
int maxElements,
278+
bool includeValueAttribute,
279+
bool isForm,
280+
out string? capturedValueAttribute)
274281
{
275282
capturedValueAttribute = null;
276283

277284
if (maxElements == 0)
278285
{
286+
EmitFormActionIfNotExplicit(output, isForm, hasExplicitActionValue: false);
279287
return position;
280288
}
281289

290+
var hasExplicitActionValue = false;
282291
for (var i = 0; i < maxElements; i++)
283292
{
284293
var candidateIndex = position + i;
@@ -291,6 +300,7 @@ private int RenderAttributes(
291300
continue;
292301
}
293302

303+
EmitFormActionIfNotExplicit(output, isForm, hasExplicitActionValue);
294304
return candidateIndex;
295305
}
296306

@@ -304,6 +314,12 @@ private int RenderAttributes(
304314
}
305315
}
306316

317+
if (isForm && frame.AttributeName.Equals("action", StringComparison.OrdinalIgnoreCase) &&
318+
!string.IsNullOrEmpty(frame.AttributeValue as string))
319+
{
320+
hasExplicitActionValue = true;
321+
}
322+
307323
switch (frame.AttributeValue)
308324
{
309325
case bool flag when flag:
@@ -323,7 +339,22 @@ private int RenderAttributes(
323339
}
324340
}
325341

342+
EmitFormActionIfNotExplicit(output, isForm, hasExplicitActionValue);
343+
326344
return position + maxElements;
345+
346+
void EmitFormActionIfNotExplicit(TextWriter output, bool isForm, bool hasExplicitActionValue)
347+
{
348+
if (isForm && _navigationManager != null && !hasExplicitActionValue)
349+
{
350+
output.Write(' ');
351+
output.Write("action");
352+
output.Write('=');
353+
output.Write('\"');
354+
_htmlEncoder.Encode(output, _navigationManager.Uri);
355+
output.Write('\"');
356+
}
357+
}
327358
}
328359

329360
private int RenderChildren(int componentId, TextWriter output, ArrayRange<RenderTreeFrame> frames, int position, int maxElements)

src/Components/Web/src/HtmlRendering/StaticHtmlRenderer.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using Microsoft.AspNetCore.Components.RenderTree;
77
using Microsoft.AspNetCore.Components.Web;
88
using Microsoft.AspNetCore.Components.Web.HtmlRendering;
9+
using Microsoft.Extensions.DependencyInjection;
910
using Microsoft.Extensions.Logging;
1011

1112
namespace Microsoft.AspNetCore.Components.HtmlRendering.Infrastructure;
@@ -18,6 +19,7 @@ namespace Microsoft.AspNetCore.Components.HtmlRendering.Infrastructure;
1819
public partial class StaticHtmlRenderer : Renderer
1920
{
2021
private static readonly Task CanceledRenderTask = Task.FromCanceled(new CancellationToken(canceled: true));
22+
private readonly NavigationManager? _navigationManager;
2123

2224
/// <summary>
2325
/// Constructs an instance of <see cref="StaticHtmlRenderer"/>.
@@ -27,6 +29,7 @@ public partial class StaticHtmlRenderer : Renderer
2729
public StaticHtmlRenderer(IServiceProvider serviceProvider, ILoggerFactory loggerFactory)
2830
: base(serviceProvider, loggerFactory)
2931
{
32+
_navigationManager = serviceProvider.GetService<NavigationManager>();
3033
}
3134

3235
/// <inheritdoc/>

0 commit comments

Comments
 (0)