diff --git a/src/Umbraco.Core/Routing/IPublishedRequest.cs b/src/Umbraco.Core/Routing/IPublishedRequest.cs index 4ffe565489..971a8a2f98 100644 --- a/src/Umbraco.Core/Routing/IPublishedRequest.cs +++ b/src/Umbraco.Core/Routing/IPublishedRequest.cs @@ -60,14 +60,7 @@ namespace Umbraco.Web.Routing /// /// Does not actually set the http response status code, only registers that the response /// should use the specified code. The code will or will not be used, in due time. - int ResponseStatusCode { get; } - - /// - /// Gets the content request http response status description. - /// - /// Does not actually set the http response status description, only registers that the response - /// should use the specified description. The description will or will not be used, in due time. - string ResponseStatusDescription { get; } + int? ResponseStatusCode { get; } /// /// Gets a list of Extensions to append to the Response.Cache object. diff --git a/src/Umbraco.Core/Routing/IPublishedRequestBuilder.cs b/src/Umbraco.Core/Routing/IPublishedRequestBuilder.cs index f94972412f..38c685b096 100644 --- a/src/Umbraco.Core/Routing/IPublishedRequestBuilder.cs +++ b/src/Umbraco.Core/Routing/IPublishedRequestBuilder.cs @@ -39,7 +39,7 @@ namespace Umbraco.Web.Routing /// /// Gets the content request http response status code. /// - int ResponseStatusCode { get; } + int? ResponseStatusCode { get; } /// /// Gets the current assigned (if any) @@ -77,7 +77,7 @@ namespace Umbraco.Web.Routing /// The requested content. /// Depending on UmbracoSettings.InternalRedirectPreservesTemplate, will /// preserve or reset the template, if any. - IPublishedRequestBuilder SetInternalRedirectPublishedContent(IPublishedContent content); + IPublishedRequestBuilder SetInternalRedirectPublishedContent(IPublishedContent content); // TODO: Need to figure this one out /// /// Tries to set the template to use to display the requested content. @@ -97,11 +97,6 @@ namespace Umbraco.Web.Routing /// Setting the template does refresh RenderingEngine. IPublishedRequestBuilder SetTemplate(ITemplate template); - /// - /// Resets the template. - /// - IPublishedRequestBuilder ResetTemplate(); - /// /// Indicates that the content request should trigger a permanent redirect (301). /// @@ -123,11 +118,10 @@ namespace Umbraco.Web.Routing /// Sets the http response status code, along with an optional associated description. /// /// The http status code. - /// The description. /// Does not actually set the http response status code and description, only registers that /// the response should use the specified code and description. The code and description will or will /// not be used, in due time. - IPublishedRequestBuilder SetResponseStatus(int code, string description = null); + IPublishedRequestBuilder SetResponseStatus(int code); IPublishedRequestBuilder SetCacheabilityNoCache(bool cacheability); @@ -140,16 +134,5 @@ namespace Umbraco.Web.Routing /// Sets a dictionary of Headers to append to the Response object. /// IPublishedRequestBuilder SetHeaders(IReadOnlyDictionary headers); - - /// - /// Sets a value indicating that the requested content could not be found. - /// - /// This is set in the PublishedContentRequestBuilder and can also be used in - /// custom content finders or Prepared event handlers, where we want to allow developers - /// to indicate a request is 404 but not to cancel it. - IPublishedRequestBuilder SetIs404(bool is404); - - // TODO: This seems to be the same as is404? - //IPublishedRequestBuilder UpdateToNotFound(); } } diff --git a/src/Umbraco.Core/Routing/PublishedRequest.cs b/src/Umbraco.Core/Routing/PublishedRequest.cs index 135a756600..fbc72247b1 100644 --- a/src/Umbraco.Core/Routing/PublishedRequest.cs +++ b/src/Umbraco.Core/Routing/PublishedRequest.cs @@ -16,20 +16,19 @@ namespace Umbraco.Web.Routing /// /// Initializes a new instance of the class. /// - public PublishedRequest(Uri uri, /*bool ignorePublishedContentCollisions, */IPublishedContent publishedContent, bool isInternalRedirectPublishedContent, ITemplate template, DomainAndUri domain, CultureInfo culture, string redirectUrl, int responseStatusCode, string responseStatusDescription, IReadOnlyList cacheExtensions, IReadOnlyDictionary headers, bool cacheabilityNoCache) + public PublishedRequest(Uri uri, /*bool ignorePublishedContentCollisions, */IPublishedContent publishedContent, bool isInternalRedirectPublishedContent, ITemplate template, DomainAndUri domain, CultureInfo culture, string redirectUrl, int? responseStatusCode, IReadOnlyList cacheExtensions, IReadOnlyDictionary headers, bool cacheabilityNoCache) { Uri = uri ?? throw new ArgumentNullException(nameof(uri)); //IgnorePublishedContentCollisions = ignorePublishedContentCollisions; - PublishedContent = publishedContent ?? throw new ArgumentNullException(nameof(publishedContent)); + PublishedContent = publishedContent; IsInternalRedirectPublishedContent = isInternalRedirectPublishedContent; - Template = template ?? throw new ArgumentNullException(nameof(template)); - Domain = domain ?? throw new ArgumentNullException(nameof(domain)); + Template = template; + Domain = domain; Culture = culture ?? throw new ArgumentNullException(nameof(culture)); - RedirectUrl = redirectUrl ?? throw new ArgumentNullException(nameof(redirectUrl)); + RedirectUrl = redirectUrl; ResponseStatusCode = responseStatusCode; - ResponseStatusDescription = responseStatusDescription ?? throw new ArgumentNullException(nameof(responseStatusDescription)); - CacheExtensions = cacheExtensions ?? throw new ArgumentNullException(nameof(cacheExtensions)); - Headers = headers ?? throw new ArgumentNullException(nameof(headers)); + CacheExtensions = cacheExtensions; + Headers = headers; CacheabilityNoCache = cacheabilityNoCache; } @@ -42,9 +41,6 @@ namespace Umbraco.Web.Routing /// public IPublishedContent PublishedContent { get; } - /// - public IPublishedContent InitialPublishedContent { get; } - /// public bool IsInternalRedirectPublishedContent { get; } @@ -61,10 +57,7 @@ namespace Umbraco.Web.Routing public string RedirectUrl { get; } /// - public int ResponseStatusCode { get; } - - /// - public string ResponseStatusDescription { get; } + public int? ResponseStatusCode { get; } /// public IReadOnlyList CacheExtensions { get; } diff --git a/src/Umbraco.Core/Routing/PublishedRequestBuilder.cs b/src/Umbraco.Core/Routing/PublishedRequestBuilder.cs index 6441fcabf7..9ff0ce5dce 100644 --- a/src/Umbraco.Core/Routing/PublishedRequestBuilder.cs +++ b/src/Umbraco.Core/Routing/PublishedRequestBuilder.cs @@ -17,19 +17,19 @@ namespace Umbraco.Web.Routing private IReadOnlyList _cacheExtensions; private IPublishedContent _internalRedirectContent; private string _redirectUrl; - private HttpStatusCode _responseStatus = HttpStatusCode.NotFound; - private string _responseDesc; + private HttpStatusCode? _responseStatus; /// /// Initializes a new instance of the class. /// - public PublishedRequestBuilder(IFileService fileService) + public PublishedRequestBuilder(Uri uri, IFileService fileService) { + Uri = uri; _fileService = fileService; } /// - public Uri Uri { get; private set; } + public Uri Uri { get; } /// public DomainAndUri Domain { get; private set; } @@ -44,7 +44,7 @@ namespace Umbraco.Web.Routing public bool IsInternalRedirectPublishedContent { get; private set; } // TODO: Not sure what this is yet /// - public int ResponseStatusCode => (int)_responseStatus; + public int? ResponseStatusCode => _responseStatus.HasValue ? (int?)_responseStatus : null; /// public IPublishedContent PublishedContent { get; private set; } @@ -58,19 +58,11 @@ namespace Umbraco.Web.Routing Domain, Culture, _redirectUrl, - (int)_responseStatus, - _responseDesc, + _responseStatus.HasValue ? (int?)_responseStatus : null, _cacheExtensions, _headers, _cacheability); - /// - public IPublishedRequestBuilder ResetTemplate() - { - Template = null; - return this; - } - /// public IPublishedRequestBuilder SetCacheabilityNoCache(bool cacheability) { @@ -113,13 +105,6 @@ namespace Umbraco.Web.Routing return this; } - /// - public IPublishedRequestBuilder SetIs404(bool is404) - { - _responseStatus = HttpStatusCode.NotFound; - return this; - } - /// public IPublishedRequestBuilder SetPublishedContent(IPublishedContent content) { @@ -144,10 +129,9 @@ namespace Umbraco.Web.Routing } /// - public IPublishedRequestBuilder SetResponseStatus(int code, string description = null) + public IPublishedRequestBuilder SetResponseStatus(int code) { _responseStatus = (HttpStatusCode)code; - _responseDesc = description; return this; } diff --git a/src/Umbraco.Core/Routing/PublishedRequestExtensions.cs b/src/Umbraco.Core/Routing/PublishedRequestExtensions.cs index b3eb21ed16..2a4e4323f0 100644 --- a/src/Umbraco.Core/Routing/PublishedRequestExtensions.cs +++ b/src/Umbraco.Core/Routing/PublishedRequestExtensions.cs @@ -2,14 +2,42 @@ using System.Net; namespace Umbraco.Web.Routing { + public static class PublishedRequestExtensions { + /// + /// Gets the + /// + public static UmbracoRouteResult GetRouteResult(this IPublishedRequest publishedRequest) + { + if (publishedRequest.IsRedirect()) + { + return UmbracoRouteResult.Redirect; + } + + if (!publishedRequest.HasPublishedContent()) + { + return UmbracoRouteResult.NotFound; + } + + return UmbracoRouteResult.Success; + } + /// /// Gets a value indicating whether the request was successfully routed /// public static bool Success(this IPublishedRequest publishedRequest) => !publishedRequest.IsRedirect() && publishedRequest.HasPublishedContent(); + /// + /// Sets the response status to be 404 not found + /// + public static IPublishedRequestBuilder SetIs404(this IPublishedRequestBuilder publishedRequest) + { + publishedRequest.SetResponseStatus((int)HttpStatusCode.NotFound); + return publishedRequest; + } + /// /// Gets a value indicating whether the content request has a content. /// diff --git a/src/Umbraco.Core/Routing/PublishedRouter.cs b/src/Umbraco.Core/Routing/PublishedRouter.cs index 7f4bc6f22f..15dab49bad 100644 --- a/src/Umbraco.Core/Routing/PublishedRouter.cs +++ b/src/Umbraco.Core/Routing/PublishedRouter.cs @@ -72,7 +72,7 @@ namespace Umbraco.Web.Routing } /// - public IPublishedRequestBuilder CreateRequest(Uri uri) => new PublishedRequestBuilder(_fileService); + public IPublishedRequestBuilder CreateRequest(Uri uri) => new PublishedRequestBuilder(uri, _fileService); /// public bool TryRouteRequest(IPublishedRequestBuilder request) @@ -106,6 +106,10 @@ namespace Umbraco.Web.Routing /// public IPublishedRequest RouteRequest(IPublishedRequestBuilder request) { + //// trigger the Preparing event - at that point anything can still be changed + //// the idea is that it is possible to change the uri + //request.OnPreparing(); + // find domain FindDomain(request); @@ -411,7 +415,7 @@ namespace Umbraco.Web.Routing // handle not found if (request.PublishedContent == null) { - request.SetIs404(true); + request.SetIs404(); _logger.LogDebug("HandlePublishedContent: No document, try last chance lookup"); // if it fails then give up, there isn't much more that we can do diff --git a/src/Umbraco.Core/Routing/UmbracoRouteResult.cs b/src/Umbraco.Core/Routing/UmbracoRouteResult.cs new file mode 100644 index 0000000000..9f42f372ac --- /dev/null +++ b/src/Umbraco.Core/Routing/UmbracoRouteResult.cs @@ -0,0 +1,20 @@ +namespace Umbraco.Web.Routing +{ + public enum UmbracoRouteResult + { + /// + /// Routing was successful and a content item was matched + /// + Success, + + /// + /// A redirection took place + /// + Redirect, + + /// + /// Nothing matched + /// + NotFound + } +} diff --git a/src/Umbraco.Infrastructure/Routing/ContentFinderByConfigured404.cs b/src/Umbraco.Infrastructure/Routing/ContentFinderByConfigured404.cs index 75fc80015a..231a68df58 100644 --- a/src/Umbraco.Infrastructure/Routing/ContentFinderByConfigured404.cs +++ b/src/Umbraco.Infrastructure/Routing/ContentFinderByConfigured404.cs @@ -111,7 +111,7 @@ namespace Umbraco.Web.Routing frequest .SetPublishedContent(content) - .SetIs404(true); + .SetIs404(); return content != null; } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/ContentModelBinderTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/ContentModelBinderTests.cs index f539d25152..d62497b173 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/ContentModelBinderTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/ContentModelBinderTests.cs @@ -12,9 +12,11 @@ using Moq; using NUnit.Framework; using Umbraco.Core; using Umbraco.Core.Models.PublishedContent; +using Umbraco.Core.Services; using Umbraco.Web.Common.ModelBinders; using Umbraco.Web.Common.Routing; using Umbraco.Web.Models; +using Umbraco.Web.Routing; namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.ModelBinders { @@ -211,9 +213,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.ModelBinders /// private ModelBindingContext CreateBindingContextForUmbracoRequest(Type modelType, IPublishedContent publishedContent) { + var builder = new PublishedRequestBuilder(new Uri("https://example.com"), Mock.Of()); + builder.SetPublishedContent(publishedContent); + IPublishedRequest publishedRequest = builder.Build(); + var httpContext = new DefaultHttpContext(); var routeData = new RouteData(); - routeData.Values.Add(Constants.Web.UmbracoRouteDefinitionDataToken, new UmbracoRouteValues(publishedContent)); + routeData.Values.Add(Constants.Web.UmbracoRouteDefinitionDataToken, new UmbracoRouteValues(publishedRequest)); { } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Website/Controllers/SurfaceControllerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Website/Controllers/SurfaceControllerTests.cs index 31603eb8c0..07f8118ad0 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Website/Controllers/SurfaceControllerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Website/Controllers/SurfaceControllerTests.cs @@ -162,8 +162,11 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Website.Controllers var umbracoContextAccessor = new TestUmbracoContextAccessor(umbracoContext); IPublishedContent content = Mock.Of(publishedContent => publishedContent.Id == 12345); + var builder = new PublishedRequestBuilder(umbracoContext.CleanedUmbracoUrl, Mock.Of()); + builder.SetPublishedContent(content); + IPublishedRequest publishedRequest = builder.Build(); - var routeDefinition = new UmbracoRouteValues(content); + var routeDefinition = new UmbracoRouteValues(publishedRequest); var routeData = new RouteData(); routeData.Values.Add(CoreConstants.Web.UmbracoRouteDefinitionDataToken, routeDefinition); diff --git a/src/Umbraco.Web.Common/ActionsResults/PublishedContentNotFoundResult.cs b/src/Umbraco.Web.Common/ActionsResults/PublishedContentNotFoundResult.cs new file mode 100644 index 0000000000..3e90a40f09 --- /dev/null +++ b/src/Umbraco.Web.Common/ActionsResults/PublishedContentNotFoundResult.cs @@ -0,0 +1,60 @@ +using System.Net; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Umbraco.Web.Routing; + +namespace Umbraco.Web.Common.ActionsResults +{ + /// + /// Returns the Umbraco not found result + /// + public class PublishedContentNotFoundResult : IActionResult + { + private readonly IUmbracoContext _umbracoContext; + private readonly string _message; + + /// + /// Initializes a new instance of the class. + /// + public PublishedContentNotFoundResult(IUmbracoContext umbracoContext, string message = null) + { + _umbracoContext = umbracoContext; + _message = message; + } + + /// + public async Task ExecuteResultAsync(ActionContext context) + { + HttpResponse response = context.HttpContext.Response; + + response.Clear(); + + response.StatusCode = StatusCodes.Status404NotFound; + + IPublishedRequest frequest = _umbracoContext.PublishedRequest; + var reason = "Cannot render the page at URL '{0}'."; + if (frequest.HasPublishedContent() == false) + { + reason = "No umbraco document matches the URL '{0}'."; + } + else if (frequest.HasTemplate() == false) + { + reason = "No template exists to render the document at URL '{0}'."; + } + + await response.WriteAsync("

Page not found

"); + await response.WriteAsync("

"); + await response.WriteAsync(string.Format(reason, WebUtility.HtmlEncode(_umbracoContext.OriginalRequestUrl.PathAndQuery))); + await response.WriteAsync("

"); + if (string.IsNullOrWhiteSpace(_message) == false) + { + await response.WriteAsync("

" + _message + "

"); + } + + await response.WriteAsync("

This page can be replaced with a custom 404. Check the documentation for \"custom 404\".

"); + await response.WriteAsync("

This page is intentionally left ugly ;-)

"); + await response.WriteAsync(""); + } + } +} diff --git a/src/Umbraco.Web.Common/Controllers/PublishedRequestFilterAttribute.cs b/src/Umbraco.Web.Common/Controllers/PublishedRequestFilterAttribute.cs new file mode 100644 index 0000000000..61e21df4cb --- /dev/null +++ b/src/Umbraco.Web.Common/Controllers/PublishedRequestFilterAttribute.cs @@ -0,0 +1,77 @@ +using System; +using System.Collections.Generic; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Filters; +using Umbraco.Web.Common.Routing; +using Umbraco.Web.Routing; + +namespace Umbraco.Web.Common.Controllers +{ + /// + /// Deals with custom headers for the umbraco request + /// + internal class PublishedRequestFilterAttribute : ResultFilterAttribute + { + /// + /// Gets the + /// + protected UmbracoRouteValues GetUmbracoRouteValues(ResultExecutingContext context) + { + if (!context.RouteData.Values.TryGetValue(Core.Constants.Web.UmbracoRouteDefinitionDataToken, out var def)) + { + throw new InvalidOperationException($"No route value found with key {Core.Constants.Web.UmbracoRouteDefinitionDataToken}"); + } + + return (UmbracoRouteValues)def; + } + + /// + /// Deals with custom headers for the umbraco request + /// + public override void OnResultExecuting(ResultExecutingContext context) + { + UmbracoRouteValues routeVals = GetUmbracoRouteValues(context); + IPublishedRequest pcr = routeVals.PublishedRequest; + + // now we can deal with headers, etc... + if (pcr.ResponseStatusCode.HasValue) + { + // set status code -- even for redirects + context.HttpContext.Response.StatusCode = pcr.ResponseStatusCode.Value; + } + + AddCacheControlHeaders(context, pcr); + + if (pcr.Headers != null) + { + foreach (KeyValuePair header in pcr.Headers) + { + context.HttpContext.Response.Headers.Append(header.Key, header.Value); + } + } + } + + private void AddCacheControlHeaders(ResultExecutingContext context, IPublishedRequest pcr) + { + var cacheControlHeaders = new List(); + + if (pcr.CacheabilityNoCache) + { + cacheControlHeaders.Add("no-cache"); + } + + if (pcr.CacheExtensions != null) + { + foreach (var cacheExtension in pcr.CacheExtensions) + { + cacheControlHeaders.Add(cacheExtension); + } + } + + if (cacheControlHeaders.Count > 0) + { + context.HttpContext.Response.Headers["Cache-Control"] = string.Join(", ", cacheControlHeaders); + } + } + } +} diff --git a/src/Umbraco.Web.Common/Controllers/RenderController.cs b/src/Umbraco.Web.Common/Controllers/RenderController.cs index 099dfd59cd..7f6d61de98 100644 --- a/src/Umbraco.Web.Common/Controllers/RenderController.cs +++ b/src/Umbraco.Web.Common/Controllers/RenderController.cs @@ -1,9 +1,11 @@ using System; +using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.ViewEngines; using Microsoft.Extensions.Logging; -using Umbraco.Core; using Umbraco.Core.Models.PublishedContent; +using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.Common.Filters; using Umbraco.Web.Common.Routing; using Umbraco.Web.Models; @@ -11,30 +13,50 @@ using Umbraco.Web.Routing; namespace Umbraco.Web.Common.Controllers { - /// /// Represents the default front-end rendering controller. /// [ModelBindingException] + [PublishedRequestFilter] public class RenderController : UmbracoController, IRenderController { private readonly ILogger _logger; private readonly ICompositeViewEngine _compositeViewEngine; + private readonly IUmbracoContextAccessor _umbracoContextAccessor; private UmbracoRouteValues _umbracoRouteValues; /// /// Initializes a new instance of the class. /// - public RenderController(ILogger logger, ICompositeViewEngine compositeViewEngine) + public RenderController(ILogger logger, ICompositeViewEngine compositeViewEngine, IUmbracoContextAccessor umbracoContextAccessor) { _logger = logger; _compositeViewEngine = compositeViewEngine; + _umbracoContextAccessor = umbracoContextAccessor; } /// /// Gets the current content item. /// - protected IPublishedContent CurrentPage => UmbracoRouteValues.PublishedContent; + protected IPublishedContent CurrentPage + { + get + { + if (!UmbracoRouteValues.PublishedRequest.HasPublishedContent()) + { + // This will never be accessed this way since the controller will handle redirects and not founds + // before this can be accessed but we need to be explicit. + throw new InvalidOperationException("There is no published content found in the request"); + } + + return UmbracoRouteValues.PublishedRequest.PublishedContent; + } + } + + /// + /// Gets the umbraco context + /// + protected IUmbracoContext UmbracoContext => _umbracoContextAccessor.UmbracoContext; /// /// Gets the @@ -95,5 +117,41 @@ namespace Umbraco.Web.Common.Controllers /// The default action to render the front-end view. /// public virtual IActionResult Index() => CurrentTemplate(new ContentModel(CurrentPage)); + + /// + /// Before the controller executes we will handle redirects and not founds + /// + public override async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) + { + IPublishedRequest pcr = UmbracoRouteValues.PublishedRequest; + + _logger.LogDebug( + "Response status: Redirect={Redirect}, Is404={Is404}, StatusCode={ResponseStatusCode}", + pcr.IsRedirect() ? (pcr.IsRedirectPermanent() ? "permanent" : "redirect") : "none", + pcr.Is404() ? "true" : "false", + pcr.ResponseStatusCode); + + UmbracoRouteResult routeStatus = pcr.GetRouteResult(); + switch (routeStatus) + { + case UmbracoRouteResult.Redirect: + + // set the redirect result and do not call next to short circuit + context.Result = pcr.IsRedirectPermanent() + ? RedirectPermanent(pcr.RedirectUrl) + : Redirect(pcr.RedirectUrl); + break; + case UmbracoRouteResult.NotFound: + + // set the redirect result and do not call next to short circuit + context.Result = new PublishedContentNotFoundResult(UmbracoContext); + break; + case UmbracoRouteResult.Success: + default: + // continue normally + await next(); + break; + } + } } } diff --git a/src/Umbraco.Web.Common/ModelBinders/ContentModelBinder.cs b/src/Umbraco.Web.Common/ModelBinders/ContentModelBinder.cs index d747a4ff86..7bdf1b13af 100644 --- a/src/Umbraco.Web.Common/ModelBinders/ContentModelBinder.cs +++ b/src/Umbraco.Web.Common/ModelBinders/ContentModelBinder.cs @@ -27,7 +27,7 @@ namespace Umbraco.Web.Common.ModelBinders return Task.CompletedTask; } - BindModel(bindingContext, umbracoRouteValues.PublishedContent, bindingContext.ModelType); + BindModel(bindingContext, umbracoRouteValues.PublishedRequest.PublishedContent, bindingContext.ModelType); return Task.CompletedTask; } diff --git a/src/Umbraco.Web.Common/Routing/UmbracoRouteValues.cs b/src/Umbraco.Web.Common/Routing/UmbracoRouteValues.cs index 2ab047a757..8622bc689e 100644 --- a/src/Umbraco.Web.Common/Routing/UmbracoRouteValues.cs +++ b/src/Umbraco.Web.Common/Routing/UmbracoRouteValues.cs @@ -20,7 +20,7 @@ namespace Umbraco.Web.Common.Routing /// Initializes a new instance of the class. /// public UmbracoRouteValues( - IPublishedContent publishedContent, + IPublishedRequest publishedRequest, string controllerName = null, Type controllerType = null, string actionName = DefaultActionName, @@ -29,7 +29,7 @@ namespace Umbraco.Web.Common.Routing { ControllerName = controllerName ?? ControllerExtensions.GetControllerName(); ControllerType = controllerType ?? typeof(RenderController); - PublishedContent = publishedContent; + PublishedRequest = publishedRequest; HasHijackedRoute = hasHijackedRoute; ActionName = actionName; TemplateName = templateName; @@ -56,9 +56,9 @@ namespace Umbraco.Web.Common.Routing public Type ControllerType { get; } /// - /// Gets the + /// Gets the /// - public IPublishedContent PublishedContent { get; } + public IPublishedRequest PublishedRequest { get; } /// /// Gets a value indicating whether the current request has a hijacked route/user controller routed for it diff --git a/src/Umbraco.Web.Website/ActionResults/RedirectToUmbracoPageResult.cs b/src/Umbraco.Web.Website/ActionResults/RedirectToUmbracoPageResult.cs index 93b3a5bb0d..d8816c48a9 100644 --- a/src/Umbraco.Web.Website/ActionResults/RedirectToUmbracoPageResult.cs +++ b/src/Umbraco.Web.Website/ActionResults/RedirectToUmbracoPageResult.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Specialized; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; @@ -12,6 +12,7 @@ using Umbraco.Web.Routing; namespace Umbraco.Web.Website.ActionResults { + /// /// Redirects to an Umbraco page by Id or Entity /// diff --git a/src/Umbraco.Web.Website/Controllers/SurfaceController.cs b/src/Umbraco.Web.Website/Controllers/SurfaceController.cs index 390da69453..3a6a7a3507 100644 --- a/src/Umbraco.Web.Website/Controllers/SurfaceController.cs +++ b/src/Umbraco.Web.Website/Controllers/SurfaceController.cs @@ -44,7 +44,7 @@ namespace Umbraco.Web.Website.Controllers } var routeDef = routeDefAttempt.Result; - return routeDef.PublishedContent; + return routeDef.PublishedRequest.PublishedContent; } } diff --git a/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs index 64d7cd0205..a94ee5e678 100644 --- a/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs @@ -23,8 +23,6 @@ namespace Umbraco.Web.Website.DependencyInjection /// public static IUmbracoBuilder AddWebsite(this IUmbracoBuilder builder) { - builder.Services.AddUnique(); - builder.WithCollectionBuilder() .Add(builder.TypeLoader.GetSurfaceControllers()); diff --git a/src/Umbraco.Web.Website/Extensions/UmbracoWebsiteApplicationBuilderExtensions.cs b/src/Umbraco.Web.Website/Extensions/UmbracoWebsiteApplicationBuilderExtensions.cs index 5ceb5e523f..36e5ff9214 100644 --- a/src/Umbraco.Web.Website/Extensions/UmbracoWebsiteApplicationBuilderExtensions.cs +++ b/src/Umbraco.Web.Website/Extensions/UmbracoWebsiteApplicationBuilderExtensions.cs @@ -37,9 +37,6 @@ namespace Umbraco.Extensions { app.UseEndpoints(endpoints => { - NoContentRoutes noContentRoutes = app.ApplicationServices.GetRequiredService(); - noContentRoutes.CreateRoutes(endpoints); - endpoints.MapDynamicControllerRoute("/{**slug}"); }); diff --git a/src/Umbraco.Web.Website/Routing/NoContentRoutes.cs b/src/Umbraco.Web.Website/Routing/NoContentRoutes.cs deleted file mode 100644 index f2f2e8dfe3..0000000000 --- a/src/Umbraco.Web.Website/Routing/NoContentRoutes.cs +++ /dev/null @@ -1,59 +0,0 @@ -using Microsoft.AspNetCore.Builder; -using Microsoft.AspNetCore.Routing; -using Microsoft.Extensions.Options; -using Umbraco.Core; -using Umbraco.Core.Configuration; -using Umbraco.Core.Configuration.Models; -using Umbraco.Core.Hosting; -using Umbraco.Web.Common.Routing; - -namespace Umbraco.Web.Website.Routing -{ - /// - /// Creates route for the no content page - /// - public class NoContentRoutes : IAreaRoutes - { - private readonly IRuntimeState _runtimeState; - private readonly string _umbracoPathSegment; - - /// - /// Initializes a new instance of the class. - /// - public NoContentRoutes( - IOptions globalSettings, - IHostingEnvironment hostingEnvironment, - IRuntimeState runtimeState) - { - _runtimeState = runtimeState; - _umbracoPathSegment = globalSettings.Value.GetUmbracoMvcArea(hostingEnvironment); - } - - /// - public void CreateRoutes(IEndpointRouteBuilder endpoints) - { - switch (_runtimeState.Level) - { - case RuntimeLevel.Install: - break; - case RuntimeLevel.Upgrade: - break; - case RuntimeLevel.Run: - - // TODO: I don't really think this is working AFAIK the code has just been migrated but it's not really enabled - // yet. Our route handler needs to be aware that there is no content and redirect there. Though, this could all be - // managed directly in UmbracoRouteValueTransformer. Else it could actually do a 'redirect' but that would need to be - // an internal rewrite. - endpoints.MapControllerRoute( - Constants.Web.NoContentRouteName, // named consistently - _umbracoPathSegment + "/UmbNoContent", - new { controller = "RenderNoContent", action = "Index" }); - break; - case RuntimeLevel.BootFailed: - case RuntimeLevel.Unknown: - case RuntimeLevel.Boot: - break; - } - } - } -} diff --git a/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs b/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs index f087a6203e..dc72777fa8 100644 --- a/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs +++ b/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs @@ -93,14 +93,19 @@ namespace Umbraco.Web.Website.Routing return values; } - bool routed = RouteRequest(_umbracoContextAccessor.UmbracoContext, out IPublishedRequest publishedRequest); - if (!routed) + // Check if there is no existing content and return the no content controller + if (!_umbracoContextAccessor.UmbracoContext.Content.HasContent()) { - return values; - // TODO: Deal with it not being routable, perhaps this should be an enum result? + values["controller"] = ControllerExtensions.GetControllerName(); + values["action"] = nameof(RenderNoContentController.Index); + + return await Task.FromResult(values); } + RouteRequest(_umbracoContextAccessor.UmbracoContext, out IPublishedRequest publishedRequest); + UmbracoRouteValues routeDef = GetUmbracoRouteDefinition(httpContext, values, publishedRequest); + values["controller"] = routeDef.ControllerName; if (string.IsNullOrWhiteSpace(routeDef.ActionName) == false) { @@ -134,7 +139,6 @@ namespace Umbraco.Web.Website.Routing var defaultControllerName = ControllerExtensions.GetControllerName(defaultControllerType); string customActionName = null; - var customControllerName = request.PublishedContent.ContentType.Alias; // never null // check that a template is defined), if it doesn't and there is a hijacked route it will just route // to the index Action @@ -143,17 +147,31 @@ namespace Umbraco.Web.Website.Routing // the template Alias should always be already saved with a safe name. // if there are hyphens in the name and there is a hijacked route, then the Action will need to be attributed // with the action name attribute. - customActionName = request.GetTemplateAlias().Split('.')[0].ToSafeAlias(_shortStringHelper); + customActionName = request.GetTemplateAlias()?.Split('.')[0].ToSafeAlias(_shortStringHelper); } // creates the default route definition which maps to the 'UmbracoController' controller var def = new UmbracoRouteValues( - request.PublishedContent, + request, defaultControllerName, defaultControllerType, templateName: customActionName); - IReadOnlyList candidates = FindControllerCandidates(customControllerName, customActionName, def.ActionName); + var customControllerName = request.PublishedContent?.ContentType.Alias; + if (customControllerName != null) + { + def = DetermineHijackedRoute(def, customControllerName, customActionName, request); + } + + // store the route definition + values.TryAdd(Constants.Web.UmbracoRouteDefinitionDataToken, def); + + return def; + } + + private UmbracoRouteValues DetermineHijackedRoute(UmbracoRouteValues routeValues, string customControllerName, string customActionName, IPublishedRequest request) + { + IReadOnlyList candidates = FindControllerCandidates(customControllerName, customActionName, routeValues.ActionName); // check if there's a custom controller assigned, base on the document type alias. var customControllerCandidates = candidates.Where(x => x.ControllerName.InvariantEquals(customControllerName)).ToList(); @@ -170,11 +188,12 @@ namespace Umbraco.Web.Website.Routing // now check if the custom action matches var customActionExists = customActionName != null && customControllerCandidates.Any(x => x.ActionName.InvariantEquals(customActionName)); - def = new UmbracoRouteValues( - request.PublishedContent, + // it's a hijacked route with a custom controller, so return the the values + return new UmbracoRouteValues( + request, controllerDescriptor.ControllerName, controllerDescriptor.ControllerTypeInfo, - customActionExists ? customActionName : def.ActionName, + customActionExists ? customActionName : routeValues.ActionName, customActionName, true); // Hijacked = true } @@ -192,10 +211,7 @@ namespace Umbraco.Web.Website.Routing } } - // store the route definition - values.TryAdd(Constants.Web.UmbracoRouteDefinitionDataToken, def); - - return def; + return routeValues; } /// @@ -228,7 +244,7 @@ namespace Umbraco.Web.Website.Routing // Maybe could be a one-time Set method instead? publishedRequest = umbracoContext.PublishedRequest = _publishedRouter.RouteRequest(requestBuilder); - return publishedRequest.Success() && publishedRequest.HasPublishedContent(); + return publishedRequest.Success(); // // HandleHttpResponseStatus returns a value indicating that the request should // // not be processed any further, eg because it has been redirect. then, exit. diff --git a/src/Umbraco.Web/Mvc/RenderRouteHandler.cs b/src/Umbraco.Web/Mvc/RenderRouteHandler.cs index d690fb579b..f4520d2af9 100644 --- a/src/Umbraco.Web/Mvc/RenderRouteHandler.cs +++ b/src/Umbraco.Web/Mvc/RenderRouteHandler.cs @@ -284,19 +284,22 @@ namespace Umbraco.Web.Mvc // missing template, so we're in a 404 here // so the content, if any, is a custom 404 page of some sort - if (request.HasPublishedContent() == false) - { - // means the builder could not find a proper document to handle 404 - return new PublishedContentNotFoundHandler(); - } - if (request.HasTemplate() == false) - { - // means the engine could find a proper document, but the document has no template - // at that point there isn't much we can do and there is no point returning - // to Mvc since Mvc can't do much - return new PublishedContentNotFoundHandler("In addition, no template exists to render the custom 404."); - } + // TODO: Handle this differently in netcore.... + + //if (request.HasPublishedContent() == false) + //{ + // // means the builder could not find a proper document to handle 404 + // return new PublishedContentNotFoundHandler(); + //} + + //if (request.HasTemplate() == false) + //{ + // // means the engine could find a proper document, but the document has no template + // // at that point there isn't much we can do and there is no point returning + // // to Mvc since Mvc can't do much + // return new PublishedContentNotFoundHandler("In addition, no template exists to render the custom 404."); + //} return null; } @@ -318,6 +321,7 @@ namespace Umbraco.Web.Mvc return HandlePostedValues(requestContext, postedInfo); } + // TODO: Surely this check is part of the PublishedRouter? // Here we need to check if there is no hijacked route and no template assigned, // if this is the case we want to return a blank page, but we'll leave that up to the NoTemplateHandler. @@ -326,13 +330,14 @@ namespace Umbraco.Web.Mvc if (request.HasTemplate() == false && Features.Disabled.DisableTemplates == false && routeDef.HasHijackedRoute == false) { - // TODO: Handle this differently + // TODO: Handle this differently in netcore.... + // request.UpdateToNotFound(); // request will go 404 // HandleHttpResponseStatus returns a value indicating that the request should // not be processed any further, eg because it has been redirect. then, exit. - if (UmbracoModule.HandleHttpResponseStatus(requestContext.HttpContext, request, Current.Logger)) - return null; + //if (UmbracoModule.HandleHttpResponseStatus(requestContext.HttpContext, request, Current.Logger)) + // return null; var handler = GetHandlerOnMissingTemplate(request); diff --git a/src/Umbraco.Web/Routing/PublishedContentNotFoundHandler.cs b/src/Umbraco.Web/Routing/PublishedContentNotFoundHandler.cs deleted file mode 100644 index 28bae7bced..0000000000 --- a/src/Umbraco.Web/Routing/PublishedContentNotFoundHandler.cs +++ /dev/null @@ -1,52 +0,0 @@ -using System.Web; -using Umbraco.Web.Composing; - -namespace Umbraco.Web.Routing -{ - /// - /// Gets executed when no document can be found in Umbraco - /// - internal class PublishedContentNotFoundHandler : IHttpHandler - { - private readonly string _message; - - public PublishedContentNotFoundHandler() - { } - - public PublishedContentNotFoundHandler(string message) - { - _message = message; - } - - public void ProcessRequest(HttpContext context) - { - WriteOutput(context); - } - - internal void WriteOutput(HttpContext context) - { - var response = context.Response; - - response.Clear(); - - var frequest = Current.UmbracoContext.PublishedRequest; - var reason = "Cannot render the page at URL '{0}'."; - if (frequest.HasPublishedContent() == false) - reason = "No umbraco document matches the URL '{0}'."; - else if (frequest.HasTemplate() == false) - reason = "No template exists to render the document at URL '{0}'."; - - response.Write("

Page not found

"); - response.Write("

"); - response.Write(string.Format(reason, HttpUtility.HtmlEncode(Current.UmbracoContext.OriginalRequestUrl.PathAndQuery))); - response.Write("

"); - if (string.IsNullOrWhiteSpace(_message) == false) - response.Write("

" + _message + "

"); - response.Write("

This page can be replaced with a custom 404. Check the documentation for \"custom 404\".

"); - response.Write("

This page is intentionally left ugly ;-)

"); - response.Write(""); - } - - public bool IsReusable => false; - } -} diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index b37b766a7d..c9ea1b1198 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -233,7 +233,6 @@ - diff --git a/src/Umbraco.Web/UmbracoInjectedModule.cs b/src/Umbraco.Web/UmbracoInjectedModule.cs index d0c2fd5de7..308e7dd48e 100644 --- a/src/Umbraco.Web/UmbracoInjectedModule.cs +++ b/src/Umbraco.Web/UmbracoInjectedModule.cs @@ -138,15 +138,15 @@ namespace Umbraco.Web var requestBuilder = _publishedRouter.CreateRequest(umbracoContext.CleanedUmbracoUrl); var request = umbracoContext.PublishedRequest = _publishedRouter.RouteRequest(requestBuilder); + // NOTE: This has been ported to netcore // HandleHttpResponseStatus returns a value indicating that the request should // not be processed any further, eg because it has been redirect. then, exit. - if (UmbracoModule.HandleHttpResponseStatus(httpContext, request, _logger)) - return; - - if (request.HasPublishedContent() == false) - httpContext.RemapHandler(new PublishedContentNotFoundHandler()); - else - RewriteToUmbracoHandler(httpContext, request); + //if (UmbracoModule.HandleHttpResponseStatus(httpContext, request, _logger)) + // return; + //if (request.HasPublishedContent() == false) + // httpContext.RemapHandler(new PublishedContentNotFoundHandler()); + //else + // RewriteToUmbracoHandler(httpContext, request); } #endregion @@ -257,40 +257,6 @@ namespace Umbraco.Web urlRouting.PostResolveRequestCache(context); } - /// - /// Rewrites to the Umbraco handler - we always send the request via our MVC rendering engine, this will deal with - /// requests destined for webforms. - /// - /// - /// - private void RewriteToUmbracoHandler(HttpContextBase context, IPublishedRequest pcr) - { - // NOTE: we do not want to use TransferRequest even though many docs say it is better with IIS7, turns out this is - // not what we need. The purpose of TransferRequest is to ensure that .net processes all of the rules for the newly - // rewritten URL, but this is not what we want! - // read: http://forums.iis.net/t/1146511.aspx - - var query = pcr.Uri.Query.TrimStart('?'); - - // GlobalSettings.Path has already been through IOHelper.ResolveUrl() so it begins with / and vdir (if any) - var rewritePath = _globalSettings.GetBackOfficePath(_hostingEnvironment).TrimEnd('/') + "/RenderMvc"; - // rewrite the path to the path of the handler (i.e. /umbraco/RenderMvc) - context.RewritePath(rewritePath, "", query, false); - - //if it is MVC we need to do something special, we are not using TransferRequest as this will - //require us to rewrite the path with query strings and then re-parse the query strings, this would - //also mean that we need to handle IIS 7 vs pre-IIS 7 differently. Instead we are just going to create - //an instance of the UrlRoutingModule and call it's PostResolveRequestCache method. This does: - // * Looks up the route based on the new rewritten URL - // * Creates the RequestContext with all route parameters and then executes the correct handler that matches the route - //we also cannot re-create this functionality because the setter for the HttpContext.Request.RequestContext is internal - //so really, this is pretty much the only way without using Server.TransferRequest and if we did that, we'd have to rethink - //a bunch of things! - var urlRouting = new UrlRoutingModule(); - urlRouting.PostResolveRequestCache(context); - } - - #endregion diff --git a/src/Umbraco.Web/UmbracoModule.cs b/src/Umbraco.Web/UmbracoModule.cs index b2b3d4c5a8..2f9c6d518a 100644 --- a/src/Umbraco.Web/UmbracoModule.cs +++ b/src/Umbraco.Web/UmbracoModule.cs @@ -41,65 +41,5 @@ namespace Umbraco.Web { EndRequest?.Invoke(sender, args); } - - // returns a value indicating whether redirection took place and the request has - // been completed - because we don't want to Response.End() here to terminate - // everything properly. - internal static bool HandleHttpResponseStatus(HttpContextBase context, IPublishedRequest pcr, ILogger logger) - { - var end = false; - var response = context.Response; - - logger.LogDebug("Response status: Redirect={Redirect}, Is404={Is404}, StatusCode={ResponseStatusCode}", - pcr.IsRedirect() ? (pcr.IsRedirectPermanent() ? "permanent" : "redirect") : "none", - pcr.Is404() ? "true" : "false", - pcr.ResponseStatusCode); - - if(pcr.CacheabilityNoCache) - response.Cache.SetCacheability(System.Web.HttpCacheability.NoCache); - - foreach (var cacheExtension in pcr.CacheExtensions) - response.Cache.AppendCacheExtension(cacheExtension); - - foreach (var header in pcr.Headers) - response.AppendHeader(header.Key, header.Value); - - if (pcr.IsRedirect()) - { - if (pcr.IsRedirectPermanent()) - response.RedirectPermanent(pcr.RedirectUrl, false); // do not end response - else - response.Redirect(pcr.RedirectUrl, false); // do not end response - end = true; - } - else if (pcr.Is404()) - { - response.StatusCode = 404; - response.TrySkipIisCustomErrors = /*Current.Configs.WebRouting().TrySkipIisCustomErrors; TODO introduce from config*/ false; - - if (response.TrySkipIisCustomErrors == false) - logger.LogWarning("Status code is 404 yet TrySkipIisCustomErrors is false - IIS will take over."); - } - - if (pcr.ResponseStatusCode > 0) - { - // set status code -- even for redirects - response.StatusCode = pcr.ResponseStatusCode; - response.StatusDescription = pcr.ResponseStatusDescription; - } - //if (pcr.IsRedirect) - // response.End(); // end response -- kills the thread and does not return! - - if (pcr.IsRedirect() == false) return end; - - response.Flush(); - // bypass everything and directly execute EndRequest event -- but returns - context.ApplicationInstance.CompleteRequest(); - // though some say that .CompleteRequest() does not properly shutdown the response - // and the request will hang until the whole code has run... would need to test? - logger.LogDebug("Response status: redirecting, complete request now."); - - return end; - } } }