diff --git a/src/Umbraco.Core/IO/PhysicalFileSystem.cs b/src/Umbraco.Core/IO/PhysicalFileSystem.cs index b801f05b8f..47daff932d 100644 --- a/src/Umbraco.Core/IO/PhysicalFileSystem.cs +++ b/src/Umbraco.Core/IO/PhysicalFileSystem.cs @@ -8,7 +8,12 @@ namespace Umbraco.Core.IO { public class PhysicalFileSystem : IFileSystem { + // the rooted, filesystem path, using directory separator chars, NOT ending with a separator + // eg "c:" or "c:\path\to\site" or "\\server\path" private readonly string _rootPath; + + // the ??? url, using url separator chars, NOT ending with a separator + // eg "" (?) or "/Scripts" or ??? private readonly string _rootUrl; public PhysicalFileSystem(string virtualRoot) @@ -22,6 +27,7 @@ namespace Umbraco.Core.IO _rootPath = _rootPath.TrimEnd(Path.DirectorySeparatorChar); _rootUrl = IOHelper.ResolveUrl(virtualRoot); + _rootUrl = EnsureUrlSeparatorChar(_rootUrl); _rootUrl = _rootUrl.TrimEnd('/'); } @@ -46,6 +52,9 @@ namespace Umbraco.Core.IO rootPath = Path.Combine(localRoot, rootPath); } + rootPath = EnsureDirectorySeparatorChar(rootPath); + rootUrl = EnsureUrlSeparatorChar(rootUrl); + _rootPath = rootPath.TrimEnd(Path.DirectorySeparatorChar); _rootUrl = rootUrl.TrimEnd('/'); } @@ -173,6 +182,8 @@ namespace Umbraco.Core.IO return File.Exists(fullpath); } + // beware, many things depend on how the GetRelative/AbsolutePath methods work! + /// /// Gets the relative path. /// @@ -180,33 +191,27 @@ namespace Umbraco.Core.IO /// The path, relative to this filesystem's root. /// /// The relative path is relative to this filesystem's root, not starting with any - /// directory separator. All separators are converted to Path.DirectorySeparatorChar. + /// directory separator. If input was recognized as a url (path), then output uses url (path) separator + /// chars. /// public string GetRelativePath(string fullPathOrUrl) { // test url - var path = EnsureUrlSeparatorChar(fullPathOrUrl); - if (IOHelper.PathStartsWith(path, _rootUrl, '/')) - return path.Substring(_rootUrl.Length) - .Replace('/', Path.DirectorySeparatorChar) - .TrimStart(Path.DirectorySeparatorChar); + var path = fullPathOrUrl.Replace('\\', '/'); // ensure url separator char + + if (IOHelper.PathStartsWith(path, _rootUrl, '/')) // if it starts with the root url... + return path.Substring(_rootUrl.Length) // strip it + .TrimStart('/'); // it's relative // test path path = EnsureDirectorySeparatorChar(fullPathOrUrl); - if (IOHelper.PathStartsWith(path, _rootPath, Path.DirectorySeparatorChar)) - return path.Substring(_rootPath.Length) - .TrimStart(Path.DirectorySeparatorChar); + if (IOHelper.PathStartsWith(path, _rootPath, Path.DirectorySeparatorChar)) // if it starts with the root path + return path.Substring(_rootPath.Length) // strip it + .TrimStart(Path.DirectorySeparatorChar); // it's relative + + // unchanged - including separators return fullPathOrUrl; - - // previous code kept for reference - - //var relativePath = fullPathOrUrl - // .TrimStart(_rootUrl) - // .Replace('/', Path.DirectorySeparatorChar) - // .TrimStart(RootPath) - // .TrimStart(Path.DirectorySeparatorChar); - //return relativePath; } /// diff --git a/src/Umbraco.Core/Models/File.cs b/src/Umbraco.Core/Models/File.cs index ce6121f46f..8ead6da5f8 100644 --- a/src/Umbraco.Core/Models/File.cs +++ b/src/Umbraco.Core/Models/File.cs @@ -27,7 +27,7 @@ namespace Umbraco.Core.Models protected File(string path, Func getFileContent = null) { - _path = path; + _path = SanitizePath(path); _originalPath = _path; GetFileContent = getFileContent; _content = getFileContent != null ? null : string.Empty; @@ -38,6 +38,14 @@ namespace Umbraco.Core.Models private string _alias; private string _name; + private static string SanitizePath(string path) + { + return path + .Replace('\\', System.IO.Path.DirectorySeparatorChar) + .Replace('/', System.IO.Path.DirectorySeparatorChar); + //.TrimStart(System.IO.Path.DirectorySeparatorChar); + } + /// /// Gets or sets the Name of the File including extension /// @@ -81,7 +89,7 @@ namespace Umbraco.Core.Models SetPropertyValueAndDetectChanges(o => { - _path = value; + _path = SanitizePath(value); return _path; }, _path, PathSelector); } diff --git a/src/Umbraco.Tests/Persistence/Repositories/PartialViewRepositoryTests.cs b/src/Umbraco.Tests/Persistence/Repositories/PartialViewRepositoryTests.cs new file mode 100644 index 0000000000..0402713cc9 --- /dev/null +++ b/src/Umbraco.Tests/Persistence/Repositories/PartialViewRepositoryTests.cs @@ -0,0 +1,119 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using NUnit.Framework; +using Umbraco.Core.IO; +using Umbraco.Core.Models; +using Umbraco.Core.Persistence.Repositories; +using Umbraco.Core.Persistence.UnitOfWork; +using Umbraco.Tests.TestHelpers; + +namespace Umbraco.Tests.Persistence.Repositories +{ + [TestFixture] + public class PartialViewRepositoryTests : BaseUmbracoApplicationTest + { + private IFileSystem _fileSystem; + + [SetUp] + public override void Initialize() + { + base.Initialize(); + + _fileSystem = new PhysicalFileSystem(SystemDirectories.MvcViews + "/Partials/"); + } + + [Test] + public void PathTests() + { + // unless noted otherwise, no changes / 7.2.8 + + var provider = new FileUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + var repository = new PartialViewRepository(unitOfWork, _fileSystem); + + var partialView = new PartialView("test-path-1.cshtml") { Content = "// partialView" }; + repository.AddOrUpdate(partialView); + unitOfWork.Commit(); + Assert.IsTrue(_fileSystem.FileExists("test-path-1.cshtml")); + Assert.AreEqual("test-path-1.cshtml", partialView.Path); + Assert.AreEqual("/Views/Partials/test-path-1.cshtml", partialView.VirtualPath); + + partialView = new PartialView("path-2/test-path-2.cshtml") { Content = "// partialView" }; + repository.AddOrUpdate(partialView); + unitOfWork.Commit(); + Assert.IsTrue(_fileSystem.FileExists("path-2/test-path-2.cshtml")); + Assert.AreEqual("path-2\\test-path-2.cshtml", partialView.Path); // fixed in 7.3 - 7.2.8 does not update the path + Assert.AreEqual("/Views/Partials/path-2/test-path-2.cshtml", partialView.VirtualPath); + + partialView = (PartialView) repository.Get("path-2/test-path-2.cshtml"); + Assert.IsNotNull(partialView); + Assert.AreEqual("path-2\\test-path-2.cshtml", partialView.Path); + Assert.AreEqual("/Views/Partials/path-2/test-path-2.cshtml", partialView.VirtualPath); + + partialView = new PartialView("path-2\\test-path-3.cshtml") { Content = "// partialView" }; + repository.AddOrUpdate(partialView); + unitOfWork.Commit(); + Assert.IsTrue(_fileSystem.FileExists("path-2/test-path-3.cshtml")); + Assert.AreEqual("path-2\\test-path-3.cshtml", partialView.Path); + Assert.AreEqual("/Views/Partials/path-2/test-path-3.cshtml", partialView.VirtualPath); + + partialView = (PartialView) repository.Get("path-2/test-path-3.cshtml"); + Assert.IsNotNull(partialView); + Assert.AreEqual("path-2\\test-path-3.cshtml", partialView.Path); + Assert.AreEqual("/Views/Partials/path-2/test-path-3.cshtml", partialView.VirtualPath); + + partialView = (PartialView) repository.Get("path-2\\test-path-3.cshtml"); + Assert.IsNotNull(partialView); + Assert.AreEqual("path-2\\test-path-3.cshtml", partialView.Path); + Assert.AreEqual("/Views/Partials/path-2/test-path-3.cshtml", partialView.VirtualPath); + + partialView = new PartialView("\\test-path-4.cshtml") { Content = "// partialView" }; + Assert.Throws(() => // fixed in 7.3 - 7.2.8 used to strip the \ + { + repository.AddOrUpdate(partialView); + }); + + partialView = (PartialView) repository.Get("missing.cshtml"); + Assert.IsNull(partialView); + + // fixed in 7.3 - 7.2.8 used to... + Assert.Throws(() => + { + partialView = (PartialView) repository.Get("\\test-path-4.cshtml"); // outside the filesystem, does not exist + }); + Assert.Throws(() => + { + partialView = (PartialView) repository.Get("../../packages.config"); // outside the filesystem, exists + }); + } + + [TearDown] + public override void TearDown() + { + base.TearDown(); + + //Delete all files + Purge((PhysicalFileSystem)_fileSystem, ""); + _fileSystem = null; + } + + private void Purge(PhysicalFileSystem fs, string path) + { + var files = fs.GetFiles(path, "*.cshtml"); + foreach (var file in files) + { + fs.DeleteFile(file); + } + var dirs = fs.GetDirectories(path); + foreach (var dir in dirs) + { + Purge(fs, dir); + fs.DeleteDirectory(dir); + } + } + } +} diff --git a/src/Umbraco.Tests/Persistence/Repositories/ScriptRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ScriptRepositoryTest.cs index f2341c7643..1fdb2be415 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ScriptRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ScriptRepositoryTest.cs @@ -220,15 +220,79 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.AreEqual(content, script.Content); } + [Test] + public void PathTests() + { + // unless noted otherwise, no changes / 7.2.8 + + var provider = new FileUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + var repository = new ScriptRepository(unitOfWork, _fileSystem, Mock.Of()); + + var script = new Script("test-path-1.js") { Content = "// script" }; + repository.AddOrUpdate(script); + unitOfWork.Commit(); + Assert.IsTrue(_fileSystem.FileExists("test-path-1.js")); + Assert.AreEqual("test-path-1.js", script.Path); + Assert.AreEqual("/scripts/test-path-1.js", script.VirtualPath); + + script = new Script("path-2/test-path-2.js") { Content = "// script" }; + repository.AddOrUpdate(script); + unitOfWork.Commit(); + Assert.IsTrue(_fileSystem.FileExists("path-2/test-path-2.js")); + Assert.AreEqual("path-2\\test-path-2.js", script.Path); // fixed in 7.3 - 7.2.8 does not update the path + Assert.AreEqual("/scripts/path-2/test-path-2.js", script.VirtualPath); + + script = repository.Get("path-2/test-path-2.js"); + Assert.IsNotNull(script); + Assert.AreEqual("path-2\\test-path-2.js", script.Path); + Assert.AreEqual("/scripts/path-2/test-path-2.js", script.VirtualPath); + + script = new Script("path-2\\test-path-3.js") { Content = "// script" }; + repository.AddOrUpdate(script); + unitOfWork.Commit(); + Assert.IsTrue(_fileSystem.FileExists("path-2/test-path-3.js")); + Assert.AreEqual("path-2\\test-path-3.js", script.Path); + Assert.AreEqual("/scripts/path-2/test-path-3.js", script.VirtualPath); + + script = repository.Get("path-2/test-path-3.js"); + Assert.IsNotNull(script); + Assert.AreEqual("path-2\\test-path-3.js", script.Path); + Assert.AreEqual("/scripts/path-2/test-path-3.js", script.VirtualPath); + + script = repository.Get("path-2\\test-path-3.js"); + Assert.IsNotNull(script); + Assert.AreEqual("path-2\\test-path-3.js", script.Path); + Assert.AreEqual("/scripts/path-2/test-path-3.js", script.VirtualPath); + + script = new Script("\\test-path-4.js") { Content = "// script" }; + Assert.Throws(() => // fixed in 7.3 - 7.2.8 used to strip the \ + { + repository.AddOrUpdate(script); + }); + + script = repository.Get("missing.js"); + Assert.IsNull(script); + + // fixed in 7.3 - 7.2.8 used to... + Assert.Throws(() => + { + script = repository.Get("\\test-path-4.js"); // outside the filesystem, does not exist + }); + Assert.Throws(() => + { + script = repository.Get("../packages.config"); // outside the filesystem, exists + }); + } + [TearDown] public override void TearDown() { base.TearDown(); - _fileSystem = null; //Delete all files - var fs = new PhysicalFileSystem(SystemDirectories.Scripts); - Purge(fs, ""); + Purge((PhysicalFileSystem) _fileSystem, ""); + _fileSystem = null; } private void Purge(PhysicalFileSystem fs, string path) diff --git a/src/Umbraco.Tests/Persistence/Repositories/StylesheetRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/StylesheetRepositoryTest.cs index 87182e4ad0..f73b89ca99 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/StylesheetRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/StylesheetRepositoryTest.cs @@ -1,4 +1,5 @@ -using System.Data; +using System; +using System.Data; using System.IO; using System.Linq; using System.Text; @@ -241,18 +242,95 @@ p{font-size:2em;}")); Assert.That(exists, Is.True); } + [Test] + public void PathTests() + { + // unless noted otherwise, no changes / 7.2.8 + + var provider = new FileUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + var repository = new StylesheetRepository(unitOfWork, _fileSystem); + + var stylesheet = new Stylesheet("test-path-1.css") { Content = "body { color:#000; } .bold {font-weight:bold;}" }; + repository.AddOrUpdate(stylesheet); + unitOfWork.Commit(); + Assert.IsTrue(_fileSystem.FileExists("test-path-1.css")); + Assert.AreEqual("test-path-1.css", stylesheet.Path); + Assert.AreEqual("/css/test-path-1.css", stylesheet.VirtualPath); + + stylesheet = new Stylesheet("path-2/test-path-2.css") { Content = "body { color:#000; } .bold {font-weight:bold;}" }; + repository.AddOrUpdate(stylesheet); + unitOfWork.Commit(); + Assert.IsTrue(_fileSystem.FileExists("path-2/test-path-2.css")); + Assert.AreEqual("path-2\\test-path-2.css", stylesheet.Path); // fixed in 7.3 - 7.2.8 does not update the path + Assert.AreEqual("/css/path-2/test-path-2.css", stylesheet.VirtualPath); + + stylesheet = repository.Get("path-2/test-path-2.css"); + Assert.IsNotNull(stylesheet); + Assert.AreEqual("path-2\\test-path-2.css", stylesheet.Path); + Assert.AreEqual("/css/path-2/test-path-2.css", stylesheet.VirtualPath); + + stylesheet = new Stylesheet("path-2\\test-path-3.css") { Content = "body { color:#000; } .bold {font-weight:bold;}" }; + repository.AddOrUpdate(stylesheet); + unitOfWork.Commit(); + Assert.IsTrue(_fileSystem.FileExists("path-2/test-path-3.css")); + Assert.AreEqual("path-2\\test-path-3.css", stylesheet.Path); + Assert.AreEqual("/css/path-2/test-path-3.css", stylesheet.VirtualPath); + + stylesheet = repository.Get("path-2/test-path-3.css"); + Assert.IsNotNull(stylesheet); + Assert.AreEqual("path-2\\test-path-3.css", stylesheet.Path); + Assert.AreEqual("/css/path-2/test-path-3.css", stylesheet.VirtualPath); + + stylesheet = repository.Get("path-2\\test-path-3.css"); + Assert.IsNotNull(stylesheet); + Assert.AreEqual("path-2\\test-path-3.css", stylesheet.Path); + Assert.AreEqual("/css/path-2/test-path-3.css", stylesheet.VirtualPath); + + stylesheet = new Stylesheet("\\test-path-4.css") { Content = "body { color:#000; } .bold {font-weight:bold;}" }; + Assert.Throws(() => // fixed in 7.3 - 7.2.8 used to strip the \ + { + repository.AddOrUpdate(stylesheet); + }); + + // fixed in 7.3 - 7.2.8 used to throw + stylesheet = repository.Get("missing.css"); + Assert.IsNull(stylesheet); + + // fixed in 7.3 - 7.2.8 used to... + Assert.Throws(() => + { + stylesheet = repository.Get("\\test-path-4.css"); // outside the filesystem, does not exist + }); + Assert.Throws(() => + { + stylesheet = repository.Get("../packages.config"); // outside the filesystem, exists + }); + } + [TearDown] - public void TearDown() + public override void TearDown() { base.TearDown(); //Delete all files - var files = _fileSystem.GetFiles("", "*.css"); + Purge((PhysicalFileSystem) _fileSystem, ""); + _fileSystem = null; + } + + private void Purge(PhysicalFileSystem fs, string path) + { + var files = fs.GetFiles(path, "*.css"); foreach (var file in files) { - _fileSystem.DeleteFile(file); + fs.DeleteFile(file); + } + var dirs = fs.GetDirectories(path); + foreach (var dir in dirs) + { + Purge(fs, dir); + fs.DeleteDirectory(dir); } - _fileSystem = null; } protected Stream CreateStream(string contents = null) diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 6dbf92a7f4..8087a6e709 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -182,6 +182,7 @@ + diff --git a/src/Umbraco.Web/WebServices/SaveFileController.cs b/src/Umbraco.Web/WebServices/SaveFileController.cs index aeb06882c6..24f6917e76 100644 --- a/src/Umbraco.Web/WebServices/SaveFileController.cs +++ b/src/Umbraco.Web/WebServices/SaveFileController.cs @@ -75,6 +75,11 @@ namespace Umbraco.Web.WebServices Func validate, Func> save) { + // sanitize input - partial view names have an extension + filename = filename + .Replace('\\', '/') + .TrimStart('/'); + // sharing the editor with partial views & partial view macros, // using path prefix to differenciate, // but the file service manages different filesystems, @@ -85,8 +90,12 @@ namespace Umbraco.Web.WebServices if (filename.InvariantStartsWith(pathPrefix)) filename = filename.TrimStart(pathPrefix); - if (oldname != null && oldname.InvariantStartsWith(pathPrefix)) - oldname = oldname.TrimStart(pathPrefix); + if (oldname != null) + { + oldname = oldname.TrimStart('/', '\\'); + if (oldname.InvariantStartsWith(pathPrefix)) + oldname = oldname.TrimStart(pathPrefix); + } var view = get(svce, oldname); if (view == null) @@ -187,7 +196,10 @@ namespace Umbraco.Web.WebServices [HttpPost] public JsonResult SaveScript(string filename, string oldName, string contents) { - filename = filename.TrimStart('/', '\\'); + // sanitize input - script names have an extension + filename = filename + .Replace('\\', '/') + .TrimStart('/'); var svce = (FileService) Services.FileService; var script = svce.GetScriptByName(oldName); @@ -223,7 +235,11 @@ namespace Umbraco.Web.WebServices [HttpPost] public JsonResult SaveStylesheet(string filename, string oldName, string contents) { - filename = filename.TrimStart('/', '\\').EnsureEndsWith(".css"); + // sanitize input - stylesheet names have no extension + filename = filename + .Replace('\\', '/') + .TrimStart('/') + .EnsureEndsWith(".css"); var svce = (FileService) Services.FileService; var stylesheet = svce.GetStylesheetByName(oldName);