AB4079 - Review fixes

This commit is contained in:
Bjarke Berg
2019-12-06 09:49:10 +01:00
parent 5c6dcc900e
commit 82bb1f00fc
15 changed files with 78 additions and 95 deletions

View File

@@ -1,14 +1,5 @@
namespace Umbraco.Core.Configuration
{
public interface IHostingSettings
{
bool DebugMode { get; }
/// <summary>
/// Gets the configuration for the location of temporary files.
/// </summary>
LocalTempStorage LocalTempStorageLocation { get; }
}
/// <summary>
/// Contains general settings information for the entire Umbraco instance based on information from web.config appsettings
/// </summary>

View File

@@ -0,0 +1,11 @@
namespace Umbraco.Core.Configuration
{
public interface IHostingSettings
{
bool DebugMode { get; }
/// <summary>
/// Gets the configuration for the location of temporary files.
/// </summary>
LocalTempStorage LocalTempStorageLocation { get; }
}
}

View File

@@ -11,7 +11,6 @@ namespace Umbraco.Core.IO
string ResolveVirtualUrl(string path);
string ResolveUrl(string virtualPath);
Attempt<string> TryResolveUrl(string virtualPath);
string MapPath(string path, bool useHttpContext);
string MapPath(string path);

View File

@@ -5,6 +5,7 @@ using System.Reflection;
using System.IO;
using System.Linq;
using Umbraco.Core.Hosting;
using Umbraco.Core.Strings;
namespace Umbraco.Core.IO
{
@@ -78,10 +79,9 @@ namespace Umbraco.Core.IO
}
}
public string MapPath(string path, bool useHttpContext)
public string MapPath(string path)
{
if (path == null) throw new ArgumentNullException("path");
useHttpContext = useHttpContext && _hostingEnvironment.IsHosted;
if (path == null) throw new ArgumentNullException(nameof(path));
// Check if the path is already mapped
if ((path.Length >= 2 && path[1] == Path.VolumeSeparatorChar)
@@ -93,7 +93,7 @@ namespace Umbraco.Core.IO
// http://umbraco.codeplex.com/workitem/30946
if (useHttpContext && _hostingEnvironment.IsHosted)
if (_hostingEnvironment.IsHosted)
{
var result = (String.IsNullOrEmpty(path) == false && (path.StartsWith("~") || path.StartsWith(Root)))
? _hostingEnvironment.MapPath(path)
@@ -113,10 +113,6 @@ namespace Umbraco.Core.IO
return retval;
}
public string MapPath(string path)
{
return MapPath(path, true);
}
/// <summary>
/// Verifies that the current filepath matches a directory where the user is allowed to edit a file.
@@ -276,7 +272,7 @@ namespace Umbraco.Core.IO
path = relativePath;
}
return path.EnsurePathIsApplicationRootPrefixed();
return PathUtility.EnsurePathIsApplicationRootPrefixed(path);
}
private string _root;

View File

@@ -1215,21 +1215,6 @@ namespace Umbraco.Core
public static string NullOrWhiteSpaceAsNull(this string text)
=> string.IsNullOrWhiteSpace(text) ? null : text;
/// <summary>
/// Ensures that a path has `~/` as prefix
/// </summary>
/// <param name="path"></param>
/// <returns></returns>
public static string EnsurePathIsApplicationRootPrefixed(this string path)
{
if (path.StartsWith("~/"))
return path;
if (path.StartsWith("/") == false && path.StartsWith("\\") == false)
path = string.Format("/{0}", path);
if (path.StartsWith("~") == false)
path = string.Format("~{0}", path);
return path;
}
/// <summary>
/// Checks if a given path is a full path including drive letter

View File

@@ -0,0 +1,22 @@
namespace Umbraco.Core.Strings
{
public static class PathUtility
{
/// <summary>
/// Ensures that a path has `~/` as prefix
/// </summary>
/// <param name="path"></param>
/// <returns></returns>
public static string EnsurePathIsApplicationRootPrefixed(string path)
{
if (path.StartsWith("~/"))
return path;
if (path.StartsWith("/") == false && path.StartsWith("\\") == false)
path = string.Format("/{0}", path);
if (path.StartsWith("~") == false)
path = string.Format("~{0}", path);
return path;
}
}
}

View File

@@ -8,6 +8,8 @@ namespace Umbraco.Core.Configuration
{
public class HostingSettings : IHostingSettings
{
private bool? _debugMode;
/// <inheritdoc />
public LocalTempStorage LocalTempStorageLocation
{
@@ -29,21 +31,25 @@ namespace Umbraco.Core.Configuration
{
get
{
try
if (!_debugMode.HasValue)
{
if (ConfigurationManager.GetSection("system.web/compilation") is ConfigurationSection compilation)
try
{
var debugElement = compilation.ElementInformation.Properties["debug"];
if (ConfigurationManager.GetSection("system.web/compilation") is ConfigurationSection compilation)
{
var debugElement = compilation.ElementInformation.Properties["debug"];
return debugElement != null && (debugElement.Value is bool debug && debug);
_debugMode = debugElement != null && (debugElement.Value is bool debug && debug);
}
}
catch
{
_debugMode = false;
}
}
catch
{
// ignored
}
return false;
return _debugMode.GetValueOrDefault();
}
}
}

View File

@@ -13,11 +13,11 @@ namespace Umbraco.Core.Logging.Serilog.Enrichers
/// </summary>
internal class HttpRequestIdEnricher : ILogEventEnricher
{
private readonly Func<IFactory> _factoryGetter;
private readonly Func<IRequestCache> _requestCacheGetter;
public HttpRequestIdEnricher(Func<IFactory> factoryGetter)
public HttpRequestIdEnricher(Func<IRequestCache> requestCacheGetter)
{
_factoryGetter = factoryGetter;
_requestCacheGetter = requestCacheGetter;
}
/// <summary>
@@ -32,12 +32,10 @@ namespace Umbraco.Core.Logging.Serilog.Enrichers
/// <param name="propertyFactory">Factory for creating new properties to add to the event.</param>
public void Enrich(LogEvent logEvent, ILogEventPropertyFactory propertyFactory)
{
if (logEvent == null) throw new ArgumentNullException("logEvent");
if (logEvent == null) throw new ArgumentNullException(nameof(logEvent));
var factory = _factoryGetter();
if(factory is null) return;
var requestCache = factory.GetInstance<IRequestCache>();
var requestCache = _requestCacheGetter();
if(requestCache is null) return;
Guid requestId;
if (!LogHttpRequest.TryGetCurrentHttpRequestId(out requestId, requestCache))

View File

@@ -15,7 +15,7 @@ namespace Umbraco.Core.Logging.Serilog.Enrichers
/// </summary>
internal class HttpRequestNumberEnricher : ILogEventEnricher
{
private readonly Func<IFactory> _factoryFunc;
private readonly Func<IRequestCache> _requestCacheGetter;
private static int _lastRequestNumber;
private static readonly string _requestNumberItemName = typeof(HttpRequestNumberEnricher).Name + "+RequestNumber";
@@ -25,9 +25,9 @@ namespace Umbraco.Core.Logging.Serilog.Enrichers
private const string _httpRequestNumberPropertyName = "HttpRequestNumber";
public HttpRequestNumberEnricher(Func<IFactory> factoryFunc)
public HttpRequestNumberEnricher(Func<IRequestCache> requestCacheGetter)
{
_factoryFunc = factoryFunc;
_requestCacheGetter = requestCacheGetter;
}
/// <summary>
@@ -39,10 +39,9 @@ namespace Umbraco.Core.Logging.Serilog.Enrichers
{
if (logEvent == null) throw new ArgumentNullException(nameof(logEvent));
var factory = _factoryFunc();
if (factory is null) return;
var requestCache = _requestCacheGetter();
if (requestCache is null) return;
var requestCache = factory.GetInstance<IRequestCache>();
var requestNumber = requestCache.Get(_requestNumberItemName,
() => Interlocked.Increment(ref _lastRequestNumber));

View File

@@ -25,7 +25,7 @@ namespace Umbraco.Core.Logging.Serilog
/// </summary>
/// <param name="logConfig">A Serilog LoggerConfiguration</param>
/// <param name="hostingEnvironment"></param>
public static LoggerConfiguration MinimalConfiguration(this LoggerConfiguration logConfig, IHostingEnvironment hostingEnvironment, ISessionIdResolver sessionIdResolver, Func<IFactory> factoryFunc)
public static LoggerConfiguration MinimalConfiguration(this LoggerConfiguration logConfig, IHostingEnvironment hostingEnvironment, ISessionIdResolver sessionIdResolver, Func<IRequestCache> requestCacheGetter)
{
global::Serilog.Debugging.SelfLog.Enable(msg => System.Diagnostics.Debug.WriteLine(msg));
@@ -43,8 +43,8 @@ namespace Umbraco.Core.Logging.Serilog
.Enrich.WithProperty("MachineName", Environment.MachineName)
.Enrich.With<Log4NetLevelMapperEnricher>()
.Enrich.With(new HttpSessionIdEnricher(sessionIdResolver))
.Enrich.With(new HttpRequestNumberEnricher(factoryFunc))
.Enrich.With(new HttpRequestIdEnricher(factoryFunc));
.Enrich.With(new HttpRequestNumberEnricher(requestCacheGetter))
.Enrich.With(new HttpRequestIdEnricher(requestCacheGetter));
return logConfig;
}

View File

@@ -38,11 +38,11 @@ namespace Umbraco.Core.Logging.Serilog
/// Creates a logger with some pre-defined configuration and remainder from config file
/// </summary>
/// <remarks>Used by UmbracoApplicationBase to get its logger.</remarks>
public static SerilogLogger CreateWithDefaultConfiguration(IHostingEnvironment hostingEnvironment, ISessionIdResolver sessionIdResolver, Func<IFactory> factoryFunc)
public static SerilogLogger CreateWithDefaultConfiguration(IHostingEnvironment hostingEnvironment, ISessionIdResolver sessionIdResolver, Func<IRequestCache> requestCacheGetter)
{
var loggerConfig = new LoggerConfiguration();
loggerConfig
.MinimalConfiguration(hostingEnvironment, sessionIdResolver, factoryFunc)
.MinimalConfiguration(hostingEnvironment, sessionIdResolver, requestCacheGetter)
.ReadFromConfigFile()
.ReadFromUserConfigFile();

View File

@@ -4,6 +4,7 @@ using Umbraco.Core.IO;
using Umbraco.Core;
using Umbraco.Core.Composing;
using Umbraco.Core.Configuration;
using Umbraco.Core.Strings;
using Umbraco.Tests.TestHelpers;
namespace Umbraco.Tests.IO
@@ -30,35 +31,13 @@ namespace Umbraco.Tests.IO
}
}
/// <summary>
///A test for MapPath verifying that HttpContext method (which includes vdirs) matches non-HttpContext method
///</summary>
[Test]
public void IOHelper_MapPathTestVDirTraversal()
{
//System.Diagnostics.Debugger.Break();
var globalSettings = SettingsForTests.GenerateMockGlobalSettings();
Assert.AreEqual(_ioHelper.MapPath(Constants.SystemDirectories.Bin, true), _ioHelper.MapPath(Constants.SystemDirectories.Bin, false));
Assert.AreEqual(_ioHelper.MapPath(Constants.SystemDirectories.Config, true), _ioHelper.MapPath(Constants.SystemDirectories.Config, false));
Assert.AreEqual(_ioHelper.MapPath(globalSettings.UmbracoCssPath, true), _ioHelper.MapPath(globalSettings.UmbracoCssPath, false));
Assert.AreEqual(_ioHelper.MapPath(Constants.SystemDirectories.Data, true), _ioHelper.MapPath(Constants.SystemDirectories.Data, false));
Assert.AreEqual(_ioHelper.MapPath(Constants.SystemDirectories.Install, true), _ioHelper.MapPath(Constants.SystemDirectories.Install, false));
Assert.AreEqual(_ioHelper.MapPath(globalSettings.UmbracoMediaPath, true), _ioHelper.MapPath(globalSettings.UmbracoMediaPath, false));
Assert.AreEqual(_ioHelper.MapPath(Constants.SystemDirectories.Packages, true), _ioHelper.MapPath(Constants.SystemDirectories.Packages, false));
Assert.AreEqual(_ioHelper.MapPath(Constants.SystemDirectories.Preview, true), _ioHelper.MapPath(Constants.SystemDirectories.Preview, false));
Assert.AreEqual(_ioHelper.MapPath(_ioHelper.Root, true), _ioHelper.MapPath(_ioHelper.Root, false));
Assert.AreEqual(_ioHelper.MapPath(globalSettings.UmbracoScriptsPath, true), _ioHelper.MapPath(globalSettings.UmbracoScriptsPath, false));
Assert.AreEqual(_ioHelper.MapPath(globalSettings.UmbracoPath, true), _ioHelper.MapPath(globalSettings.UmbracoPath, false));
}
[Test]
public void EnsurePathIsApplicationRootPrefixed()
{
//Assert
Assert.AreEqual("~/Views/Template.cshtml", "Views/Template.cshtml".EnsurePathIsApplicationRootPrefixed());
Assert.AreEqual("~/Views/Template.cshtml", "/Views/Template.cshtml".EnsurePathIsApplicationRootPrefixed());
Assert.AreEqual("~/Views/Template.cshtml", "~/Views/Template.cshtml".EnsurePathIsApplicationRootPrefixed());
Assert.AreEqual("~/Views/Template.cshtml", PathUtility.EnsurePathIsApplicationRootPrefixed("Views/Template.cshtml"));
Assert.AreEqual("~/Views/Template.cshtml", PathUtility.EnsurePathIsApplicationRootPrefixed("/Views/Template.cshtml"));
Assert.AreEqual("~/Views/Template.cshtml", PathUtility.EnsurePathIsApplicationRootPrefixed("~/Views/Template.cshtml"));
}
}
}

View File

@@ -71,7 +71,7 @@ namespace Umbraco.Tests.Web.Controllers
}
else
{
var baseDir = IOHelper.MapPath("", false).TrimEnd(IOHelper.DirSepChar);
var baseDir = IOHelper.MapPath("").TrimEnd(IOHelper.DirSepChar);
HttpContext.Current = new HttpContext(new SimpleWorkerRequest("/", baseDir, "", "", new StringWriter()));
}
IOHelper.ForceNotHosted = true;

View File

@@ -5,10 +5,6 @@ using System.Web.Mvc;
using System.Web.WebPages;
using Umbraco.Core;
using Umbraco.Core.Cache;
using Umbraco.Core.Composing;
using Umbraco.Core.Configuration;
using Umbraco.Core.Dictionary;
using Umbraco.Core.IO;
using Umbraco.Core.Models.PublishedContent;
using Umbraco.Core.Services;
using Umbraco.Core.Strings;
@@ -238,9 +234,9 @@ namespace Umbraco.Web.Mvc
public override void Write(object value)
{
if (value is IHtmlEncodedString)
if (value is IHtmlEncodedString htmlEncodedString)
{
base.WriteLiteral(value);
base.WriteLiteral(htmlEncodedString.ToHtmlString());
}
else
{

View File

@@ -4,6 +4,7 @@ using System.Threading;
using System.Web;
using System.Web.Hosting;
using Umbraco.Core;
using Umbraco.Core.Cache;
using Umbraco.Core.Composing;
using Umbraco.Core.Configuration;
using Umbraco.Core.Hosting;
@@ -45,7 +46,7 @@ namespace Umbraco.Web
_profiler = new LogProfiler(_logger);
_logger = SerilogLogger.CreateWithDefaultConfiguration(_hostingEnvironment, new AspNetSessionIdResolver(), () => _factory);
_logger = SerilogLogger.CreateWithDefaultConfiguration(_hostingEnvironment, new AspNetSessionIdResolver(), () => _factory?.GetInstance<IRequestCache>());
_backOfficeInfo = new AspNetBackOfficeInfo(_configs.Global(), _ioHelper, _configs.Settings(), _logger);
}