Skip to content

Commit

Permalink
feat: ownPropertyOnly option to protect prototype, #454
Browse files Browse the repository at this point in the history
  • Loading branch information
harttle committed Jan 28, 2022
1 parent 527858f commit 7e99efc
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 9 deletions.
2 changes: 2 additions & 0 deletions docs/source/tutorials/options.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ it defaults to false. For example, when set to true, a blank string would evalu

**lenientIf** modifies the behavior of `strictVariables` to allow handling optional variables. If set to `true`, an undefined variable will *not* cause an exception in the following two situations: a) it is the condition to an `if`, `elsif`, or `unless` tag; b) it occurs right before a `default` filter. Irrelevant if `strictVariables` is not set. Defaults to `false`.

**ownPropertyOnly** hides scope variables from prototypes, useful when you're passing a not sanitized object into LiquidJS or need to hide prototypes from templates. Defaults to `false`.

{% note info Non-existent Tags %}
Non-existent tags always throw errors during parsing and this behavior can not be customized.
{% endnote %}
Expand Down
2 changes: 2 additions & 0 deletions docs/source/zh-cn/tutorials/options.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ LiquidJS 把这个选项默认值设为 <code>true</code> 以兼容于 shopify/l

**strictVariables** 用来启用变量严格模式。如果设置为 `true` 变量不存在时渲染会抛出异常,默认为 `false` 这时不存在的变量会被渲染为空字符串。

**ownPropertyOnly** 用来隐藏原型上的变量,如果你需要把未经处理过的对象传递给模板时,可以设置 `ownPropertyOnly``true`,默认为 `false`

{% note info 不存在的标签 %}
不存在的标签总是会抛出一个解析异常,这一行为无法自定义。
{% endnote %}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"types": "dist/liquid.d.ts",
"scripts": {
"lint": "eslint \"**/*.ts\" .",
"check": "npm run build && npm test && npm run lint",
"check": "npm run build && npm test && npm run lint && npm run perf:diff",
"test": "nyc mocha \"test/**/*.ts\"",
"test:e2e": "mocha \"test/e2e/**/*.ts\"",
"perf": "cd benchmark && npm ci && npm start",
Expand Down
17 changes: 11 additions & 6 deletions src/context/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,11 @@ export class Context {
return this.getFromScope(scope, paths)
}
public getFromScope (scope: object, paths: string[] | string) {
if (typeof paths === 'string') paths = paths.split('.')
return paths.reduce((scope, path) => {
scope = readProperty(scope, path)
if (isString(paths)) paths = paths.split('.')
return paths.reduce((scope, path, i) => {
scope = readProperty(scope, path, this.opts.ownPropertyOnly)
if (isNil(scope) && this.strictVariables) {
throw new InternalUndefinedVariableError(path)
throw new InternalUndefinedVariableError((paths as string[]).slice(0, i + 1).join!('.'))
}
return scope
}, scope)
Expand All @@ -87,17 +87,22 @@ export class Context {
}
}

export function readProperty (obj: Scope, key: string) {
export function readProperty (obj: Scope, key: string, ownPropertyOnly: boolean) {
if (isNil(obj)) return obj
obj = toLiquid(obj)
if (isFunction(obj[key])) return obj[key]()
const jsProperty = readJSProperty(obj, key, ownPropertyOnly)
if (isFunction(jsProperty)) return jsProperty.call(obj)
if (obj instanceof Drop) {
if (obj.hasOwnProperty(key)) return obj[key]
return obj.liquidMethodMissing(key)
}
if (key === 'size') return readSize(obj)
if (key === 'first') return readFirst(obj)
if (key === 'last') return readLast(obj)
return jsProperty
}
export function readJSProperty (obj: Scope, key: string, ownPropertyOnly: boolean) {
if (ownPropertyOnly && !Object.hasOwnProperty.call(obj, key)) return undefined
return obj[key]
}

Expand Down
8 changes: 8 additions & 0 deletions src/liquid-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ export interface LiquidOptions {
strictFilters?: boolean;
/** Whether or not to assert variable existence. If set to `false`, undefined variables will be rendered as empty string. Otherwise, undefined variables will cause an exception. Defaults to `false`. */
strictVariables?: boolean;
/** Hide scope variables from prototypes, useful when you're passing a not sanitized object into LiquidJS or need to hide prototypes from templates. */
ownPropertyOnly?: boolean;
/** Modifies the behavior of `strictVariables`. If set, a single undefined variable will *not* cause an exception in the context of the `if`/`elsif`/`unless` tag and the `default` filter. Instead, it will evaluate to `false` and `null`, respectively. Irrelevant if `strictVariables` is not set. Defaults to `false`. **/
lenientIf?: boolean;
/** JavaScript timezoneOffset for `date` filter, default to local time. That means if you're in Australia (UTC+10), it'll default to -600 */
Expand Down Expand Up @@ -80,6 +82,10 @@ export interface RenderOptions {
* Same as `strictVariables` on LiquidOptions, but only for current render() call
*/
strictVariables?: boolean;
/**
* Same as `ownPropertyOnly` on LiquidOptions, but only for current render() call
*/
ownPropertyOnly?: boolean;
}

interface NormalizedOptions extends LiquidOptions {
Expand All @@ -103,6 +109,7 @@ export interface NormalizedFullOptions extends NormalizedOptions {
fs: FS;
strictFilters: boolean;
strictVariables: boolean;
ownPropertyOnly: boolean;
lenientIf: boolean;
trimTagRight: boolean;
trimTagLeft: boolean;
Expand Down Expand Up @@ -143,6 +150,7 @@ export const defaultOptions: NormalizedFullOptions = {
preserveTimezones: false,
strictFilters: false,
strictVariables: false,
ownPropertyOnly: false,
lenientIf: false,
globals: {},
keepOutputType: false,
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/issues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,4 +194,9 @@ describe('Issues', function () {
const html = engine.parseAndRenderSync(template, { array: [1, 2, 3] })
expect(html).to.equal('4#8#12#6')
})
it('#454 leaking JS prototype getter functions in evaluation', async () => {
const engine = new Liquid({ ownPropertyOnly: true })
const html = engine.parseAndRenderSync('{{foo | size}}-{{bar.coo}}', { foo: 'foo', bar: Object.create({ coo: 'COO' }) })
expect(html).to.equal('3-')
})
})
66 changes: 64 additions & 2 deletions test/unit/context/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,11 @@ describe('Context', function () {
})
it('should throw when deep variable not exist', async function () {
ctx.push({ foo: 'FOO' })
return expect(() => ctx.get(['foo', 'bar', 'not', 'defined'])).to.throw(/undefined variable: bar/)
return expect(() => ctx.get(['foo', 'bar', 'not', 'defined'])).to.throw(/undefined variable: foo.bar/)
})
it('should throw when itself not defined', async function () {
ctx.push({ foo: 'FOO' })
return expect(() => ctx.get(['foo', 'BAR'])).to.throw(/undefined variable: BAR/)
return expect(() => ctx.get(['foo', 'BAR'])).to.throw(/undefined variable: foo.BAR/)
})
it('should find variable in parent scope', async function () {
ctx.push({ 'foo': 'foo' })
Expand All @@ -110,6 +110,68 @@ describe('Context', function () {
})
})

describe('ownPropertyOnly', async function () {
let ctx: Context
beforeEach(function () {
ctx = new Context(ctx, {
ownPropertyOnly: true
} as any)
})
it('should return undefined for prototype object property', function () {
ctx.push({ foo: Object.create({ bar: 'BAR' }) })
return expect(ctx.get(['foo', 'bar'])).to.equal(undefined)
})
it('should return undefined for Array.prototype.reduce', function () {
ctx.push({ foo: [] })
return expect(ctx.get(['foo', 'reduce'])).to.equal(undefined)
})
it('should return undefined for function prototype property', function () {
function Foo () {}
Foo.prototype.bar = 'BAR'
ctx.push({ foo: new (Foo as any)() })
return expect(ctx.get(['foo', 'bar'])).to.equal(undefined)
})
it('should allow function constructor properties', function () {
function Foo (this: any) { this.bar = 'BAR' }
ctx.push({ foo: new (Foo as any)() })
return expect(ctx.get(['foo', 'bar'])).to.equal('BAR')
})
it('should return undefined for class method', function () {
class Foo { bar () {} }
ctx.push({ foo: new Foo() })
return expect(ctx.get(['foo', 'bar'])).to.equal(undefined)
})
it('should allow class property', function () {
class Foo { bar = 'BAR' }
ctx.push({ foo: new Foo() })
return expect(ctx.get(['foo', 'bar'])).to.equal('BAR')
})
it('should allow Array.prototype.length', function () {
ctx.push({ foo: [1, 2] })
return expect(ctx.get(['foo', 'length'])).to.equal(2)
})
it('should allow size to access Array.prototype.length', function () {
ctx.push({ foo: [1, 2] })
return expect(ctx.get(['foo', 'size'])).to.equal(2)
})
it('should allow size to access Set.prototype.size', function () {
ctx.push({ foo: new Set([1, 2]) })
return expect(ctx.get(['foo', 'size'])).to.equal(2)
})
it('should allow size to access Object key count', function () {
ctx.push({ foo: { bar: 'BAR', coo: 'COO' } })
return expect(ctx.get(['foo', 'size'])).to.equal(2)
})
it('should throw when property is hidden and strictVariables is true', function () {
ctx = new Context(ctx, {
ownPropertyOnly: true,
strictVariables: true
} as any)
ctx.push({ foo: Object.create({ bar: 'BAR' }) })
return expect(() => ctx.get(['foo', 'bar'])).to.throw(/undefined variable: foo.bar/)
})
})

describe('.getAll()', function () {
it('should get all properties when arguments empty', async function () {
expect(ctx.getAll()).deep.equal(scope)
Expand Down

0 comments on commit 7e99efc

Please sign in to comment.