mirror of
https://github.com/n8n-io/n8n.git
synced 2026-05-12 16:10:30 +02:00
fix(core): Add missing break statements in filter condition evaluation (#27708)
Co-authored-by: Bernhard Wittmann <bernhard.wittmann@n8n.io>
This commit is contained in:
parent
67af2e177d
commit
1e77f7146d
|
|
@ -310,6 +310,8 @@ export function executeFilterCondition(
|
|||
case 'lte':
|
||||
return left <= right;
|
||||
}
|
||||
|
||||
break;
|
||||
}
|
||||
case 'dateTime': {
|
||||
const left = leftValue as DateTime;
|
||||
|
|
@ -339,6 +341,8 @@ export function executeFilterCondition(
|
|||
case 'beforeOrEquals':
|
||||
return left.toMillis() <= right.toMillis();
|
||||
}
|
||||
|
||||
break;
|
||||
}
|
||||
case 'boolean': {
|
||||
const left = leftValue as boolean;
|
||||
|
|
@ -358,6 +362,8 @@ export function executeFilterCondition(
|
|||
case 'notEquals':
|
||||
return left !== right;
|
||||
}
|
||||
|
||||
break;
|
||||
}
|
||||
case 'array': {
|
||||
const left = (leftValue ?? []) as unknown[];
|
||||
|
|
@ -385,6 +391,8 @@ export function executeFilterCondition(
|
|||
case 'notEmpty':
|
||||
return left.length !== 0;
|
||||
}
|
||||
|
||||
break;
|
||||
}
|
||||
case 'object': {
|
||||
const left = leftValue;
|
||||
|
|
@ -395,6 +403,8 @@ export function executeFilterCondition(
|
|||
case 'notEmpty':
|
||||
return !!left && Object.keys(left).length !== 0;
|
||||
}
|
||||
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1331,6 +1331,163 @@ describe('FilterParameter', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('number equals with string coercion (loose validation)', () => {
|
||||
it.each([
|
||||
{ left: '200', right: 200, expected: true },
|
||||
{ left: '0', right: 0, expected: true },
|
||||
{ left: '15.5', right: 15.5, expected: true },
|
||||
{ left: '-1', right: -1, expected: true },
|
||||
{ left: '200', right: 201, expected: false },
|
||||
])(
|
||||
'number:equals(string "$left", $right) === $expected with loose validation',
|
||||
({ left, right, expected }) => {
|
||||
const result = executeFilter(
|
||||
filterFactory({
|
||||
conditions: [
|
||||
{
|
||||
id: '1',
|
||||
leftValue: left,
|
||||
rightValue: right,
|
||||
operator: { operation: 'equals', type: 'number' },
|
||||
},
|
||||
],
|
||||
options: { typeValidation: 'loose' },
|
||||
}),
|
||||
);
|
||||
expect(result).toBe(expected);
|
||||
},
|
||||
);
|
||||
|
||||
it('should route HTTP status code 200 to true branch when comparing number equals', () => {
|
||||
const result = executeFilter(
|
||||
filterFactory({
|
||||
conditions: [
|
||||
{
|
||||
id: '1',
|
||||
leftValue: 200,
|
||||
rightValue: 200,
|
||||
operator: { operation: 'equals', type: 'number' },
|
||||
},
|
||||
],
|
||||
}),
|
||||
);
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('should route HTTP status code as string "200" to true branch with loose validation', () => {
|
||||
const result = executeFilter(
|
||||
filterFactory({
|
||||
conditions: [
|
||||
{
|
||||
id: '1',
|
||||
leftValue: '200',
|
||||
rightValue: 200,
|
||||
operator: { operation: 'equals', type: 'number' },
|
||||
},
|
||||
],
|
||||
options: { typeValidation: 'loose' },
|
||||
}),
|
||||
);
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('should throw on string value with strict validation', () => {
|
||||
expect(() =>
|
||||
executeFilter(
|
||||
filterFactory({
|
||||
conditions: [
|
||||
{
|
||||
id: '1',
|
||||
leftValue: '200',
|
||||
rightValue: 200,
|
||||
operator: { operation: 'equals', type: 'number' },
|
||||
},
|
||||
],
|
||||
options: { typeValidation: 'strict' },
|
||||
}),
|
||||
),
|
||||
).toThrowError();
|
||||
});
|
||||
});
|
||||
|
||||
describe('unknown operator does not fall through between type cases', () => {
|
||||
it('returns false for unknown number operation matching the reporter scenario (NODE-4872)', () => {
|
||||
// The reporter's workflow contained `operation: 'equal'` (legacy name;
|
||||
// the valid name is 'equals'). Without the break after the `number`
|
||||
// case, execution fell through into `dateTime`, `boolean`, `array`,
|
||||
// `object` before reaching the final warn + return false.
|
||||
const run = () =>
|
||||
executeFilter(
|
||||
filterFactory({
|
||||
conditions: [
|
||||
{
|
||||
id: '1',
|
||||
leftValue: 200,
|
||||
rightValue: 200,
|
||||
operator: { operation: 'equal' as never, type: 'number' },
|
||||
},
|
||||
],
|
||||
}),
|
||||
);
|
||||
expect(run).not.toThrow();
|
||||
expect(run()).toBe(false);
|
||||
});
|
||||
|
||||
it('does not call DateTime methods on a number when operation collides with a DateTime op', () => {
|
||||
// `before` is a valid DateTime operation but not a number operation.
|
||||
// Without the break, falls through to the `dateTime` case and calls
|
||||
// `.toMillis()` on a plain number, throwing TypeError.
|
||||
const run = () =>
|
||||
executeFilter(
|
||||
filterFactory({
|
||||
conditions: [
|
||||
{
|
||||
id: '1',
|
||||
leftValue: 100,
|
||||
rightValue: 50,
|
||||
operator: { operation: 'before' as never, type: 'number' },
|
||||
},
|
||||
],
|
||||
}),
|
||||
);
|
||||
expect(run).not.toThrow();
|
||||
expect(run()).toBe(false);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
type: 'dateTime' as const,
|
||||
leftValue: DateTime.fromISO('2025-01-01'),
|
||||
rightValue: DateTime.fromISO('2025-01-01'),
|
||||
},
|
||||
{ type: 'boolean' as const, leftValue: true, rightValue: false },
|
||||
{ type: 'array' as const, leftValue: [1, 2, 3], rightValue: 1 },
|
||||
{ type: 'object' as const, leftValue: { a: 1 }, rightValue: null },
|
||||
])(
|
||||
'returns false for unknown operation on type=$type',
|
||||
({ type, leftValue, rightValue }) => {
|
||||
const run = () =>
|
||||
executeFilter(
|
||||
filterFactory({
|
||||
conditions: [
|
||||
{
|
||||
id: '1',
|
||||
leftValue: leftValue as never,
|
||||
rightValue: rightValue as never,
|
||||
// `rightType: 'any'` skips right-side coercion, which is not
|
||||
// relevant to the fall-through invariant being tested here.
|
||||
operator: { operation: 'bogus' as never, type, rightType: 'any' },
|
||||
},
|
||||
],
|
||||
options: { typeValidation: 'loose' },
|
||||
}),
|
||||
);
|
||||
expect(run).not.toThrow();
|
||||
expect(run()).toBe(false);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe('arrayContainsValue', () => {
|
||||
test('should return true if the array contains the value', () => {
|
||||
expect(arrayContainsValue([1, 2, 3], 2, false)).toBe(true);
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user