fix: some filters throw on nil input, see #481

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": {

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": {}
 }

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<T> (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<T> (this: FilterImpl, arr: T[], property?: string) {
   })
 }
 
+export function sortNatural<T> (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<T> (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<T1, T2> (v: T1[], arg: T2[] | T2): (T1 | T2)[] {
+export function concat<T1, T2> (v: T1[], arg: T2[]): (T1 | T2)[] {
+  v = toValue(v)
   return toArray(v).concat(arg)
 }
 
-export function slice<T> (v: T[], begin: number, length = 1): T[] {
+export function slice<T> (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<T extends object> (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<T extends object> (this: FilterImpl, arr: T[], property: s
 }
 
 export function uniq<T> (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
   })

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)) {

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, '<br />\n')
+  return stringify(v).replace(/\n/g, '<br />\n')
 }
 
 export function stripHtml (v: string) {
-  return v.replace(/<script.*?<\/script>|<!--.*?-->|<style.*?<\/style>|<.*?>/g, '')
+  return stringify(v).replace(/<script.*?<\/script>|<!--.*?-->|<style.*?<\/style>|<.*?>/g, '')
 }

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)
-}

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<T1 extends boolean, T2> (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
 }

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

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 ]
 }

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 <T> (
 ) {
   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<F extends (...args: any) => any> (fn: F) {
+  return (...args: Parameters<F>) => fn(...args.map(toValue))
+}

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('')
+  })
 })

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 () {

test/integration/builtin/filters/html.ts

@@ -18,6 +18,8 @@ describe('filters/html', function () {
       test('{{ "1 < 2 & 3" | escape_once }}', '1 &lt; 2 &amp; 3'))
     it('should not escape twice',
       () => test('{{ "1 &lt; 2 &amp; 3" | escape_once }}', '1 &lt; 2 &amp; 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 () {

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'))