Skip to content
Merged
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
1 change: 1 addition & 0 deletions changelog.d/evaluate-condition-fail-closed.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Unevaluable validation `condition` / `unless` expressions now throw `Wheels.InvalidValidationCondition` even when `showErrorInformation` is false, instead of silently skipping the validation (#3398)
19 changes: 7 additions & 12 deletions vendor/wheels/model/validations.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -580,19 +580,14 @@ component {
try {
local[local.key] = $evaluateConditionString(arguments[local.item]);
} catch (any e) {
if ($get("showErrorInformation")) {
Throw(
type = "Wheels.InvalidValidationCondition",
message = "The `#local.item#` expression `#arguments[local.item]#` could not be evaluated: #e.message#",
extendedInfo = "Supported forms: `this.property`, `this.method()` (with named `key='val'` or positional `'val'` arguments), bare `method()` (optionally negated with `!`), and binary comparisons using eq/neq/lt/lte/gt/gte or ==/!=/</<=/>/>=."
);
}
cflog(
text = "Wheels: validation `#local.item#` expression `#arguments[local.item]#` could not be evaluated (#e.message#); validation skipped.",
type = "error",
file = "wheels-errors"
// Fail closed regardless of showErrorInformation. Returning
// false here means "skip this validation" ($validate), which
// is fail-open for a broken condition/unless expression.
Throw(
type = "Wheels.InvalidValidationCondition",
message = "The `#local.item#` expression `#arguments[local.item]#` could not be evaluated: #e.message#",
extendedInfo = "Supported forms: `this.property`, `this.method()` (with named `key='val'` or positional `'val'` arguments), bare `method()` (optionally negated with `!`), and binary comparisons using eq/neq/lt/lte/gt/gte or ==/!=/</<=/>/>=."
);
return false;
}
}
}
Expand Down
25 changes: 23 additions & 2 deletions vendor/wheels/tests/specs/model/validationsSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -310,13 +310,34 @@ component extends="wheels.WheelsTest" {
expect(callValid).toThrow("Wheels.InvalidValidationCondition")
})

it("skips the validation without throwing in production when a condition cannot be evaluated", () => {
// M1: $evaluateCondition must fail closed when the expression cannot
// be evaluated. Returning false here means "skip this validation"
// ($validate: if ($evaluateCondition(...))), which is fail-open.
it("throws when a condition cannot be evaluated even if showErrorInformation is false", () => {
args.condition = "noSuchMethod()"
user.validatesLengthOf(argumentCollection = args)
var originalShowErrorInformation = application.wheels.showErrorInformation
try {
application.wheels.showErrorInformation = false
expect(user.valid()).toBeTrue()
var callValid = () => {
user.valid()
}
expect(callValid).toThrow("Wheels.InvalidValidationCondition")
} finally {
application.wheels.showErrorInformation = originalShowErrorInformation
}
})

it("throws when an unless expression cannot be evaluated even if showErrorInformation is false", () => {
args.unless = "noSuchMethod()"
user.validatesLengthOf(argumentCollection = args)
var originalShowErrorInformation = application.wheels.showErrorInformation
try {
application.wheels.showErrorInformation = false
var callValid = () => {
user.valid()
}
expect(callValid).toThrow("Wheels.InvalidValidationCondition")
} finally {
application.wheels.showErrorInformation = originalShowErrorInformation
}
Expand Down