From 28c06aa189309b39efa6501575851e5d0e3a6073 Mon Sep 17 00:00:00 2001 From: DovydasGusarovas Date: Thu, 24 Jul 2025 10:29:59 +0300 Subject: [PATCH 1/3] Infinite loop redirecting fix --- src/Geta.404Handler/Core/RequestHandler.cs | 44 ++++++++++++------- .../RequestHandlerTests.cs | 26 ++++++++++- 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/src/Geta.404Handler/Core/RequestHandler.cs b/src/Geta.404Handler/Core/RequestHandler.cs index bad740b..4637e62 100644 --- a/src/Geta.404Handler/Core/RequestHandler.cs +++ b/src/Geta.404Handler/Core/RequestHandler.cs @@ -1,4 +1,4 @@ -// Copyright (c) Geta Digital. All rights reserved. +// Copyright (c) Geta Digital. All rights reserved. // Licensed under Apache-2.0. See the LICENSE file in the project root for more information using System; @@ -21,6 +21,7 @@ public class RequestHandler private readonly IConfiguration _configuration; private const string HandledRequestItemKey = "404handler:handled"; + private const string RedirectedOnceKey = "404handler:redirected"; private static readonly ILogger Logger = LogManager.GetLogger(); public RequestHandler( @@ -54,7 +55,7 @@ public virtual void Handle(HttpContextBase context) LogDebug("Not handled, custom redirect manager is set to off.", context); return; } - // If we're only doing this for remote users, we need to test for local host +// If we're only doing this for remote users, we need to test for local host if (_configuration.FileNotFoundHandlerMode == FileNotFoundMode.RemoteOnly) { // Determine if we're on localhost @@ -78,18 +79,24 @@ public virtual void Handle(HttpContextBase context) } var query = context.Request.ServerVariables["QUERY_STRING"]; - - // avoid duplicate log entries if (query != null && query.StartsWith("404;")) { LogDebug("Skipping request with 404; in the query string.", context); return; } + if (context.Items.Contains(RedirectedOnceKey)) + { + LogDebug("Redirect already attempted in this request, skipping to avoid loop.", context); + return; + } + var canHandleRedirect = HandleRequest(context.Request.UrlReferrer, notFoundUri, out var newUrl); if (canHandleRedirect && newUrl.State == (int)RedirectState.Saved) { - LogDebug("Handled saved URL", context); + LogDebug("Handled saved URL", context); + + context.Items[RedirectedOnceKey] = true; context .ClearServerError() @@ -98,18 +105,15 @@ public virtual void Handle(HttpContextBase context) else if (canHandleRedirect && newUrl.State == (int)RedirectState.Deleted) { LogDebug("Handled deleted URL", context); - SetStatusCodeAndShow404(context, 410); } else { LogDebug("Not handled. Current URL is ignored or no redirect found.", context); - SetStatusCodeAndShow404(context); } MarkHandled(context); - } private bool IsHandled(HttpContextBase context) @@ -140,22 +144,32 @@ public virtual bool HandleRequest(Uri referrer, Uri urlNotFound, out CustomRedir if (redirect.State.Equals((int)RedirectState.Saved)) { - // Found it, however, we need to make sure we're not running in an - // infinite loop. The new url must not be the referrer to this page - if (string.Compare(redirect.NewUrl, urlNotFound.PathAndQuery, StringComparison.InvariantCultureIgnoreCase) != 0) + // Prevent redirect loop + var currentPath = urlNotFound.PathAndQuery; + var newPath = redirect.NewUrl; + + if (string.Equals(newPath, currentPath, StringComparison.OrdinalIgnoreCase)) { + LogDebug($"Redirect leads back to the same path: {newPath}. Skipping to avoid loop.", null); + return false; + } - foundRedirect = redirect; - return true; + // Avoid redirecting back to the referrer (loop possibility) + if (referrer != null && + string.Equals(referrer.PathAndQuery, newPath, StringComparison.OrdinalIgnoreCase)) + { + LogDebug($"Redirect target matches referrer: {newPath}. Skipping to avoid loop.", null); + return false; } + + foundRedirect = redirect; + return true; } } else { - // log request to database - if logging is turned on. if (_configuration.Logging == LoggerMode.On) { - // Safe logging var logUrl = _configuration.LogWithHostname ? urlNotFound.ToString() : urlNotFound.PathAndQuery; _requestLogger.LogRequest(logUrl, referrer?.ToString()); } diff --git a/tests/Geta.404Handler.Tests/RequestHandlerTests.cs b/tests/Geta.404Handler.Tests/RequestHandlerTests.cs index ee8d27b..39c187e 100644 --- a/tests/Geta.404Handler.Tests/RequestHandlerTests.cs +++ b/tests/Geta.404Handler.Tests/RequestHandlerTests.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Web; using BVNetwork.NotFound.Core; using BVNetwork.NotFound.Core.Configuration; @@ -226,6 +226,28 @@ public void HandleRequest_returns_false_when_redirect_is_same_as_not_found() Assert.False(actual); } + [Fact] + public void HandleRequest_returns_false_when_redirect_matches_referrer() + { + // Arrange + var notFoundUri = new Uri("http://example.com/missing-page"); + var referrerUri = new Uri("http://example.com/redirect-target"); + + var redirect = new CustomRedirect(notFoundUri.ToString(), (int)RedirectState.Saved, 1) + { + NewUrl = referrerUri.PathAndQuery + }; + + WhenRedirectFound(redirect); + + // Act + var actual = _sut.HandleRequest(referrerUri, notFoundUri, out var foundRedirect); + + // Assert + Assert.False(actual); + Assert.Null(foundRedirect); + } + private void WhenRedirectFound(CustomRedirect redirect) { A.CallTo(() => _redirectHandler.Find(A._)).Returns(redirect); @@ -338,4 +360,4 @@ private void AssertNotHandled(HttpContextBase context) Assert.False(context.Response.TrySkipIisCustomErrors); } } -} \ No newline at end of file +} From c10a1ba2f9daf4245efb99fb2c0bfd1b3642091c Mon Sep 17 00:00:00 2001 From: DovydasGusarovas Date: Thu, 24 Jul 2025 10:32:26 +0300 Subject: [PATCH 2/3] Restoring old comments --- src/Geta.404Handler/Core/RequestHandler.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Geta.404Handler/Core/RequestHandler.cs b/src/Geta.404Handler/Core/RequestHandler.cs index 4637e62..1f0f124 100644 --- a/src/Geta.404Handler/Core/RequestHandler.cs +++ b/src/Geta.404Handler/Core/RequestHandler.cs @@ -55,7 +55,7 @@ public virtual void Handle(HttpContextBase context) LogDebug("Not handled, custom redirect manager is set to off.", context); return; } -// If we're only doing this for remote users, we need to test for local host + // If we're only doing this for remote users, we need to test for local host if (_configuration.FileNotFoundHandlerMode == FileNotFoundMode.RemoteOnly) { // Determine if we're on localhost @@ -79,6 +79,8 @@ public virtual void Handle(HttpContextBase context) } var query = context.Request.ServerVariables["QUERY_STRING"]; + + // avoid duplicate log entries if (query != null && query.StartsWith("404;")) { LogDebug("Skipping request with 404; in the query string.", context); @@ -105,15 +107,18 @@ public virtual void Handle(HttpContextBase context) else if (canHandleRedirect && newUrl.State == (int)RedirectState.Deleted) { LogDebug("Handled deleted URL", context); + SetStatusCodeAndShow404(context, 410); } else { LogDebug("Not handled. Current URL is ignored or no redirect found.", context); + SetStatusCodeAndShow404(context); } MarkHandled(context); + } private bool IsHandled(HttpContextBase context) @@ -168,8 +173,10 @@ public virtual bool HandleRequest(Uri referrer, Uri urlNotFound, out CustomRedir } else { + // log request to database - if logging is turned on. if (_configuration.Logging == LoggerMode.On) { + // Safe logging var logUrl = _configuration.LogWithHostname ? urlNotFound.ToString() : urlNotFound.PathAndQuery; _requestLogger.LogRequest(logUrl, referrer?.ToString()); } From 1c80702974d2f7947518be8af6ca07a1aff3a662 Mon Sep 17 00:00:00 2001 From: DovydasGusarovas Date: Thu, 24 Jul 2025 15:30:57 +0300 Subject: [PATCH 3/3] PR feedback changes --- src/Geta.404Handler/Core/RequestHandler.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Geta.404Handler/Core/RequestHandler.cs b/src/Geta.404Handler/Core/RequestHandler.cs index 1f0f124..516818d 100644 --- a/src/Geta.404Handler/Core/RequestHandler.cs +++ b/src/Geta.404Handler/Core/RequestHandler.cs @@ -96,7 +96,7 @@ public virtual void Handle(HttpContextBase context) var canHandleRedirect = HandleRequest(context.Request.UrlReferrer, notFoundUri, out var newUrl); if (canHandleRedirect && newUrl.State == (int)RedirectState.Saved) { - LogDebug("Handled saved URL", context); + LogDebug("Handled saved URL", context); context.Items[RedirectedOnceKey] = true; @@ -155,7 +155,7 @@ public virtual bool HandleRequest(Uri referrer, Uri urlNotFound, out CustomRedir if (string.Equals(newPath, currentPath, StringComparison.OrdinalIgnoreCase)) { - LogDebug($"Redirect leads back to the same path: {newPath}. Skipping to avoid loop.", null); + Logger.Debug($"Redirect leads back to the same path: {newPath}. Skipping to avoid loop."); return false; } @@ -163,7 +163,7 @@ public virtual bool HandleRequest(Uri referrer, Uri urlNotFound, out CustomRedir if (referrer != null && string.Equals(referrer.PathAndQuery, newPath, StringComparison.OrdinalIgnoreCase)) { - LogDebug($"Redirect target matches referrer: {newPath}. Skipping to avoid loop.", null); + Logger.Debug($"Redirect target matches referrer: {newPath}. Skipping to avoid loop."); return false; }