diff --git a/docs/themes/navy/layout/partial/all-contributors.swig b/docs/themes/navy/layout/partial/all-contributors.swig index 9d0180180a..d73b9aacf3 100644 --- a/docs/themes/navy/layout/partial/all-contributors.swig +++ b/docs/themes/navy/layout/partial/all-contributors.swig @@ -48,6 +48,8 @@ + + diff --git a/package.json b/package.json index 99f89b69ea..505933d281 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,7 @@ "lint": "eslint \"**/*.ts\" .", "check": "npm test && npm run lint", "test": "nyc mocha \"test/**/*.ts\"", + "test:e2e": "mocha \"test/e2e/**/*.ts\"", "perf": "cd benchmark && npm ci && npm start", "perf:diff": "bin/perf-diff.sh", "perf:engines": "cd benchmark && npm run engines", diff --git a/src/builtin/filters/string.ts b/src/builtin/filters/string.ts index 400927270e..d323490914 100644 --- a/src/builtin/filters/string.ts +++ b/src/builtin/filters/string.ts @@ -7,12 +7,12 @@ import { stringify } from '../../util/underscore' import { assert } from '../../util/assert' export function append (v: string, arg: string) { - assert(arguments.length === 2, () => 'append expect 2 arguments') + assert(arguments.length === 2, 'append expect 2 arguments') return stringify(v) + stringify(arg) } export function prepend (v: string, arg: string) { - assert(arguments.length === 2, () => 'prepend expect 2 arguments') + assert(arguments.length === 2, 'prepend expect 2 arguments') return stringify(arg) + stringify(v) } diff --git a/src/fs/browser.ts b/src/fs/browser.ts index 053d9dea8f..fe0dd94446 100644 --- a/src/fs/browser.ts +++ b/src/fs/browser.ts @@ -60,3 +60,9 @@ export async function exists (filepath: string) { export function existsSync (filepath: string) { return true } + +export function dirname (filepath: string) { + return domResolve(filepath, '.') +} + +export const sep = '/' diff --git a/src/fs/fs.ts b/src/fs/fs.ts index badbf9a95e..7526f80b31 100644 --- a/src/fs/fs.ts +++ b/src/fs/fs.ts @@ -1,8 +1,20 @@ +import { LoaderOptions } from './loader' + export interface FS { + /** check if a file exists asynchronously */ exists: (filepath: string) => Promise; - readFile: (filepath: string) => Promise; + /** check if a file exists synchronously */ existsSync: (filepath: string) => boolean; + /** read a file asynchronously */ + readFile: (filepath: string) => Promise; + /** read a file synchronously */ readFileSync: (filepath: string) => string; - resolve: (root: string, file: string, ext: string) => string; + /** resolve a file against directory, for given `ext` option */ + resolve: (dir: string, file: string, ext: string, options?: LoaderOptions) => string; + /** defaults to "/", will be used for "within roots" check */ + sep?: string; + /** dirname for a filepath, used when resolving relative path */ + dirname?: (file: string) => string; + /** fallback file for lookup failure */ fallback?: (file: string) => string | undefined; } diff --git a/src/fs/loader.ts b/src/fs/loader.ts index c1b7f8970f..bc383a9b75 100644 --- a/src/fs/loader.ts +++ b/src/fs/loader.ts @@ -1,6 +1,8 @@ import { FS } from './fs' +import { escapeRegex } from '../util/underscore' +import { assert } from '../util/assert' -interface LoaderOptions { +export interface LoaderOptions { fs: FS; extname: string; root: string[]; @@ -15,9 +17,13 @@ export enum LookupType { } export class Loader { private options: LoaderOptions + private sep: string + private rRelativePath: RegExp constructor (options: LoaderOptions) { this.options = options + this.sep = this.options.fs.sep || '/' + this.rRelativePath = new RegExp(['.' + this.sep, '..' + this.sep].map(prefix => escapeRegex(prefix)).join('|')) } public * lookup (file: string, type: LookupType, sync?: boolean, currentFile?: string) { @@ -29,20 +35,12 @@ export class Loader { throw this.lookupError(file, dirs) } - public shouldLoadRelative (currentFile: string) { - return this.options.relativeReference && this.isRelativePath(currentFile) - } - - public isRelativePath (path: string) { - return path.startsWith('./') || path.startsWith('../') - } - public * candidates (file: string, dirs: string[], currentFile?: string, enforceRoot?: boolean) { const { fs, extname } = this.options if (this.shouldLoadRelative(file) && currentFile) { - const referenced = fs.resolve(this.dirname(currentFile), file, extname) + const referenced = fs.resolve(this.dirname(currentFile), file, extname, this.options) for (const dir of dirs) { - if (!enforceRoot || referenced.startsWith(dir)) { + if (!enforceRoot || this.withinDir(referenced, dir)) { // the relatively referenced file is within one of root dirs yield referenced break @@ -51,7 +49,7 @@ export class Loader { } for (const dir of dirs) { const referenced = fs.resolve(dir, file, extname) - if (!enforceRoot || referenced.startsWith(dir)) { + if (!enforceRoot || this.withinDir(referenced, dir)) { yield referenced } } @@ -61,10 +59,19 @@ export class Loader { } } + private withinDir (file: string, dir: string) { + dir = dir.endsWith(this.sep) ? dir : dir + this.sep + return file.startsWith(dir) + } + + private shouldLoadRelative (referencedFile: string) { + return this.options.relativeReference && this.rRelativePath.test(referencedFile) + } + private dirname (path: string) { - const segments = path.split('/') - segments.pop() - return segments.join('/') + const fs = this.options.fs + assert(fs.dirname, '`fs.dirname` is required for relative reference') + return fs.dirname!(path) } private lookupError (file: string, roots: string[]) { diff --git a/src/fs/node.ts b/src/fs/node.ts index 8ef7bdadea..ce1d32f7c6 100644 --- a/src/fs/node.ts +++ b/src/fs/node.ts @@ -1,6 +1,7 @@ import * as _ from '../util/underscore' -import { resolve as nodeResolve, extname } from 'path' +import { resolve as nodeResolve, extname, dirname as nodeDirname } from 'path' import { stat, statSync, readFile as nodeReadFile, readFileSync as nodeReadFileSync } from 'fs' +import { LiquidOptions } from '../liquid-options' const statAsync = _.promisify(stat) const readFileAsync = _.promisify(nodeReadFile) @@ -27,7 +28,7 @@ export function existsSync (filepath: string) { export function readFileSync (filepath: string) { return nodeReadFileSync(filepath, 'utf8') } -export function resolve (root: string, file: string, ext: string) { +export function resolve (root: string, file: string, ext: string, opts: LiquidOptions) { if (!extname(file)) file += ext return nodeResolve(root, file) } @@ -36,3 +37,7 @@ export function fallback (file: string) { return require.resolve(file) } catch (e) {} } +export function dirname (filepath: string) { + return nodeDirname(filepath) +} +export { sep } from 'path' diff --git a/src/liquid-options.ts b/src/liquid-options.ts index 2803ec4e2b..2a7fcd4e94 100644 --- a/src/liquid-options.ts +++ b/src/liquid-options.ts @@ -169,5 +169,5 @@ export function normalizeDirectoryList (value: any): string[] { let list: string[] = [] if (_.isArray(value)) list = value if (_.isString(value)) list = [value] - return list.map(str => fs.resolve(str, '.', '')).map(str => str[str.length - 1] !== '/' ? str + '/' : str) + return list } diff --git a/src/render/expression.ts b/src/render/expression.ts index f1efd51834..e284670257 100644 --- a/src/render/expression.ts +++ b/src/render/expression.ts @@ -21,7 +21,7 @@ export class Expression { this.postfix = [...toPostfix(tokens)] } public * evaluate (ctx: Context, lenient: boolean): any { - assert(ctx, () => 'unable to evaluate: context not defined') + assert(ctx, 'unable to evaluate: context not defined') const operands: any[] = [] for (const token of this.postfix) { if (TypeGuards.isOperatorToken(token)) { diff --git a/src/util/assert.ts b/src/util/assert.ts index c2fc92f1b5..67dcd7eca5 100644 --- a/src/util/assert.ts +++ b/src/util/assert.ts @@ -1,8 +1,10 @@ import { AssertionError } from './error' -export function assert (predicate: T | null | undefined, message?: () => string) { +export function assert (predicate: T | null | undefined, message?: string | (() => string)) { if (!predicate) { - const msg = message ? message() : `expect ${predicate} to be true` + const msg = typeof message === 'function' + ? message() + : (message || `expect ${predicate} to be true`) throw new AssertionError(msg) } } diff --git a/src/util/underscore.ts b/src/util/underscore.ts index e57ef74831..2cc55a6661 100644 --- a/src/util/underscore.ts +++ b/src/util/underscore.ts @@ -11,6 +11,10 @@ export function isFunction (value: any): value is Function { return typeof value === 'function' } +export function escapeRegex (str: string) { + return str.replace(/[-/\\^$*+?.()|[\]{}]/g, '\\$&') +} + export function promisify (fn: (arg1: T1, cb: (err: Error | null, result: T2) => void) => void): (arg1: T1) => Promise; export function promisify (fn: (arg1: T1, arg2: T2, cb: (err: Error | null, result: T3) => void) => void): (arg1: T1, arg2: T2) => Promise; export function promisify (fn: any) { diff --git a/test/e2e/issues.ts b/test/e2e/issues.ts index 90a7472a95..31e947ffaf 100644 --- a/test/e2e/issues.ts +++ b/test/e2e/issues.ts @@ -114,4 +114,19 @@ describe('Issues', function () { const html = await engine.render(tpl, { date: '2021-10-06T15:31:00+08:00' }) expect(html).to.equal('2021-10-06 17:31 PM +1000') }) + it('#412 Pass root as it is to `resolve`', async () => { + const engine = new Liquid({ + root: '/tmp', + fs: { + readFileSync: (file: string) => file, + async readFile (file: string) { return 'foo' }, + existsSync (file: string) { return true }, + async exists (file: string) { return true }, + resolve: (dir: string, file: string) => dir + '/' + file + } + }) + const tpl = engine.parse('{% include "foo.liquid" %}') + const html = await engine.renderSync(tpl) + expect(html).to.equal('/tmp/foo.liquid') + }) }) diff --git a/test/integration/liquid/fs-option.ts b/test/integration/liquid/fs-option.ts index 83b9ed9bf0..5450d93058 100644 --- a/test/integration/liquid/fs-option.ts +++ b/test/integration/liquid/fs-option.ts @@ -6,16 +6,16 @@ use(chaiAsPromised) describe('LiquidOptions#fs', function () { let engine: Liquid const fs = { - exists: (x: string) => Promise.resolve(x.match(/^\/root\/files\//)), - existsSync: (x: string) => x.match(/^\/root\/files\//), + exists: (x: string) => Promise.resolve(!x.match(/not-exist/)), + existsSync: (x: string) => !x.match(/not-exist/), readFile: (x: string) => Promise.resolve(`content for ${x}`), readFileSync: (x: string) => `content for ${x}`, fallback: (x: string) => '/root/files/fallback', - resolve: (base: string, path: string) => base + path + resolve: (base: string, path: string) => base + '/' + path } beforeEach(function () { engine = new Liquid({ - root: '/root/', + root: '/root', fs } as any) }) @@ -25,12 +25,12 @@ describe('LiquidOptions#fs', function () { }) it('should support fallback', async function () { - const html = await engine.renderFile('notexist/foo') + const html = await engine.renderFile('not-exist/foo') expect(html).to.equal('content for /root/files/fallback') }) it('should support renderSync', function () { - const html = engine.renderFileSync('notexist/foo') + const html = engine.renderFileSync('not-exist/foo') expect(html).to.equal('content for /root/files/fallback') }) @@ -39,7 +39,7 @@ describe('LiquidOptions#fs', function () { root: '/root/', fs: { ...fs, fallback: undefined } } as any) - return expect(engine.renderFile('notexist/foo')) + return expect(engine.renderFile('not-exist/foo')) .to.be.rejectedWith('Failed to lookup') }) }) diff --git a/test/integration/liquid/liquid.ts b/test/integration/liquid/liquid.ts index 2adf9d712c..5a5418d421 100644 --- a/test/integration/liquid/liquid.ts +++ b/test/integration/liquid/liquid.ts @@ -75,7 +75,7 @@ describe('Liquid', function () { extname: '.html' }) return expect(engine.renderFile('/not/exist.html')).to - .be.rejectedWith(/Failed to lookup "\/not\/exist.html" in "\/boo\/,\/root\/"/) + .be.rejectedWith(/Failed to lookup "\/not\/exist.html" in "\/boo,\/root\/"/) }) }) describe('#parseFile', function () { @@ -85,7 +85,7 @@ describe('Liquid', function () { extname: '.html' }) return expect(engine.parseFile('/not/exist.html')).to - .be.rejectedWith(/Failed to lookup "\/not\/exist.html" in "\/boo\/,\/root\/"/) + .be.rejectedWith(/Failed to lookup "\/not\/exist.html" in "\/boo,\/root\/"/) }) it('should fallback to require.resolve in Node.js', async function () { const engine = new Liquid({ @@ -144,7 +144,7 @@ describe('Liquid', function () { extname: '.html' }) return expect(() => engine.parseFileSync('/not/exist.html')) - .to.throw(/Failed to lookup "\/not\/exist.html" in "\/boo\/,\/root\/"/) + .to.throw(/Failed to lookup "\/not\/exist.html" in "\/boo,\/root\/"/) }) it('should throw with lookup list when file not exist', function () { const engine = new Liquid({ @@ -152,7 +152,7 @@ describe('Liquid', function () { extname: '.html' }) return expect(() => engine.parseFileSync('/not/exist.html')) - .to.throw(/Failed to lookup "\/not\/exist.html" in "\/boo\/,\/root\/"/) + .to.throw(/Failed to lookup "\/not\/exist.html" in "\/boo,\/root\/"/) }) }) describe('#enderToNodeStream', function () { diff --git a/test/integration/liquid/root.ts b/test/integration/liquid/root.ts index cc9aba147f..d6caf34281 100644 --- a/test/integration/liquid/root.ts +++ b/test/integration/liquid/root.ts @@ -1,12 +1,11 @@ import { normalize } from '../../../src/liquid-options' import { expect } from 'chai' -import { resolve } from 'path' describe('LiquidOptions#root', function () { describe('#normalize ()', function () { it('should normalize string typed root array', function () { const options = normalize({ root: 'foo' }) - expect(options.root).to.eql([resolve('foo') + '/']) + expect(options.root).to.eql(['foo']) }) it('should normalize null typed root as empty array', function () { const options = normalize({ root: null } as any) diff --git a/test/unit/fs/browser.ts b/test/unit/fs/browser.ts index 9588c33d39..729a659616 100644 --- a/test/unit/fs/browser.ts +++ b/test/unit/fs/browser.ts @@ -6,23 +6,23 @@ import * as chaiAsPromised from 'chai-as-promised' use(chaiAsPromised) describe('fs/browser', function () { + if (+(process.version.match(/^v(\d+)/) as RegExpMatchArray)[1] < 8) { + console.info('jsdom not supported, skipping template-browser...') + return + } + const JSDOM = require('jsdom').JSDOM + beforeEach(function () { + const dom = new JSDOM(``, { + url: 'https://example.com/foo/bar/', + contentType: 'text/html', + includeNodeLocations: true + }); + (global as any).document = dom.window.document + }) + afterEach(function () { + delete (global as any).document + }) describe('#resolve()', function () { - if (+(process.version.match(/^v(\d+)/) as RegExpMatchArray)[1] < 8) { - console.info('jsdom not supported, skipping template-browser...') - return - } - const JSDOM = require('jsdom').JSDOM - beforeEach(function () { - const dom = new JSDOM(``, { - url: 'https://example.com/foo/bar/', - contentType: 'text/html', - includeNodeLocations: true - }); - (global as any).document = dom.window.document - }) - afterEach(function () { - delete (global as any).document - }) it('should support relative root', function () { expect(fs.resolve('./views/', 'foo', '')).to.equal('https://example.com/foo/bar/views/foo') }) @@ -52,6 +52,13 @@ describe('fs/browser', function () { }) }) + describe('#dirname()', () => { + it('should return dirname of file', async function () { + const val = fs.dirname('https://example.com/views/foo/bar') + expect(val).to.equal('https://example.com/views/foo/') + }) + }) + describe('#exists()', () => { it('should always return true', async function () { const val = await fs.exists('/foo/bar') diff --git a/test/unit/fs/loader.ts b/test/unit/fs/loader.ts index 63eddeba69..21429660bf 100644 --- a/test/unit/fs/loader.ts +++ b/test/unit/fs/loader.ts @@ -12,5 +12,15 @@ describe('fs/loader', function () { const candidates = [...loader.candidates('./foo/bar', ['/root', '/root/foo'], '/root/current', true)] expect(candidates).to.contain('/root/foo/bar') }) + it('should not include out of root candidates', async function () { + const loader = new Loader({ relativeReference: true, fs, extname: '' } as any) + const candidates = [...loader.candidates('../foo/bar', ['/root'], '/root/current', true)] + expect(candidates).to.have.lengthOf(0) + }) + it('should treat root as a terminated path', async function () { + const loader = new Loader({ relativeReference: true, fs, extname: '' } as any) + const candidates = [...loader.candidates('../root-dir/bar', ['/root'], '/root/current', true)] + expect(candidates).to.have.lengthOf(0) + }) }) })