From 2ebb79df8b8bc54c9f2f19f0ebf4a841621e01f2 Mon Sep 17 00:00:00 2001 From: Kyle Fuller Date: Wed, 30 Nov 2016 17:07:38 +0000 Subject: [PATCH] refactor(loader): Throw when template not found --- CHANGELOG.md | 5 ++ Sources/Errors.swift | 14 ++++ Sources/Include.swift | 5 +- Sources/Inheritence.swift | 4 +- Sources/Loader.swift | 101 +++++++++++++++++++++++ Tests/StencilTests/IncludeSpec.swift | 2 +- Tests/StencilTests/InheritenceSpec.swift | 4 +- Tests/StencilTests/LoaderSpec.swift | 31 +++++++ 8 files changed, 156 insertions(+), 10 deletions(-) create mode 100644 Sources/Errors.swift create mode 100644 Sources/Loader.swift create mode 100644 Tests/StencilTests/LoaderSpec.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 94ec4bd..874d136 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Master +### Breaking + +- `Loader`s will now throw a `TemplateDoesNotExist` error when a template + is not found. + ### Enhancements - `FileSystemLoader` will now ensure that template paths are within the base diff --git a/Sources/Errors.swift b/Sources/Errors.swift new file mode 100644 index 0000000..7151561 --- /dev/null +++ b/Sources/Errors.swift @@ -0,0 +1,14 @@ +public class TemplateDoesNotExist: Error, CustomStringConvertible { + let templateNames: [String] + let loader: Loader + + public init(templateNames: [String], loader: Loader) { + self.templateNames = templateNames + self.loader = loader + } + + public var description: String { + let templates = templateNames.joined(separator: ", ") + return "Template named `\(templates)` does not exist in loader \(loader)" + } +} diff --git a/Sources/Include.swift b/Sources/Include.swift index 97c2359..aac580d 100644 --- a/Sources/Include.swift +++ b/Sources/Include.swift @@ -27,10 +27,7 @@ class IncludeNode : NodeType { throw TemplateSyntaxError("'\(self.templateName)' could not be resolved as a string") } - guard let template = try loader.loadTemplate(name: templateName) else { - throw TemplateSyntaxError("'\(templateName)' template not found") - } - + let template = try loader.loadTemplate(name: templateName) return try template.render(context) } } diff --git a/Sources/Inheritence.swift b/Sources/Inheritence.swift index 428fe1c..941d626 100644 --- a/Sources/Inheritence.swift +++ b/Sources/Inheritence.swift @@ -67,9 +67,7 @@ class ExtendsNode : NodeType { throw TemplateSyntaxError("'\(self.templateName)' could not be resolved as a string") } - guard let template = try loader.loadTemplate(name: templateName) else { - throw TemplateSyntaxError("'\(templateName)' template not found") - } + let template = try loader.loadTemplate(name: templateName) let blockContext: BlockContext if let context = context[BlockContext.contextKey] as? BlockContext { diff --git a/Sources/Loader.swift b/Sources/Loader.swift new file mode 100644 index 0000000..f44cb04 --- /dev/null +++ b/Sources/Loader.swift @@ -0,0 +1,101 @@ +import Foundation +import PathKit + + +public protocol Loader { + func loadTemplate(name: String) throws -> Template + func loadTemplate(names: [String]) throws -> Template +} + + +extension Loader { + func loadTemplate(names: [String]) throws -> Template { + for name in names { + do { + return try loadTemplate(name: name) + } catch is TemplateDoesNotExist { + continue + } catch { + throw error + } + } + + throw TemplateDoesNotExist(templateNames: names, loader: self) + } +} + + +// A class for loading a template from disk +public class FileSystemLoader: Loader, CustomStringConvertible { + public let paths: [Path] + + public init(paths: [Path]) { + self.paths = paths + } + + public init(bundle: [Bundle]) { + self.paths = bundle.map { + return Path($0.bundlePath) + } + } + + public var description: String { + return "FileSystemLoader(\(paths))" + } + + public func loadTemplate(name: String) throws -> Template { + for path in paths { + let templatePath = try path.safeJoin(path: Path(name)) + + if !templatePath.exists { + continue + } + + return try Template(path: templatePath) + } + + throw TemplateDoesNotExist(templateNames: [name], loader: self) + } + + public func loadTemplate(names: [String]) throws -> Template { + for path in paths { + for templateName in names { + let templatePath = try path.safeJoin(path: Path(templateName)) + + if templatePath.exists { + return try Template(path: templatePath) + } + } + } + + throw TemplateDoesNotExist(templateNames: names, loader: self) + } +} + + +extension Path { + func safeJoin(path: Path) throws -> Path { + let newPath = self + path + + if !newPath.absolute().description.hasPrefix(absolute().description) { + throw SuspiciousFileOperation(basePath: self, path: newPath) + } + + return newPath + } +} + + +class SuspiciousFileOperation: Error { + let basePath: Path + let path: Path + + init(basePath: Path, path: Path) { + self.basePath = basePath + self.path = path + } + + var description: String { + return "Path `\(path)` is located outside of base path `\(basePath)`" + } +} diff --git a/Tests/StencilTests/IncludeSpec.swift b/Tests/StencilTests/IncludeSpec.swift index 7e2cbf5..85ab839 100644 --- a/Tests/StencilTests/IncludeSpec.swift +++ b/Tests/StencilTests/IncludeSpec.swift @@ -45,7 +45,7 @@ func testInclude() { do { _ = try node.render(Context(dictionary: ["loader": loader])) } catch { - try expect("\(error)".hasPrefix("'unknown.html' template not found")).to.beTrue() + try expect("\(error)".hasPrefix("Template named `unknown.html` does not exist in loader")).to.beTrue() } } diff --git a/Tests/StencilTests/InheritenceSpec.swift b/Tests/StencilTests/InheritenceSpec.swift index 5267bf8..88c9a17 100644 --- a/Tests/StencilTests/InheritenceSpec.swift +++ b/Tests/StencilTests/InheritenceSpec.swift @@ -11,13 +11,13 @@ func testInheritence() { $0.it("can inherit from another template") { let context = Context(dictionary: ["loader": loader]) let template = try loader.loadTemplate(name: "child.html") - try expect(try template?.render(context)) == "Header\nChild" + try expect(try template.render(context)) == "Header\nChild" } $0.it("can inherit from another template inheriting from another template") { let context = Context(dictionary: ["loader": loader]) let template = try loader.loadTemplate(name: "child-child.html") - try expect(try template?.render(context)) == "Child Child Header\nChild" + try expect(try template.render(context)) == "Child Child Header\nChild" } } } diff --git a/Tests/StencilTests/LoaderSpec.swift b/Tests/StencilTests/LoaderSpec.swift new file mode 100644 index 0000000..6999613 --- /dev/null +++ b/Tests/StencilTests/LoaderSpec.swift @@ -0,0 +1,31 @@ +import Spectre +import Stencil +import PathKit + + +func testTemplateLoader() { + describe("FileSystemLoader") { + let path = Path(#file) + ".." + "fixtures" + let loader = FileSystemLoader(paths: [path]) + + $0.it("errors when a template cannot be found") { + try expect(try loader.loadTemplate(name: "unknown.html")).toThrow() + } + + $0.it("errors when an array of templates cannot be found") { + try expect(try loader.loadTemplate(names: ["unknown.html", "unknown2.html"])).toThrow() + } + + $0.it("can load a template from a file") { + _ = try loader.loadTemplate(name: "test.html") + } + + $0.it("errors when loading absolute file outside of the selected path") { + try expect(try loader.loadTemplate(name: "/etc/hosts")).toThrow() + } + + $0.it("errors when loading relative file outside of the selected path") { + try expect(try loader.loadTemplate(name: "../LoaderSpec.swift")).toThrow() + } + } +}