Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ var filters = {
},
default: (v, arg) => (isTruthy(v) ? v : arg),
divided_by: (v, arg) => divide(v, arg),
downcase: v => v.toLowerCase(),
downcase: v => (v == null ? '' : String(v).toLowerCase()),
escape: escape,
escape_once: str => escape(unescape(str)),
first: v => v[0],
first: v => (v == null ? undefined : v[0]),
floor: v => Math.floor(v),
Comment on lines +49 to 53

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These nil-safety guards repeat the same v == null ? <fallback> : <expression> pattern across downcase, first, join, last, and size, should we extract a shared nilSafe helper like const nilSafe = (fn, fallback) => v => (v == null ? fallback : fn(v))?

Severity

Want Baz to fix this for you? Activate Fixer

join: (v, arg) => v.join(arg),
last: v => v[v.length - 1],
join: (v, arg) => (v == null ? '' : v.join(arg)),
last: v => (v == null ? undefined : v[v.length - 1]),
lstrip: v => stringify(v).replace(/^\s+/, ""),
map: (arr, arg) => arr.map(v => v[arg]),
sumArray: (arr, key, defaultSum) => sumArray(arr, key, defaultSum),
Expand All @@ -74,7 +74,7 @@ var filters = {
return Math.round(v * amp, arg) / amp;
},
rstrip: str => stringify(str).replace(/\s+$/, ""),
size: v => v.length,
size: v => (v == null ? 0 : v.length),
slice: (v, begin, length) =>
v.substr(begin, length === undefined ? 1 : length),
sort: (v, arg) => v.sort(arg),
Expand Down
1 change: 1 addition & 0 deletions src/scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ var Scope = {
return seq

function push () {
name = (name == null) ? '' : name // nil-safety: bracket accessor can return undefined
if (name.length) seq.push(name)
name = ''
}
Expand Down
31 changes: 31 additions & 0 deletions test/filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -1108,3 +1108,34 @@ describe('filters', function () {
it('should support object', () => test('{{ "a" | obj_test: k1: "v1", k2: "v2" }}', 'a,k1,v1,k2,v2'))
})
})

describe('nil-safe filter guards (SPD-23149)', function () {
const Liquid = require('../index.js')
const engine = Liquid()
const render = (tpl, ctx) => engine.parseAndRender(tpl, ctx)

it('downcase should return empty string for nil input', async function () {
const result = await render('{{ val | downcase }}', { val: null })
expect(result).to.equal('')
})

it('size should return 0 for nil input', async function () {
const result = await render('{{ val | size }}', { val: null })
expect(result).to.equal('0')
})

it('first should return empty for nil input', async function () {
const result = await render('{{ val | first }}', { val: null })
expect(result).to.equal('')
})

it('last should return empty for nil input', async function () {
const result = await render('{{ val | last }}', { val: null })
expect(result).to.equal('')
})

it('join should return empty string for nil input', async function () {
const result = await render('{{ val | join: ", " }}', { val: null })
expect(result).to.equal('')
})
})
17 changes: 17 additions & 0 deletions test/scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -231,3 +231,20 @@ describe('scope', function () {
})
})
})

describe('nil-bracket-variable safety (SPD-23149)', function () {
it('should return empty string for bracket access with undefined variable (not crash)', function () {
// Simulates: unit_mapping[self.sd_product] where self is not in scope
// self.sd_product resolves to undefined; bracket accessor must not throw
var scope = Scope.factory({ unit_mapping: { 'Product A': 'seats' } })
var result = scope.propertyAccessSeq('unit_mapping[self.sd_product]')
// name resolves to undefined → coerced to '' → not pushed → seq = ['unit_mapping']
expect(result).to.deep.equal(['unit_mapping'])
})

it('should resolve bracket access normally when variable is defined', function () {
var scope = Scope.factory({ self: { sd_product: 'Product A' }, unit_mapping: { 'Product A': 'seats' } })
var result = scope.propertyAccessSeq('unit_mapping[self.sd_product]')
expect(result).to.deep.equal(['unit_mapping', 'Product A'])
})
})
115 changes: 115 additions & 0 deletions test/sd-custom/computeColumn-nil-safe.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/**
* Regression tests for SPD-23149:
* computeColumn sd_unit with bracket accessor on nil self.sd_product
* should not crash — mirrors the Wingify/VWO Order Details / sd_unit failure.
*
* Before fix:
* unit_mapping[self.sd_product] where self.sd_product is nil/undefined
* caused: TypeError: Cannot read properties of undefined (reading 'length')
* at push() in scope.js propertyAccessSeq — blocked all document generation.
*
* After fix:
* nil bracket key is coerced to '' → path collapses to ['unit_mapping'] →
* returns the mapping object instead of crashing. downcase/size/first/last/join
* filters also guard nil inputs.
*/
const chai = require('chai')
const expect = chai.expect
const Liquid = require('../../index.js')

describe('computeColumn nil-safe bracket access (SPD-23149)', function () {
let engine

beforeEach(function () {
engine = Liquid({ root: __dirname })
})

it('should compute sd_unit via unit_mapping lookup correctly when sd_product is set', async function () {
const tpl = `
{% computeColumn sd_order_details sd_unit %}
{% parseAssign unit_mapping = '{"VWO Rollouts - Web":"sessions","VWO Testing - Web":"users"}' %}
{% assign $$answer = unit_mapping[self.sd_product] %}
{% endcomputeColumn %}
`
const ctx = {
sd_order_details: [
{ sd_product: 'VWO Rollouts - Web', sd_quota: 100 },
{ sd_product: 'VWO Testing - Web', sd_quota: 50 },
]
}
await engine.parseAndRender(tpl, ctx)
expect(ctx.sd_order_details[0].sd_unit).to.equal('sessions')
expect(ctx.sd_order_details[1].sd_unit).to.equal('users')
})

it('should NOT crash when sd_product is missing from a row (nil bracket-variable safety)', async function () {
const tpl = `
{% computeColumn sd_order_details sd_unit %}
{% parseAssign unit_mapping = '{"VWO Rollouts - Web":"sessions"}' %}
{% assign $$answer = unit_mapping[self.sd_product] %}
{% endcomputeColumn %}
`
const ctx = {
sd_order_details: [
{ sd_quota: 100 } // sd_product intentionally missing
]
}
// Before the fix: TypeError: Cannot read properties of undefined (reading 'length')
// After the fix: no throw; formula completes (value may be the fallback mapping object)
let threw = false
try {
await engine.parseAndRender(tpl, ctx)
} catch (e) {
threw = true
}
expect(threw).to.equal(false)
})

it('should NOT crash when downcase is applied to a nil sd_product field', async function () {
const tpl = `
{% computeColumn sd_order_details sd_unit %}
{% assign lc = self.sd_product | downcase %}
{% if lc == 'vwo rollouts - web' %}
{% assign $$answer = 'sessions' %}
{% else %}
{% assign $$answer = 'unknown' %}
{% endif %}
{% endcomputeColumn %}
`
const ctx = {
sd_order_details: [
{ sd_product: 'VWO Rollouts - Web' },
{} // no sd_product — previously crashed at | downcase on nil
]
}
await engine.parseAndRender(tpl, ctx)
expect(ctx.sd_order_details[0].sd_unit).to.equal('sessions')
expect(ctx.sd_order_details[1].sd_unit).to.equal('unknown')
})

it('should process all rows even when one row has missing fields', async function () {
const tpl = `
{% computeColumn sd_order_details sd_unit %}
{% parseAssign unit_mapping = '{"VWO Rollouts - Web":"sessions","VWO Testing - Web":"users"}' %}
{% assign $$answer = unit_mapping[self.sd_product] %}
{% endcomputeColumn %}
`
const ctx = {
sd_order_details: [
{ sd_product: 'VWO Rollouts - Web', sd_quota: 500 },
{ sd_quota: 100 }, // missing sd_product — should not crash entire table
{ sd_product: 'VWO Testing - Web', sd_quota: 200 },
]
}
let threw = false
try {
await engine.parseAndRender(tpl, ctx)
} catch (e) {
threw = true
}
expect(threw).to.equal(false)
expect(ctx.sd_order_details[0].sd_unit).to.equal('sessions')
// row[1] sd_unit will be the fallback value (whole mapping), not crash
expect(ctx.sd_order_details[2].sd_unit).to.equal('users')
})
})