From fa2e9c3715bf7de7d12c6d0cf071025aea9f5642 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 3 Apr 2020 09:13:55 +1100 Subject: [PATCH] Fixing AspNetCoreHostingEnvironment, cleaning up more IOHelper, adds notes --- src/Umbraco.Core/IO/IIOHelper.cs | 21 +++++++++-------- src/Umbraco.Core/IO/IOHelper.cs | 20 ++++++---------- .../Manifest/ManifestContentAppFactory.cs | 6 ++--- .../Manifest/DataEditorConverter.cs | 2 +- .../Manifest/ManifestParser.cs | 12 +++++----- .../ConfigurationEditorOfTConfiguration.cs | 2 +- .../AspNetCoreHostingEnvironment.cs | 23 +++++++++---------- .../AspNet/AspNetHostingEnvironment.cs | 2 +- 8 files changed, 41 insertions(+), 47 deletions(-) diff --git a/src/Umbraco.Core/IO/IIOHelper.cs b/src/Umbraco.Core/IO/IIOHelper.cs index b6fd100133..dccd394817 100644 --- a/src/Umbraco.Core/IO/IIOHelper.cs +++ b/src/Umbraco.Core/IO/IIOHelper.cs @@ -9,11 +9,18 @@ namespace Umbraco.Core.IO char DirSepChar { get; } string FindFile(string virtualPath); - string ResolveVirtualUrl(string path); + + // TODO: This is the same as IHostingEnvironment.ToAbsolute string ResolveUrl(string virtualPath); - Attempt TryResolveUrl(string virtualPath); - string MapPath(string path); + Attempt TryResolveUrl(string virtualPath); + + /// + /// Maps a virtual path to a physical path in the content root folder (i.e. www) + /// + /// + /// + string MapPath(string path); /// /// Verifies that the current filepath matches a directory where the user is allowed to edit a file. @@ -50,13 +57,7 @@ namespace Umbraco.Core.IO /// string GetRelativePath(string path); - /// - /// Gets the root path of the application - /// - /// - /// This is (should be) the same as IHostingEnvironment.ApplicationVirtualPath - /// - /// + // TODO: This is the same as IHostingEnvironment.ApplicationVirtualPath i don't think we want this string Root { get; diff --git a/src/Umbraco.Core/IO/IOHelper.cs b/src/Umbraco.Core/IO/IOHelper.cs index 33deabf1e8..a33195b45e 100644 --- a/src/Umbraco.Core/IO/IOHelper.cs +++ b/src/Umbraco.Core/IO/IOHelper.cs @@ -52,21 +52,13 @@ namespace Umbraco.Core.IO return retval; } - public string ResolveVirtualUrl(string path) - { - if (string.IsNullOrWhiteSpace(path)) return path; - return path.StartsWith("~/") ? ResolveUrl(path) : path; - } - - //Replaces tildes with the root dir + // TODO: This is the same as IHostingEnvironment.ToAbsolute public string ResolveUrl(string virtualPath) { - if (virtualPath.StartsWith("~")) - return virtualPath.Replace("~", Root).Replace("//", "/"); - else if (Uri.IsWellFormedUriString(virtualPath, UriKind.Absolute)) - return virtualPath; - else - return _hostingEnvironment.ToAbsolute(virtualPath, Root); + if (string.IsNullOrWhiteSpace(virtualPath)) return virtualPath; + // TODO: This is a bit odd, the whole "Root" thing is strange, we're passing this into IHostingEnvironment, but it already should know this value in it's ApplicationVirtualPath + return _hostingEnvironment.ToAbsolute(virtualPath, Root); + } public Attempt TryResolveUrl(string virtualPath) @@ -77,6 +69,8 @@ namespace Umbraco.Core.IO return Attempt.Succeed(virtualPath.Replace("~", Root).Replace("//", "/")); if (Uri.IsWellFormedUriString(virtualPath, UriKind.Absolute)) return Attempt.Succeed(virtualPath); + + // TODO: This is a bit odd, the whole "Root" thing is strange, we're passing this into IHostingEnvironment, but it already should know this value in it's ApplicationVirtualPath return Attempt.Succeed(_hostingEnvironment.ToAbsolute(virtualPath, Root)); } catch (Exception ex) diff --git a/src/Umbraco.Core/Manifest/ManifestContentAppFactory.cs b/src/Umbraco.Core/Manifest/ManifestContentAppFactory.cs index 903efe2897..fcd36d8a7e 100644 --- a/src/Umbraco.Core/Manifest/ManifestContentAppFactory.cs +++ b/src/Umbraco.Core/Manifest/ManifestContentAppFactory.cs @@ -136,14 +136,14 @@ namespace Umbraco.Core.Manifest // else // content app can be displayed - return _app ?? (_app = new ContentApp + return _app ??= new ContentApp { Alias = _definition.Alias, Name = _definition.Name, Icon = _definition.Icon, - View = _ioHelper.ResolveVirtualUrl(_definition.View), + View = _ioHelper.ResolveUrl(_definition.View), Weight = _definition.Weight - }); + }; } private class ShowRule diff --git a/src/Umbraco.Infrastructure/Manifest/DataEditorConverter.cs b/src/Umbraco.Infrastructure/Manifest/DataEditorConverter.cs index aac05def9f..7b0e85d8dc 100644 --- a/src/Umbraco.Infrastructure/Manifest/DataEditorConverter.cs +++ b/src/Umbraco.Infrastructure/Manifest/DataEditorConverter.cs @@ -140,7 +140,7 @@ namespace Umbraco.Core.Manifest private string RewriteVirtualUrl(JValue view) { - return _ioHelper.ResolveVirtualUrl(view.Value as string); + return _ioHelper.ResolveUrl(view.Value as string); } private void PrepareForParameterEditor(JObject jobject, DataEditor target) diff --git a/src/Umbraco.Infrastructure/Manifest/ManifestParser.cs b/src/Umbraco.Infrastructure/Manifest/ManifestParser.cs index 987d3a98fb..a2bfd340e5 100644 --- a/src/Umbraco.Infrastructure/Manifest/ManifestParser.cs +++ b/src/Umbraco.Infrastructure/Manifest/ManifestParser.cs @@ -195,21 +195,21 @@ namespace Umbraco.Core.Manifest // scripts and stylesheets are raw string, must process here for (var i = 0; i < manifest.Scripts.Length; i++) - manifest.Scripts[i] = _ioHelper.ResolveVirtualUrl(manifest.Scripts[i]); + manifest.Scripts[i] = _ioHelper.ResolveUrl(manifest.Scripts[i]); for (var i = 0; i < manifest.Stylesheets.Length; i++) - manifest.Stylesheets[i] = _ioHelper.ResolveVirtualUrl(manifest.Stylesheets[i]); + manifest.Stylesheets[i] = _ioHelper.ResolveUrl(manifest.Stylesheets[i]); foreach (var contentApp in manifest.ContentApps) { - contentApp.View = _ioHelper.ResolveVirtualUrl(contentApp.View); + contentApp.View = _ioHelper.ResolveUrl(contentApp.View); } foreach (var dashboard in manifest.Dashboards) { - dashboard.View = _ioHelper.ResolveVirtualUrl(dashboard.View); + dashboard.View = _ioHelper.ResolveUrl(dashboard.View); } foreach (var gridEditor in manifest.GridEditors) { - gridEditor.View = _ioHelper.ResolveVirtualUrl(gridEditor.View); - gridEditor.Render = _ioHelper.ResolveVirtualUrl(gridEditor.Render); + gridEditor.View = _ioHelper.ResolveUrl(gridEditor.View); + gridEditor.Render = _ioHelper.ResolveUrl(gridEditor.Render); } // add property editors that are also parameter editors, to the parameter editors list diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ConfigurationEditorOfTConfiguration.cs b/src/Umbraco.Infrastructure/PropertyEditors/ConfigurationEditorOfTConfiguration.cs index 0e56e0030e..39cd91599b 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ConfigurationEditorOfTConfiguration.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ConfigurationEditorOfTConfiguration.cs @@ -36,7 +36,7 @@ namespace Umbraco.Core.PropertyEditors ConfigurationField field; - var attributeView = ioHelper.ResolveVirtualUrl(attribute.View); + var attributeView = ioHelper.ResolveUrl(attribute.View); // if the field does not have its own type, use the base type if (attribute.Type == null) { diff --git a/src/Umbraco.Web.Common/AspNetCore/AspNetCoreHostingEnvironment.cs b/src/Umbraco.Web.Common/AspNetCore/AspNetCoreHostingEnvironment.cs index 42c6323d0e..d0c703ca87 100644 --- a/src/Umbraco.Web.Common/AspNetCore/AspNetCoreHostingEnvironment.cs +++ b/src/Umbraco.Web.Common/AspNetCore/AspNetCoreHostingEnvironment.cs @@ -14,15 +14,13 @@ namespace Umbraco.Web.Common.AspNetCore private readonly IHostingSettings _hostingSettings; private readonly IWebHostEnvironment _webHostEnvironment; - private readonly IHttpContextAccessor _httpContextAccessor; private string _localTempPath; - public AspNetCoreHostingEnvironment(IHostingSettings hostingSettings, IWebHostEnvironment webHostEnvironment, IHttpContextAccessor httpContextAccessor) + public AspNetCoreHostingEnvironment(IHostingSettings hostingSettings, IWebHostEnvironment webHostEnvironment) { _hostingSettings = hostingSettings ?? throw new ArgumentNullException(nameof(hostingSettings)); _webHostEnvironment = webHostEnvironment ?? throw new ArgumentNullException(nameof(webHostEnvironment)); - _httpContextAccessor = httpContextAccessor ?? throw new ArgumentNullException(nameof(httpContextAccessor)); SiteName = webHostEnvironment.ApplicationName; ApplicationId = AppDomain.CurrentDomain.Id.ToString(); @@ -88,21 +86,22 @@ namespace Umbraco.Web.Common.AspNetCore return Path.Combine(_webHostEnvironment.WebRootPath, newPath); } - // TODO: Need to take into account 'root' here + // TODO: Need to take into account 'root' here, maybe not, Root probably shouldn't be a param, see notes in IOHelper that calls this, we already know ApplicationVirtualPath public string ToAbsolute(string virtualPath, string root) { - if (Uri.TryCreate(virtualPath, UriKind.Absolute, out _)) - { + if (!virtualPath.StartsWith("~/") && !virtualPath.StartsWith("/")) + throw new InvalidOperationException($"{nameof(virtualPath)} must start with ~/ or /"); + if (!root.StartsWith("/")) + throw new InvalidOperationException($"{nameof(virtualPath)} must start with /"); + + // will occur if it starts with "/" + if (Uri.IsWellFormedUriString(virtualPath, UriKind.Absolute)) return virtualPath; - } - var segment = new PathString(virtualPath.Substring(1)); - var applicationPath = _httpContextAccessor.HttpContext.Request.PathBase; + var fullPath = root.EnsureEndsWith('/') + virtualPath.TrimStart("~/"); - return applicationPath.Add(segment).Value; + return fullPath; } - - } diff --git a/src/Umbraco.Web/AspNet/AspNetHostingEnvironment.cs b/src/Umbraco.Web/AspNet/AspNetHostingEnvironment.cs index 0ae177517a..06a61c6fe9 100644 --- a/src/Umbraco.Web/AspNet/AspNetHostingEnvironment.cs +++ b/src/Umbraco.Web/AspNet/AspNetHostingEnvironment.cs @@ -41,7 +41,7 @@ namespace Umbraco.Web.Hosting return HostingEnvironment.MapPath(path); } - public string ToAbsolute(string virtualPath, string root) => VirtualPathUtility.ToAbsolute(virtualPath, root); + public string ToAbsolute(string virtualPath, string root) => VirtualPathUtility.ToAbsolute(virtualPath, root.EnsureStartsWith('/')); public string LocalTempPath