Skip to content

Commit

Permalink
Fix NaN handling in ordered checks
Browse files Browse the repository at this point in the history
  • Loading branch information
@Alfus
Alfus committed Aug 24, 2023
1 parent 12a6c81 commit 9b21c01
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 24 deletions.
48 changes: 24 additions & 24 deletions proto/protovalidate/buf/validate/validate.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ message FloatRules {
float lt = 2 [(priv.field).cel = {
id: "float.lt",
expression:
"!has(rules.gte) && !has(rules.gt) && this >= rules.lt"
"!has(rules.gte) && !has(rules.gt) && (this.isNan() || this >= rules.lt)"
"? 'value must be less than %s'.format([rules.lt]) : ''"
}];

Expand All @@ -241,7 +241,7 @@ message FloatRules {
float lte = 3 [(priv.field).cel = {
id: "float.lte",
expression:
"!has(rules.gte) && !has(rules.gt) && this > rules.lte"
"!has(rules.gte) && !has(rules.gt) && (this.isNan() || this > rules.lte)"
"? 'value must be less than or equal to %s'.format([rules.lte]) : ''"
}];
}
Expand Down Expand Up @@ -269,31 +269,31 @@ message FloatRules {
(priv.field).cel = {
id: "float.gt",
expression:
"!has(rules.lt) && !has(rules.lte) && this <= rules.gt"
"!has(rules.lt) && !has(rules.lte) && (this.isNan() || this <= rules.gt)"
"? 'value must be greater than %s'.format([rules.gt]) : ''"
},
(priv.field).cel = {
id: "float.gt_lt",
expression:
"has(rules.lt) && rules.lt >= rules.gt && (this >= rules.lt || this <= rules.gt)"
"has(rules.lt) && rules.lt >= rules.gt && (this.isNan() || this >= rules.lt || this <= rules.gt)"
"? 'value must be greater than %s and less than %s'.format([rules.gt, rules.lt]) : ''"
},
(priv.field).cel = {
id: "float.gt_lt_exclusive",
expression:
"has(rules.lt) && rules.lt < rules.gt && (rules.lt <= this && this <= rules.gt)"
"has(rules.lt) && rules.lt < rules.gt && (this.isNan() || (rules.lt <= this && this <= rules.gt))"
"? 'value must be greater than %s or less than %s'.format([rules.gt, rules.lt]) : ''"
},
(priv.field).cel = {
id: "float.gt_lte",
expression:
"has(rules.lte) && rules.lte >= rules.gt && (this > rules.lte || this <= rules.gt)"
"has(rules.lte) && rules.lte >= rules.gt && (this.isNan() || this > rules.lte || this <= rules.gt)"
"? 'value must be greater than %s and less than or equal to %s'.format([rules.gt, rules.lte]) : ''"
},
(priv.field).cel = {
id: "float.gt_lte_exclusive",
expression:
"has(rules.lte) && rules.lte < rules.gt && (rules.lte < this && this <= rules.gt)"
"has(rules.lte) && rules.lte < rules.gt && (this.isNan() || (rules.lte < this && this <= rules.gt))"
"? 'value must be greater than %s or less than or equal to %s'.format([rules.gt, rules.lte]) : ''"
}
];
Expand All @@ -320,31 +320,31 @@ message FloatRules {
(priv.field).cel = {
id: "float.gte",
expression:
"!has(rules.lt) && !has(rules.lte) && this < rules.gte"
"!has(rules.lt) && !has(rules.lte) && (this.isNan() || this < rules.gte)"
"? 'value must be greater than or equal to %s'.format([rules.gte]) : ''"
},
(priv.field).cel = {
id: "float.gte_lt",
expression:
"has(rules.lt) && rules.lt >= rules.gte && (this >= rules.lt || this < rules.gte)"
"has(rules.lt) && rules.lt >= rules.gte && (this.isNan() || this >= rules.lt || this < rules.gte)"
"? 'value must be greater than or equal to %s and less than %s'.format([rules.gte, rules.lt]) : ''"
},
(priv.field).cel = {
id: "float.gte_lt_exclusive",
expression:
"has(rules.lt) && rules.lt < rules.gte && (rules.lt <= this && this < rules.gte)"
"has(rules.lt) && rules.lt < rules.gte && (this.isNan() || (rules.lt <= this && this < rules.gte))"
"? 'value must be greater than or equal to %s or less than %s'.format([rules.gte, rules.lt]) : ''"
},
(priv.field).cel = {
id: "float.gte_lte",
expression:
"has(rules.lte) && rules.lte >= rules.gte && (this > rules.lte || this < rules.gte)"
"has(rules.lte) && rules.lte >= rules.gte && (this.isNan() || this > rules.lte || this < rules.gte)"
"? 'value must be greater than or equal to %s and less than or equal to %s'.format([rules.gte, rules.lte]) : ''"
},
(priv.field).cel = {
id: "float.gte_lte_exclusive",
expression:
"has(rules.lte) && rules.lte < rules.gte && (rules.lte < this && this < rules.gte)"
"has(rules.lte) && rules.lte < rules.gte && (this.isNan() || (rules.lte < this && this < rules.gte))"
"? 'value must be greater than or equal to %s or less than or equal to %s'.format([rules.gte, rules.lte]) : ''"
}
];
Expand Down Expand Up @@ -418,7 +418,7 @@ message DoubleRules {
double lt = 2 [(priv.field).cel = {
id: "double.lt",
expression:
"!has(rules.gte) && !has(rules.gt) && this >= rules.lt"
"!has(rules.gte) && !has(rules.gt) && (this.isNan() || this >= rules.lt)"
"? 'value must be less than %s'.format([rules.lt]) : ''"
}];

Expand All @@ -435,7 +435,7 @@ message DoubleRules {
double lte = 3 [(priv.field).cel = {
id: "double.lte",
expression:
"!has(rules.gte) && !has(rules.gt) && this > rules.lte"
"!has(rules.gte) && !has(rules.gt) && (this.isNan() || this > rules.lte)"
"? 'value must be less than or equal to %s'.format([rules.lte]) : ''"
}];
}
Expand All @@ -462,31 +462,31 @@ message DoubleRules {
(priv.field).cel = {
id: "double.gt",
expression:
"!has(rules.lt) && !has(rules.lte) && this <= rules.gt"
"!has(rules.lt) && !has(rules.lte) && (this.isNan() || this <= rules.gt)"
"? 'value must be greater than %s'.format([rules.gt]) : ''"
},
(priv.field).cel = {
id: "double.gt_lt",
expression:
"has(rules.lt) && rules.lt >= rules.gt && (this >= rules.lt || this <= rules.gt)"
"has(rules.lt) && rules.lt >= rules.gt && (this.isNan() || this >= rules.lt || this <= rules.gt)"
"? 'value must be greater than %s and less than %s'.format([rules.gt, rules.lt]) : ''"
},
(priv.field).cel = {
id: "double.gt_lt_exclusive",
expression:
"has(rules.lt) && rules.lt < rules.gt && (rules.lt <= this && this <= rules.gt)"
"has(rules.lt) && rules.lt < rules.gt && (this.isNan() || (rules.lt <= this && this <= rules.gt))"
"? 'value must be greater than %s or less than %s'.format([rules.gt, rules.lt]) : ''"
},
(priv.field).cel = {
id: "double.gt_lte",
expression:
"has(rules.lte) && rules.lte >= rules.gt && (this > rules.lte || this <= rules.gt)"
"has(rules.lte) && rules.lte >= rules.gt && (this.isNan() || this > rules.lte || this <= rules.gt)"
"? 'value must be greater than %s and less than or equal to %s'.format([rules.gt, rules.lte]) : ''"
},
(priv.field).cel = {
id: "double.gt_lte_exclusive",
expression:
"has(rules.lte) && rules.lte < rules.gt && (rules.lte < this && this <= rules.gt)"
"has(rules.lte) && rules.lte < rules.gt && (this.isNan() || (rules.lte < this && this <= rules.gt))"
"? 'value must be greater than %s or less than or equal to %s'.format([rules.gt, rules.lte]) : ''"
}
];
Expand All @@ -513,31 +513,31 @@ message DoubleRules {
(priv.field).cel = {
id: "double.gte",
expression:
"!has(rules.lt) && !has(rules.lte) && this < rules.gte"
"!has(rules.lt) && !has(rules.lte) && (this.isNan() || this < rules.gte)"
"? 'value must be greater than or equal to %s'.format([rules.gte]) : ''"
},
(priv.field).cel = {
id: "double.gte_lt",
expression:
"has(rules.lt) && rules.lt >= rules.gte && (this >= rules.lt || this < rules.gte)"
"has(rules.lt) && rules.lt >= rules.gte && (this.isNan() || this >= rules.lt || this < rules.gte)"
"? 'value must be greater than or equal to %s and less than %s'.format([rules.gte, rules.lt]) : ''"
},
(priv.field).cel = {
id: "double.gte_lt_exclusive",
expression:
"has(rules.lt) && rules.lt < rules.gte && (rules.lt <= this && this < rules.gte)"
"has(rules.lt) && rules.lt < rules.gte && (this.isNan() || (rules.lt <= this && this < rules.gte))"
"? 'value must be greater than or equal to %s or less than %s'.format([rules.gte, rules.lt]) : ''"
},
(priv.field).cel = {
id: "double.gte_lte",
expression:
"has(rules.lte) && rules.lte >= rules.gte && (this > rules.lte || this < rules.gte)"
"has(rules.lte) && rules.lte >= rules.gte && (this.isNan() || this > rules.lte || this < rules.gte)"
"? 'value must be greater than or equal to %s and less than or equal to %s'.format([rules.gte, rules.lte]) : ''"
},
(priv.field).cel = {
id: "double.gte_lte_exclusive",
expression:
"has(rules.lte) && rules.lte < rules.gte && (rules.lte < this && this < rules.gte)"
"has(rules.lte) && rules.lte < rules.gte && (this.isNan() || (rules.lte < this && this < rules.gte))"
"? 'value must be greater than or equal to %s or less than or equal to %s'.format([rules.gte, rules.lte]) : ''"
}
];
Expand Down
49 changes: 49 additions & 0 deletions tools/protovalidate-conformance/internal/cases/cases_double.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ func doubleSuite() suites.Suite {
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.const"}),
},
"const/invalid_nan": {
Message: &cases.DoubleConst{Val: math.NaN()},
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.const"}),
},
"in/valid": {
Message: &cases.DoubleIn{Val: 7.89},
Expected: results.Success(true),
Expand All @@ -47,10 +52,19 @@ func doubleSuite() suites.Suite {
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.in"}),
},
"in/invalid_nan": {
Message: &cases.DoubleIn{Val: math.NaN()},
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.in"}),
},
"not_in/valid": {
Message: &cases.DoubleNotIn{Val: 1},
Expected: results.Success(true),
},
"not_in/valid_nan": {
Message: &cases.DoubleNotIn{Val: math.NaN()},
Expected: results.Success(true),
},
"not_in/invalid": {
Message: &cases.DoubleNotIn{Val: 0},
Expected: results.Violations(
Expand All @@ -70,6 +84,11 @@ func doubleSuite() suites.Suite {
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.lt"}),
},
"lt/invalid/nan": {
Message: &cases.DoubleLT{Val: math.NaN()},
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.lt"}),
},
"lte/valid/less": {
Message: &cases.DoubleLTE{Val: 63},
Expected: results.Success(true),
Expand All @@ -83,6 +102,11 @@ func doubleSuite() suites.Suite {
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.lte"}),
},
"lte/invalid/nan": {
Message: &cases.DoubleLTE{Val: math.NaN()},
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.lte"}),
},
"gt/valid/greater": {
Message: &cases.DoubleGT{Val: 17},
Expected: results.Success(true),
Expand All @@ -97,6 +121,11 @@ func doubleSuite() suites.Suite {
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.gt"}),
},
"gt/invalid/nan": {
Message: &cases.DoubleGT{Val: math.NaN()},
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.gt"}),
},
"gte/valid/greater": {
Message: &cases.DoubleGTE{Val: 9},
Expected: results.Success(true),
Expand All @@ -110,6 +139,11 @@ func doubleSuite() suites.Suite {
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.gte"}),
},
"gte/invalid/nan": {
Message: &cases.DoubleGTE{Val: math.NaN()},
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.gte"}),
},
"gt_lt/inclusive/valid/within": {
Message: &cases.DoubleGTLT{Val: 5},
Expected: results.Success(true),
Expand Down Expand Up @@ -157,6 +191,11 @@ func doubleSuite() suites.Suite {
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.gt_lt_exclusive"}),
},
"gt_lt/exclusive/invalid/nan": {
Message: &cases.DoubleExLTGT{Val: math.NaN()},
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.gt_lt_exclusive"}),
},
"gte_lte/inclusive/valid/within": {
Message: &cases.DoubleGTELTE{Val: 200},
Expected: results.Success(true),
Expand All @@ -179,6 +218,11 @@ func doubleSuite() suites.Suite {
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.gte_lte"}),
},
"gte_lte/inclusive/invalid/nan": {
Message: &cases.DoubleGTELTE{Val: math.NaN()},
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.gte_lte"}),
},
"gte_lte/exclusive/valid/above": {
Message: &cases.DoubleExGTELTE{Val: 300},
Expected: results.Success(true),
Expand All @@ -200,6 +244,11 @@ func doubleSuite() suites.Suite {
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.gte_lte_exclusive"}),
},
"gte_lte/exclusive/invalid/nan": {
Message: &cases.DoubleExGTELTE{Val: math.NaN()},
Expected: results.Violations(
&validate.Violation{FieldPath: "val", ConstraintId: "double.gte_lte_exclusive"}),
},
"finite/valid": {
Message: &cases.DoubleFinite{Val: 1},
Expected: results.Success(true),
Expand Down
Loading

0 comments on commit 9b21c01

Please sign in to comment.