Skip to content

Commit

Permalink
Block postbacks for 5s after redirects
Browse files Browse the repository at this point in the history
The page does not unload right after we  perform
a navigation, and allows the user to hit the submit
button a second time. In the past, we used to disable
all postbacks after a redirect, but that also blocks
the page after a file is returned. There probably isn't
a 100% reliable way to detect if the location change
is a navigation or a file return.

With this patch, we will block the page again, but only
for a limited time (5 seconds). We also only only block
the postback queue, which means that only postbacks
with Concurrency=Deny or Queue will be affected.

Standard file returns as provided by DotVVM are
excluded from this, but it should also work acceptably
with custom file returns (some buttons will not work
for 5 seconds after the file return)
  • Loading branch information
exyi committed Sep 7, 2024
1 parent 3a3f38c commit 4746085
Show file tree
Hide file tree
Showing 11 changed files with 227 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@ public static void RedirectToRoutePermanent(this IDotvvmRequestContext context,
context.RedirectToUrlPermanent(url, replaceInHistory, allowSpaRedirect);
}

public static void SetRedirectResponse(this IDotvvmRequestContext context, string url, int statusCode = (int)HttpStatusCode.Redirect, bool replaceInHistory = false, bool allowSpaRedirect = false) =>
context.Configuration.ServiceProvider.GetRequiredService<IHttpRedirectService>().WriteRedirectResponse(context.HttpContext, url, statusCode, replaceInHistory, allowSpaRedirect);
public static void SetRedirectResponse(this IDotvvmRequestContext context, string url, int statusCode = (int)HttpStatusCode.Redirect, bool replaceInHistory = false, bool allowSpaRedirect = false, string? downloadName = null) =>
context.Configuration.ServiceProvider.GetRequiredService<IHttpRedirectService>().WriteRedirectResponse(context.HttpContext, url, statusCode, replaceInHistory, allowSpaRedirect, downloadName);

internal static Task SetCachedViewModelMissingResponse(this IDotvvmRequestContext context)
{
Expand Down Expand Up @@ -262,8 +262,10 @@ public static async Task ReturnFileAsync(this IDotvvmRequestContext context, Str
AttachmentDispositionType = attachmentDispositionType ?? "attachment"
};

var downloadAttribute = attachmentDispositionType == "inline" ? null : fileName;

var generatedFileId = await returnedFileStorage.StoreFileAsync(stream, metadata).ConfigureAwait(false);
context.SetRedirectResponse(context.TranslateVirtualPath("~/dotvvmReturnedFile?id=" + generatedFileId));
context.SetRedirectResponse(context.TranslateVirtualPath("~/dotvvmReturnedFile?id=" + generatedFileId), downloadName: downloadAttribute);
throw new DotvvmInterruptRequestExecutionException(InterruptReason.ReturnFile, fileName);
}

Expand Down
7 changes: 3 additions & 4 deletions src/Framework/Framework/Hosting/HttpRedirectService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,13 @@ namespace DotVVM.Framework.Hosting
{
public interface IHttpRedirectService
{
void WriteRedirectResponse(IHttpContext httpContext, string url, int statusCode = (int)HttpStatusCode.Redirect, bool replaceInHistory = false, bool allowSpaRedirect = false);
void WriteRedirectResponse(IHttpContext httpContext, string url, int statusCode = (int)HttpStatusCode.Redirect, bool replaceInHistory = false, bool allowSpaRedirect = false, string? downloadName = null);
}

public class DefaultHttpRedirectService: IHttpRedirectService
{
public void WriteRedirectResponse(IHttpContext httpContext, string url, int statusCode = (int)HttpStatusCode.Redirect, bool replaceInHistory = false, bool allowSpaRedirect = false)
public void WriteRedirectResponse(IHttpContext httpContext, string url, int statusCode = (int)HttpStatusCode.Redirect, bool replaceInHistory = false, bool allowSpaRedirect = false, string? downloadName = null)
{

if (DotvvmRequestContext.DetermineRequestType(httpContext) is DotvvmRequestType.Navigate)
{
httpContext.Response.Headers["Location"] = url;
Expand All @@ -43,7 +42,7 @@ public void WriteRedirectResponse(IHttpContext httpContext, string url, int stat
httpContext.Response.StatusCode = 200;
httpContext.Response.ContentType = "application/json";
httpContext.Response
.WriteAsync(DefaultViewModelSerializer.GenerateRedirectActionResponse(url, replaceInHistory, allowSpaRedirect))
.WriteAsync(DefaultViewModelSerializer.GenerateRedirectActionResponse(url, replaceInHistory, allowSpaRedirect, downloadName))
.GetAwaiter().GetResult();
// ^ we just wait for this Task. Redirect API never was async and the response size is small enough that we can't quite safely wait for the result
// .GetAwaiter().GetResult() preserves stack traces across async calls, thus I like it more that .Wait()
Expand Down
17 changes: 14 additions & 3 deletions src/Framework/Framework/Resources/Scripts/postback/redirect.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
import * as events from '../events';
import * as magicNavigator from '../utils/magic-navigator'
import { handleSpaNavigationCore } from "../spa/spa";
import { delay } from '../utils/promise';


export function performRedirect(url: string, replace: boolean, allowSpa: boolean): Promise<any> {
if (replace) {
location.replace(url);
return Promise.resolve();
} else if (compileConstants.isSpa && allowSpa) {
return handleSpaNavigationCore(url);
} else {
magicNavigator.navigate(url);
return Promise.resolve();
}

// When performing redirect, we pretend that the request takes additional X second to avoid
// double submit with Postback.Concurrency=Deny or Queue.
// We do not want to block the page forever, as the redirect might just return a file (or HTTP 204/205),
// and the page will continue to live.
return delay(5_000);
}

export async function handleRedirect(options: PostbackOptions, resultObject: any, response: Response, replace: boolean = false): Promise<DotvvmRedirectEventArgs> {
Expand All @@ -28,7 +34,12 @@ export async function handleRedirect(options: PostbackOptions, resultObject: any
}
events.redirect.trigger(redirectArgs);

await performRedirect(url, replace, resultObject.allowSpa);
const downloadFileName = resultObject.download
if (downloadFileName != null) {
magicNavigator.navigate(url, downloadFileName)
} else {
await performRedirect(url, replace, resultObject.allowSpa);
}

return redirectArgs;
}
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ const fetchDefinitions = {
postbackRedirect: async <T>(url: string, init: RequestInit) => {
return {
action: "redirect",
url: "/newUrl"
url: "/newUrl",
download: "some-file" // say it's a file, so that DotVVM does not block postback queue after the test
} as any;
},

Expand Down Expand Up @@ -213,7 +214,8 @@ const fetchDefinitions = {
return {
action: "redirect",
url: "/newUrl",
allowSpa: true
allowSpa: true,
download: "some-file"
} as any;
},
spaNavigateError: async <T>(url: string, init: RequestInit) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
let fakeAnchor: HTMLAnchorElement | undefined;
export function navigate(url: string) {
export function navigate(url: string, downloadName: string | null | undefined = null) {
if (!fakeAnchor) {
fakeAnchor = <HTMLAnchorElement> document.createElement("a");
fakeAnchor.style.display = "none";
document.body.appendChild(fakeAnchor);
}
if (downloadName == null) {
fakeAnchor.removeAttribute("download");
} else {
fakeAnchor.download = downloadName
}
fakeAnchor.href = url;
fakeAnchor.click();
}
1 change: 1 addition & 0 deletions src/Framework/Framework/Resources/Scripts/utils/promise.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/** Runs the callback in the next event loop cycle */
export const defer = <T>(callback: () => T) => Promise.resolve().then(callback)
export const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms))
Original file line number Diff line number Diff line change
Expand Up @@ -258,14 +258,15 @@ public JObject BuildResourcesJson(IDotvvmRequestContext context, Func<string, bo
/// <summary>
/// Serializes the redirect action.
/// </summary>
public static string GenerateRedirectActionResponse(string url, bool replace, bool allowSpa)
public static string GenerateRedirectActionResponse(string url, bool replace, bool allowSpa, string? downloadName)
{
// create result object
var result = new JObject();
result["url"] = url;
result["action"] = "redirect";
if (replace) result["replace"] = true;
if (allowSpa) result["allowSpa"] = true;
if (downloadName is object) result["download"] = downloadName;
return result.ToString(Formatting.None);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
using System.Diagnostics.Metrics;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using DotVVM.Core.Storage;
using DotVVM.Framework.ViewModel;

namespace DotVVM.Samples.BasicSamples.ViewModels.FeatureSamples.Redirect
{
class RedirectPostbackConcurrencyViewModel : DotvvmViewModelBase
{
public static int GlobalCounter = 0;
private readonly IReturnedFileStorage returnedFileStorage;

[Bind(Direction.ServerToClient)]
public int Counter { get; set; } = GlobalCounter;

public int MiniCounter { get; set; } = 0;

[FromQuery("empty")]
public bool IsEmptyPage { get; set; } = false;
[FromQuery("loadDelay")]
public int LoadDelay { get; set; } = 0;

public RedirectPostbackConcurrencyViewModel(IReturnedFileStorage returnedFileStorage)
{
this.returnedFileStorage = returnedFileStorage;
}
public override async Task Init()
{
await Task.Delay(LoadDelay); // delay to enable user to click DelayIncrement button between it succeeding and loading the next page
await base.Init();
}

public async Task DelayIncrement()
{
await Task.Delay(1000);

Interlocked.Increment(ref GlobalCounter);

Context.RedirectToRoute(Context.Route.RouteName, query: new { empty = true, loadDelay = 2000 });
}

public async Task GetFileStandard()
{
await Context.ReturnFileAsync("test file"u8.ToArray(), "test.txt", "text/plain");
}

public async Task GetFileCustom()
{
var metadata = new ReturnedFileMetadata()
{
FileName = "test_custom.txt",
MimeType = "text/plain",
AttachmentDispositionType = "attachment"
};

var stream = new MemoryStream("test custom file"u8.ToArray());
var generatedFileId = await returnedFileStorage.StoreFileAsync(stream, metadata).ConfigureAwait(false);

var url = Context.TranslateVirtualPath("~/dotvvmReturnedFile?id=" + generatedFileId);
Context.RedirectToUrl(url);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
@viewModel DotVVM.Samples.BasicSamples.ViewModels.FeatureSamples.Redirect.RedirectPostbackConcurrencyViewModel, DotVVM.Samples.Common
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>Hello from DotVVM!</title>
</head>
<body>
<div IncludeInPage={resource: IsEmptyPage}>
<dot:RouteLink RouteName={resource: Context.Route.RouteName} Query-empty=false>Back</dot:RouteLink>
</div>
<div class="container" IncludeInPage={resource: !IsEmptyPage}>
<h1>Redirect and postback concurrency test</h1>
<p>
Testing Concurrency=Deny / Concurrency=Queue with redirect and file returns.
</p>
<p>First, we have a set of buttons incrementing a static variable, each takes about 2sec and redirects to a blank page afterwards</p>
<p>GlobalCounter = <span data-ui="counter" InnerText={value: Counter} /></p>
<p>MiniCounter(Concurrency=Deny) = <dot:Button data-ui=minicounter Text={value: MiniCounter} PostBack.Concurrency=Deny Click={staticCommand: MiniCounter = MiniCounter + 1} /></p>

<p>
<dot:Button data-ui="inc-default" Click={command: DelayIncrement()} PostBack.Concurrency=Default>Increment (Concurrency=Default)</dot:Button>
<dot:Button data-ui="inc-deny" Click={command: DelayIncrement()} PostBack.Concurrency=Deny>Increment (Concurrency=Deny)</dot:Button>
<dot:Button data-ui="inc-queue" Click={command: DelayIncrement()} PostBack.Concurrency=Queue>Increment (Concurrency=Queue)</dot:Button>
</p>

<p>We also test that returning files does not block the page forever, </p>

<p>
<dot:Button data-ui="file-std" Click={command: GetFileStandard()}>Standard return file</dot:Button>
<dot:Button data-ui="file-custom" Click={command: GetFileCustom()}>Custom return file (will have delay before page works)</dot:Button>
</p>
</div>
</body>
</html>

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

94 changes: 93 additions & 1 deletion src/Samples/Tests/Tests/Feature/RedirectTests.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
using System;
using System.Linq;
using System.Threading;
using DotVVM.Samples.Tests.Base;
using DotVVM.Testing.Abstractions;
using OpenQA.Selenium;
using Riganti.Selenium.Core;
using Riganti.Selenium.Core.Abstractions;
using Riganti.Selenium.DotVVM;
using Xunit;
using Xunit.Abstractions;
Expand Down Expand Up @@ -69,6 +73,94 @@ public void Feature_Redirect_RedirectionHelpers()
Assert.Matches($"{SamplesRouteUrls.FeatureSamples_Redirect_RedirectionHelpers_PageE}/1221\\?x=a", currentUrl.LocalPath + currentUrl.Query);
});
}


bool TryClick(IElementWrapper element)
{
if (element is null) return false;
try
{
element.Click();
return true;
}
catch (StaleElementReferenceException)
{
return false;
}
}

[Fact]
public void Feature_Redirect_RedirectPostbackConcurrency()
{
RunInAllBrowsers(browser => {
browser.NavigateToUrl(SamplesRouteUrls.FeatureSamples_Redirect_RedirectPostbackConcurrency);
int globalCounter() => int.Parse(browser.First("counter", SelectByDataUi).GetText());
var initialCounter = globalCounter();
for (int i = 0; i < 20; i++)
{
TryClick(browser.FirstOrDefault("inc-default", SelectByDataUi));
Thread.Sleep(1);
}
browser.WaitFor(() => Assert.Contains("empty=true", browser.CurrentUrl, StringComparison.OrdinalIgnoreCase), 7000, "Redirect did not happen");
browser.NavigateToUrl(SamplesRouteUrls.FeatureSamples_Redirect_RedirectPostbackConcurrency);
// must increment at least 20 times, otherwise delays are too short
Assert.Equal(globalCounter(), initialCounter + 20);
initialCounter = globalCounter();
var clickCount = 0;
while (TryClick(browser.FirstOrDefault("inc-deny", SelectByDataUi)))
{
clickCount++;
Thread.Sleep(1);
}
Assert.InRange(clickCount, 3, int.MaxValue);
browser.WaitFor(() => Assert.Contains("empty=true", browser.CurrentUrl, StringComparison.OrdinalIgnoreCase), timeout: 500, "Redirect did not happen");
browser.NavigateToUrl(SamplesRouteUrls.FeatureSamples_Redirect_RedirectPostbackConcurrency);
Assert.Equal(globalCounter(), initialCounter + 1); // only one click was allowed
initialCounter = globalCounter();
clickCount = 0;
while (TryClick(browser.FirstOrDefault("inc-queue", SelectByDataUi)))
{
clickCount++;
Thread.Sleep(1);
}
Assert.InRange(clickCount, 3, int.MaxValue);
browser.WaitFor(() => Assert.Contains("empty=true", browser.CurrentUrl, StringComparison.OrdinalIgnoreCase), timeout: 500, "Redirect did not happen");
browser.NavigateToUrl(SamplesRouteUrls.FeatureSamples_Redirect_RedirectPostbackConcurrency);
Assert.Equal(globalCounter(), initialCounter + 1); // only one click was allowed
});
}

[Fact]
public void Feature_Redirect_RedirectPostbackConcurrencyFileReturn()
{
RunInAllBrowsers(browser => {
browser.NavigateToUrl(SamplesRouteUrls.FeatureSamples_Redirect_RedirectPostbackConcurrency);
void increment(int timeout)
{
browser.WaitFor(() => {
var original = int.Parse(browser.First("minicounter", SelectByDataUi).GetText());
browser.First("minicounter", SelectByDataUi).Click();
AssertUI.TextEquals(browser.First("minicounter", SelectByDataUi), (original + 1).ToString());
}, timeout, "Could not increment minicounter in given timeout (postback queue is blocked)");
}
increment(3000);
browser.First("file-std", SelectByDataUi).Click();
increment(3000);
browser.First("file-custom", SelectByDataUi).Click();
// longer timeout, because DotVVM blocks postback queue for 5s after redirects to debounce any further requests
increment(15000);
});
}
}
}

0 comments on commit 4746085

Please sign in to comment.