From 7dfb620d30f8818685e1cfb8e7492313a0d036ab Mon Sep 17 00:00:00 2001 From: Harttle Date: Sun, 27 Feb 2022 01:46:25 +0800 Subject: [PATCH] fix: some filters throw on nil input, see #481 --- package-lock.json | 6 +- package.json | 21 ++++-- src/builtin/filters/array.ts | 42 ++++++++---- src/builtin/filters/date.ts | 4 +- src/builtin/filters/html.ts | 8 +-- src/builtin/filters/math.ts | 34 ++++------ src/builtin/filters/object.ts | 2 +- src/builtin/filters/string.ts | 10 ++- src/util/collection.ts | 3 +- src/util/underscore.ts | 8 ++- test/e2e/issues.ts | 14 ++++ test/integration/builtin/filters/array.ts | 83 +++++++++++++++++++++-- test/integration/builtin/filters/html.ts | 2 + test/integration/builtin/filters/math.ts | 38 ----------- 14 files changed, 182 insertions(+), 93 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4b307191e..d515b6557 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2662,9 +2662,9 @@ } }, "caniuse-lite": { - "version": "1.0.30001239", - "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001239.tgz", - "integrity": "sha512-cyBkXJDMeI4wthy8xJ2FvDU6+0dtcZSJW3voUF8+e9f1bBeuvyZfc3PNbkOETyhbR+dGCPzn9E7MA3iwzusOhQ==", + "version": "1.0.30001312", + "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001312.tgz", + "integrity": "sha512-Wiz1Psk2MEK0pX3rUzWaunLTZzqS2JYZFzNKqAiJGiuxIjRPLgV6+VDPOg6lQOUxmDwhTlh198JsTTi8Hzw6aQ==", "dev": true }, "cardinal": { diff --git a/package.json b/package.json index 25579fd93..45c5cd9a1 100644 --- a/package.json +++ b/package.json @@ -129,11 +129,21 @@ } ], [ - "@semantic-release/github", { + "@semantic-release/github", + { "assets": [ - {"path": "dist/*.umd.js", "label": "liquid.js"}, - {"path": "dist/*.min.js", "label": "liquid.min.js"}, - {"path": "dist/*.min.js.map", "label": "liquid.min.js.map"} + { + "path": "dist/*.umd.js", + "label": "liquid.js" + }, + { + "path": "dist/*.min.js", + "label": "liquid.min.js" + }, + { + "path": "dist/*.min.js.map", + "label": "liquid.min.js.map" + } ] } ] @@ -149,5 +159,6 @@ "pre-commit": "npm run check", "commit-msg": "commitlint -E HUSKY_GIT_PARAMS" } - } + }, + "dependencies": {} } diff --git a/src/builtin/filters/array.ts b/src/builtin/filters/array.ts index 8cce3efd0..46194eea6 100644 --- a/src/builtin/filters/array.ts +++ b/src/builtin/filters/array.ts @@ -1,17 +1,18 @@ -import { isArray, isNil, last as arrayLast } from '../../util/underscore' +import { argumentsToValue, toValue, stringify, caseInsensitiveCompare, isArray, isNil, last as arrayLast, hasOwnProperty } from '../../util/underscore' import { toArray } from '../../util/collection' import { isTruthy } from '../../render/boolean' import { FilterImpl } from '../../template/filter/filter-impl' import { Scope } from '../../context/scope' import { isComparable } from '../../drop/comparable' -export const join = (v: any[], arg: string) => v.join(arg === undefined ? ' ' : arg) -export const last = (v: any) => isArray(v) ? arrayLast(v) : '' -export const first = (v: any) => isArray(v) ? v[0] : '' -export const reverse = (v: any[]) => [...v].reverse() +export const join = argumentsToValue((v: any[], arg: string) => toArray(v).join(arg === undefined ? ' ' : arg)) +export const last = argumentsToValue((v: any) => isArray(v) ? arrayLast(v) : '') +export const first = argumentsToValue((v: any) => isArray(v) ? v[0] : '') +export const reverse = argumentsToValue((v: any[]) => [...toArray(v)].reverse()) export function sort (this: FilterImpl, arr: T[], property?: string) { - const getValue = (obj: Scope) => property ? this.context.getFromScope(obj, property.split('.')) : obj + arr = toValue(arr) + const getValue = (obj: Scope) => property ? this.context.getFromScope(obj, stringify(property).split('.')) : obj return [...toArray(arr)].sort((lhs, rhs) => { lhs = getValue(lhs) rhs = getValue(rhs) @@ -19,28 +20,44 @@ export function sort (this: FilterImpl, arr: T[], property?: string) { }) } +export function sortNatural (input: T[], property?: string) { + input = toValue(input) + const propertyString = stringify(property) + const compare = property === undefined + ? caseInsensitiveCompare + : (lhs: T, rhs: T) => caseInsensitiveCompare(lhs[propertyString], rhs[propertyString]) + return [...toArray(input)].sort(compare) +} + export const size = (v: string | any[]) => (v && v.length) || 0 export function map (this: FilterImpl, arr: Scope[], property: string) { - return toArray(arr).map(obj => this.context.getFromScope(obj, property.split('.'))) + arr = toValue(arr) + return toArray(arr).map(obj => this.context.getFromScope(obj, stringify(property).split('.'))) } export function compact (this: FilterImpl, arr: T[]) { - return toArray(arr).filter(x => !isNil(x)) + arr = toValue(arr) + return toArray(arr).filter(x => !isNil(toValue(x))) } -export function concat (v: T1[], arg: T2[] | T2): (T1 | T2)[] { +export function concat (v: T1[], arg: T2[]): (T1 | T2)[] { + v = toValue(v) return toArray(v).concat(arg) } -export function slice (v: T[], begin: number, length = 1): T[] { +export function slice (v: T[] | string, begin: number, length = 1): T[] | string { + v = toValue(v) + if (isNil(v)) return [] + if (!isArray(v)) v = stringify(v) begin = begin < 0 ? v.length + begin : begin return v.slice(begin, begin + length) } export function where (this: FilterImpl, arr: T[], property: string, expected?: any): T[] { + arr = toValue(arr) return toArray(arr).filter(obj => { - const value = this.context.getFromScope(obj, String(property).split('.')) + const value = this.context.getFromScope(obj, stringify(property).split('.')) if (expected === undefined) return isTruthy(value, this.context) if (isComparable(expected)) return expected.equals(value) return value === expected @@ -48,9 +65,10 @@ export function where (this: FilterImpl, arr: T[], property: s } export function uniq (arr: T[]): T[] { + arr = toValue(arr) const u = {} return (arr || []).filter(val => { - if (u.hasOwnProperty(String(val))) return false + if (hasOwnProperty.call(u, String(val))) return false u[String(val)] = true return true }) diff --git a/src/builtin/filters/date.ts b/src/builtin/filters/date.ts index 6559df36e..f424bcb42 100644 --- a/src/builtin/filters/date.ts +++ b/src/builtin/filters/date.ts @@ -1,12 +1,14 @@ import strftime from '../../util/strftime' import { LiquidDate } from '../../util/liquid-date' -import { isString, isNumber } from '../../util/underscore' +import { toValue, stringify, isString, isNumber } from '../../util/underscore' import { FilterImpl } from '../../template/filter/filter-impl' import { TimezoneDate } from '../../util/timezone-date' export function date (this: FilterImpl, v: string | Date, arg: string) { const opts = this.context.opts let date: LiquidDate + v = toValue(v) + arg = stringify(arg) if (v === 'now' || v === 'today') { date = new Date() } else if (isNumber(v)) { diff --git a/src/builtin/filters/html.ts b/src/builtin/filters/html.ts index 2c1e1bd20..cdb340fe9 100644 --- a/src/builtin/filters/html.ts +++ b/src/builtin/filters/html.ts @@ -20,17 +20,17 @@ export function escape (str: string) { } function unescape (str: string) { - return String(str).replace(/&(amp|lt|gt|#34|#39);/g, m => unescapeMap[m]) + return stringify(str).replace(/&(amp|lt|gt|#34|#39);/g, m => unescapeMap[m]) } export function escapeOnce (str: string) { - return escape(unescape(str)) + return escape(unescape(stringify(str))) } export function newlineToBr (v: string) { - return v.replace(/\n/g, '
\n') + return stringify(v).replace(/\n/g, '
\n') } export function stripHtml (v: string) { - return v.replace(/|||<.*?>/g, '') + return stringify(v).replace(/|||<.*?>/g, '') } diff --git a/src/builtin/filters/math.ts b/src/builtin/filters/math.ts index a1670b986..350fc1735 100644 --- a/src/builtin/filters/math.ts +++ b/src/builtin/filters/math.ts @@ -1,30 +1,24 @@ -import { caseInsensitiveCompare } from '../../util/underscore' +import { toValue, argumentsToValue } from '../../util/underscore' -export const abs = Math.abs -export const atLeast = Math.max -export const atMost = Math.min -export const ceil = Math.ceil -export const dividedBy = (v: number, arg: number) => v / arg -export const floor = Math.floor -export const minus = (v: number, arg: number) => v - arg -export const modulo = (v: number, arg: number) => v % arg -export const times = (v: number, arg: number) => v * arg +export const abs = argumentsToValue(Math.abs) +export const atLeast = argumentsToValue(Math.max) +export const atMost = argumentsToValue(Math.min) +export const ceil = argumentsToValue(Math.ceil) +export const dividedBy = argumentsToValue((v: number, arg: number) => v / arg) +export const floor = argumentsToValue(Math.floor) +export const minus = argumentsToValue((v: number, arg: number) => v - arg) +export const modulo = argumentsToValue((v: number, arg: number) => v % arg) +export const times = argumentsToValue((v: number, arg: number) => v * arg) export function round (v: number, arg = 0) { + v = toValue(v) + arg = toValue(arg) const amp = Math.pow(10, arg) return Math.round(v * amp) / amp } export function plus (v: number, arg: number) { + v = toValue(v) + arg = toValue(arg) return Number(v) + Number(arg) } - -export function sortNatural (input: any[], property?: string) { - if (!input || !input.sort) return [] - if (property !== undefined) { - return [...input].sort( - (lhs, rhs) => caseInsensitiveCompare(lhs[property], rhs[property]) - ) - } - return [...input].sort(caseInsensitiveCompare) -} diff --git a/src/builtin/filters/object.ts b/src/builtin/filters/object.ts index 474cc5485..bdbfb4b6e 100644 --- a/src/builtin/filters/object.ts +++ b/src/builtin/filters/object.ts @@ -3,8 +3,8 @@ import { isArray, isString, toValue } from '../../util/underscore' import { FilterImpl } from '../../template/filter/filter-impl' export function Default (this: FilterImpl, value: T1, defaultValue: T2, ...args: Array<[string, any]>): T1 | T2 { - if (isArray(value) || isString(value)) return value.length ? value : defaultValue value = toValue(value) + if (isArray(value) || isString(value)) return value.length ? value : defaultValue if (value === false && (new Map(args)).get('allow_false')) return false as T1 return isFalsy(value, this.context) ? defaultValue : value } diff --git a/src/builtin/filters/string.ts b/src/builtin/filters/string.ts index d32349091..650602af1 100644 --- a/src/builtin/filters/string.ts +++ b/src/builtin/filters/string.ts @@ -41,7 +41,11 @@ export function rstrip (str: string) { } export function split (v: string, arg: string) { - return stringify(v).split(String(arg)) + const arr = stringify(v).split(String(arg)) + // align to ruby split, which is the behavior of shopify/liquid + // see: https://ruby-doc.org/core-2.4.0/String.html#method-i-split + while (arr.length && arr[arr.length - 1] === '') arr.pop() + return arr } export function strip (v: string) { @@ -68,11 +72,11 @@ export function replaceFirst (v: string, arg1: string, arg2: string) { export function truncate (v: string, l = 50, o = '...') { v = stringify(v) if (v.length <= l) return v - return v.substr(0, l - o.length) + o + return v.substring(0, l - o.length) + o } export function truncatewords (v: string, l = 15, o = '...') { - const arr = v.split(/\s+/) + const arr = stringify(v).split(/\s+/) let ret = arr.slice(0, l).join(' ') if (arr.length >= l) ret += o return ret diff --git a/src/util/collection.ts b/src/util/collection.ts index edbeeb886..48cd75b23 100644 --- a/src/util/collection.ts +++ b/src/util/collection.ts @@ -1,4 +1,4 @@ -import { isString, isObject, isArray } from './underscore' +import { isNil, isString, isObject, isArray } from './underscore' export function toEnumerable (val: any) { if (isArray(val)) return val @@ -8,6 +8,7 @@ export function toEnumerable (val: any) { } export function toArray (val: any) { + if (isNil(val)) return [] if (isArray(val)) return val return [ val ] } diff --git a/src/util/underscore.ts b/src/util/underscore.ts index 9a32239fc..fdc335ad0 100644 --- a/src/util/underscore.ts +++ b/src/util/underscore.ts @@ -3,6 +3,8 @@ import { Drop } from '../drop/drop' const toStr = Object.prototype.toString const toLowerCase = String.prototype.toLowerCase +export const hasOwnProperty = Object.hasOwnProperty + export function isString (value: any): value is string { return typeof value === 'string' } @@ -72,7 +74,7 @@ export function forOwn ( ) { obj = obj || {} for (const k in obj) { - if (Object.hasOwnProperty.call(obj, k)) { + if (hasOwnProperty.call(obj, k)) { if (iteratee(obj[k], k, obj) === false) break } } @@ -150,3 +152,7 @@ export function caseInsensitiveCompare (a: any, b: any) { if (a > b) return 1 return 0 } + +export function argumentsToValue any> (fn: F) { + return (...args: Parameters) => fn(...args.map(toValue)) +} diff --git a/test/e2e/issues.ts b/test/e2e/issues.ts index 2beca164f..ad6b47f3b 100644 --- a/test/e2e/issues.ts +++ b/test/e2e/issues.ts @@ -207,4 +207,18 @@ describe('Issues', function () { const html = await engine.render(tpl, { v: undefined }) expect(html).to.equal('') }) + it('#481 filters that should not throw', async () => { + const engine = new Liquid() + const tpl = engine.parse(` + {{ foo | join }} + {{ foo | map: "k" }} + {{ foo | reverse }} + {{ foo | slice: 2 }} + {{ foo | newline_to_br }} + {{ foo | strip_html }} + {{ foo | truncatewords }} + `) + const html = await engine.render(tpl, { foo: undefined }) + expect(html.trim()).to.equal('') + }) }) diff --git a/test/integration/builtin/filters/array.ts b/test/integration/builtin/filters/array.ts index d8f8ca430..7258c0351 100644 --- a/test/integration/builtin/filters/array.ts +++ b/test/integration/builtin/filters/array.ts @@ -28,10 +28,27 @@ describe('filters/array', function () { return expect(render(src)).to.be.rejectedWith('unexpected token at "\\" and \\"", line:1, col:65') }) }) - it('should support split/last', function () { - const src = '{% assign my_array = "zebra, octopus, giraffe, tiger" | split: ", " %}' + - '{{ my_array|last }}' - return test(src, 'tiger') + describe('last', () => { + it('should support last', function () { + const src = '{{ arr | last }}' + const scope = { arr: ['zebra', 'octopus', 'giraffe', 'tiger'] } + return test(src, scope, 'tiger') + }) + }) + describe('split', () => { + it('should support split', function () { + const src = '{% assign my_array = "zebra, octopus, giraffe, tiger" | split: ", " %}' + + '{{ my_array|last }}' + return test(src, 'tiger') + }) + it('should remove trailing empty strings', async () => { + const src = '{{ "zebra,octopus,,,," | split: "," | join: ", " }}' + return test(src, {}, 'zebra, octopus') + }) + it('should return empty array for nil value', async () => { + await test('{{ notDefined | split: "," | size }}', {}, '0') + await test('{{ nil | split: "," | size }}', {}, '0') + }) }) describe('map', () => { it('should support map', function () { @@ -56,6 +73,15 @@ describe('filters/array', function () { return test('{{posts | map: "category" | compact}}', { posts }, 'foobar') }) }) + + describe('concat', () => { + it('should ignore nil left value', async () => { + const scope = { undefinedValue: undefined, nullValue: null, arr: ['foo', 'bar'] } + await test('{{ undefinedValue | concat: arr | join: "," }}', scope, 'foo,bar') + await test('{{ nullValue | concat: arr | join: "," }}', scope, 'foo,bar') + }) + }) + describe('reverse', function () { it('should support reverse', () => test( '{{ "Ground control to Major Tom." | split: "" | reverse | join: "" }}', @@ -118,6 +144,7 @@ describe('filters/array', function () { it('should slice substr by -3,2', () => test('{{ "Liquid" | slice: -3, 2 }}', 'ui')) it('should slice substr by -2,2', () => test('{{ "abc" | slice: -2, 2 }}', 'bc')) it('should support array', () => test('{{ "1,2,3,4" | split: "," | slice: 1,2 | join }}', '2 3')) + it('should return empty array for nil value', () => test('{{ nil | slice: 0 }}', '')) }) describe('sort', function () { it('should support sort', function () { @@ -142,6 +169,54 @@ describe('filters/array', function () { const arr = ['one', 'two', 'three', 'four', 'five'] return test('{{arr | sort}} {{arr}}', { arr }, 'fivefouronethreetwo onetwothreefourfive') }) + it('should return empty array for nil value', () => { + return test('{{notDefined | sort | size}}', {}, '0') + }) + }) + describe('sort_natural', function () { + it('should sort alphabetically', () => { + return test( + '{% assign my_array = "zebra, octopus, giraffe, Sally Snake" | split: ", " %}{{ my_array | sort_natural | join: ", " }}', + 'giraffe, octopus, Sally Snake, zebra' + ) + }) + it('should sort with specified property', () => test( + '{{ students | sort_natural: "name" | map: "name" | join }}', + { students: [{ name: 'bob' }, { name: 'alice' }, { name: 'carol' }] }, + 'alice bob carol' + )) + it('should be stable', () => test( + '{{ students | sort_natural: "age" | map: "name" | join }}', + { students: [{ name: 'bob', age: 1 }, { name: 'alice', age: 1 }, { name: 'carol', age: 1 }] }, + 'bob alice carol' + )) + it('should be stable when it comes to undefined props', () => test( + '{{ students | sort_natural: "age" | map: "name" | join }}', + { students: [{ name: 'bob' }, { name: 'alice', age: 2 }, { name: 'amber' }, { name: 'watson' }, { name: 'michael' }, { name: 'charlie' }] }, + 'alice bob amber watson michael charlie' + )) + it('should tolerate undefined props', () => test( + '{{ students | sort_natural: "age" | map: "name" | join }}', + { students: [{ name: 'bob' }, { name: 'alice', age: 2 }, { name: 'carol' }] }, + 'alice bob carol' + )) + it('should tolerate non array', async () => { + await test( + '{{ students | sort_natural: "age" | map: "name" | join }}', + { students: {} }, + '' + ) + await test( + '{{ students | sort_natural: "age" | map: "name" | size }}', + { students: {} }, + '1' + ) + }) + it('should return empty array for nil value', () => test( + '{{ students | sort_natural: "age" | map: "name" | size }}', + { students: undefined }, + '0' + )) }) describe('uniq', function () { it('should uniq string list', function () { diff --git a/test/integration/builtin/filters/html.ts b/test/integration/builtin/filters/html.ts index f587609a6..4cb2c1cd0 100644 --- a/test/integration/builtin/filters/html.ts +++ b/test/integration/builtin/filters/html.ts @@ -18,6 +18,8 @@ describe('filters/html', function () { test('{{ "1 < 2 & 3" | escape_once }}', '1 < 2 & 3')) it('should not escape twice', () => test('{{ "1 < 2 & 3" | escape_once }}', '1 < 2 & 3')) + it('should escape nil value to empty string', () => + test('{{ undefinedValue | escape_once }}', '')) }) describe('newline_to_br', function () { it('should support string_with_newlines', function () { diff --git a/test/integration/builtin/filters/math.ts b/test/integration/builtin/filters/math.ts index c14c72db5..1d1247a48 100644 --- a/test/integration/builtin/filters/math.ts +++ b/test/integration/builtin/filters/math.ts @@ -60,44 +60,6 @@ describe('filters/math', function () { it('should support variable', () => test('{{ 4 | plus: b }}', { b: 2 }, '6')) }) - describe('sort_natural', function () { - it('should sort alphabetically', () => { - return test( - '{% assign my_array = "zebra, octopus, giraffe, Sally Snake" | split: ", " %}{{ my_array | sort_natural | join: ", " }}', - 'giraffe, octopus, Sally Snake, zebra' - ) - }) - it('should sort with specified property', () => test( - '{{ students | sort_natural: "name" | map: "name" | join }}', - { students: [{ name: 'bob' }, { name: 'alice' }, { name: 'carol' }] }, - 'alice bob carol' - )) - it('should be stable', () => test( - '{{ students | sort_natural: "age" | map: "name" | join }}', - { students: [{ name: 'bob', age: 1 }, { name: 'alice', age: 1 }, { name: 'carol', age: 1 }] }, - 'bob alice carol' - )) - it('should be stable when it comes to undefined props', () => test( - '{{ students | sort_natural: "age" | map: "name" | join }}', - { students: [{ name: 'bob' }, { name: 'alice', age: 2 }, { name: 'amber' }, { name: 'watson' }, { name: 'michael' }, { name: 'charlie' }] }, - 'alice bob amber watson michael charlie' - )) - it('should tolerate undefined props', () => test( - '{{ students | sort_natural: "age" | map: "name" | join }}', - { students: [{ name: 'bob' }, { name: 'alice', age: 2 }, { name: 'carol' }] }, - 'alice bob carol' - )) - it('should tolerate non array', () => test( - '{{ students | sort_natural: "age" | map: "name" | join }}', - { students: {} }, - '' - )) - it('should tolerate falsy input', () => test( - '{{ students | sort_natural: "age" | map: "name" | join }}', - { students: undefined }, - '' - )) - }) describe('round', function () { it('should return "1" for 1.2', () => test('{{1.2|round}}', '1')) it('should return "3" for 2.7', () => test('{{2.7|round}}', '3'))