fixed conditional errors with false schemas and unevaluatedProperties

This commit is contained in:
2025-06-12 22:27:49 -04:00
parent 1085964c17
commit cbda45e610
2 changed files with 197 additions and 11 deletions

View File

@ -91,25 +91,40 @@ fn cache_json_schema(schema_id: &str, schema: JsonB, strict: bool) -> JsonB {
}
}
// Helper function to recursively apply strict validation to all objects in a schema
// Helper function to apply strict validation to a schema
//
// This recursively adds unevaluatedProperties: false to object-type schemas,
// but SKIPS schemas inside if/then/else to avoid breaking conditional validation.
fn apply_strict_validation(schema: &mut Value) {
apply_strict_validation_recursive(schema, false);
}
fn apply_strict_validation_recursive(schema: &mut Value, inside_conditional: bool) {
match schema {
Value::Object(map) => {
// If this is an object type schema, add unevaluatedProperties: false
if let Some(Value::String(t)) = map.get("type") {
if t == "object" && !map.contains_key("unevaluatedProperties") && !map.contains_key("additionalProperties") {
map.insert("unevaluatedProperties".to_string(), Value::Bool(false));
// Skip adding strict validation if we're inside a conditional
if !inside_conditional {
// Add strict validation to object schemas only at top level
if let Some(Value::String(t)) = map.get("type") {
if t == "object" && !map.contains_key("unevaluatedProperties") && !map.contains_key("additionalProperties") {
// At top level, use unevaluatedProperties: false
// This considers all evaluated properties from all schemas
map.insert("unevaluatedProperties".to_string(), Value::Bool(false));
}
}
}
// Recurse into all properties
for (_, value) in map.iter_mut() {
apply_strict_validation(value);
for (key, value) in map.iter_mut() {
// Mark when we're inside conditional branches
let in_conditional = inside_conditional || matches!(key.as_str(), "if" | "then" | "else");
apply_strict_validation_recursive(value, in_conditional);
}
}
Value::Array(arr) => {
// Recurse into array items
// Recurse into array items
for item in arr.iter_mut() {
apply_strict_validation(item);
apply_strict_validation_recursive(item, inside_conditional);
}
}
_ => {}
@ -139,7 +154,9 @@ fn validate_json_schema(schema_id: &str, instance: JsonB) -> JsonB {
let mut error_list = Vec::new();
collect_errors(&validation_error, &mut error_list);
let errors = format_errors(error_list, &instance_value, schema_id);
JsonB(json!({ "errors": errors }))
// Filter out FALSE_SCHEMA errors if there are other validation errors
let filtered_errors = filter_false_schema_errors(errors);
JsonB(json!({ "errors": filtered_errors }))
}
}
}
@ -154,6 +171,19 @@ fn collect_errors(error: &ValidationError, errors_list: &mut Vec<Error>) {
ErrorKind::Group | ErrorKind::AllOf | ErrorKind::AnyOf | ErrorKind::Not | ErrorKind::OneOf(_)
);
// Special handling for FalseSchema - if it has causes, use those instead
if matches!(&error.kind, ErrorKind::FalseSchema) {
if !error.causes.is_empty() {
// FalseSchema often wraps more specific errors in if/then conditionals
for cause in &error.causes {
collect_errors(cause, errors_list);
}
return;
}
// If FalseSchema has no causes, it's likely from unevaluatedProperties
// We'll handle it as a leaf error below
}
if error.causes.is_empty() && !is_structural {
// Handle errors with multiple fields specially
match &error.kind {
@ -223,6 +253,21 @@ fn collect_errors(error: &ValidationError, errors_list: &mut Vec<Error>) {
});
}
}
ErrorKind::Pattern { got, want } => {
// Special handling for pattern errors to include the pattern in the message
let display_value = if got.len() > 50 {
format!("{}...", &got[..50])
} else {
got.to_string()
};
errors_list.push(Error {
path: error.instance_location.to_string(),
code: "PATTERN_VIOLATED".to_string(),
message: format!("Value does not match pattern: {}", want),
cause: format!("'{}' does not match pattern {}", display_value, want),
});
}
_ => {
// Handle all other error types normally
let original_message = format!("{}", error.kind);
@ -403,6 +448,32 @@ fn convert_error_kind(kind: &ErrorKind) -> (String, String) {
}
}
// Filter out FALSE_SCHEMA errors if there are other validation errors
fn filter_false_schema_errors(errors: Vec<Value>) -> Vec<Value> {
// Check if there are any non-FALSE_SCHEMA errors
let has_non_false_schema = errors.iter().any(|e| {
e.get("code")
.and_then(|c| c.as_str())
.map(|code| code != "FALSE_SCHEMA")
.unwrap_or(false)
});
if has_non_false_schema {
// Filter out FALSE_SCHEMA errors
errors.into_iter()
.filter(|e| {
e.get("code")
.and_then(|c| c.as_str())
.map(|code| code != "FALSE_SCHEMA")
.unwrap_or(true)
})
.collect()
} else {
// Keep all errors (they're all FALSE_SCHEMA)
errors
}
}
// Formats errors according to DropError structure
fn format_errors(errors: Vec<Error>, instance: &Value, schema_id: &str) -> Vec<Value> {
// Deduplicate by instance_path and format as DropError

View File

@ -335,7 +335,7 @@ fn test_validate_json_schema_oneof_validation_errors() {
let result_empty_obj = validate_json_schema(schema_id, jsonb(invalid_empty_obj));
// Now we expect 2 errors because required fields are split into individual errors
assert_failure_with_json!(result_empty_obj, 2);
let errors_empty = result_empty_obj.0["errors"].as_array().expect("Expected error array for empty object");
let errors_empty = result_empty_obj.0["errors"].as_array().unwrap();
assert_eq!(errors_empty.len(), 2, "Expected two errors for missing required fields");
// Check that we have errors for both missing fields
@ -628,6 +628,121 @@ fn test_auto_strict_validation() {
let result_permissive = validate_json_schema(schema_id_permissive, jsonb(instance_with_extra));
assert_success_with_json!(result_permissive, "Instance with extra property should pass when additionalProperties is explicitly true, even with strict=true");
// Test 7: Schema with conditionals (if/then/else) should NOT add unevaluatedProperties to conditional branches
let schema_id_conditional = "conditional_strict_test";
let conditional_schema = json!({
"type": "object",
"properties": {
"kind": { "type": "string", "enum": ["checking", "savings"] },
"creating": { "type": "boolean" }
},
"if": {
"properties": {
"creating": { "const": true }
}
},
"then": {
"properties": {
"account_number": {
"type": "string",
"pattern": "^[0-9]{4,17}$"
},
"routing_number": {
"type": "string",
"pattern": "^[0-9]{9}$"
}
},
"required": ["account_number", "routing_number"]
}
});
let _ = cache_json_schema(schema_id_conditional, jsonb(conditional_schema), true); // strict=true
// Valid data with properties from both main schema and then clause
let valid_conditional = json!({
"kind": "checking",
"creating": true,
"account_number": "1234567890",
"routing_number": "123456789"
});
let result_conditional = validate_json_schema(schema_id_conditional, jsonb(valid_conditional));
assert_success_with_json!(result_conditional, "Conditional properties should be recognized as evaluated");
// Invalid: extra property not defined anywhere
let invalid_conditional = json!({
"kind": "checking",
"creating": true,
"account_number": "1234567890",
"routing_number": "123456789",
"extra": "not allowed"
});
let result_invalid_conditional = validate_json_schema(schema_id_conditional, jsonb(invalid_conditional));
assert_failure_with_json!(result_invalid_conditional, 1, "Schema validation always fails");
let errors_conditional = result_invalid_conditional.0["errors"].as_array().unwrap();
assert_eq!(errors_conditional[0]["code"], "FALSE_SCHEMA");
assert_eq!(errors_conditional[0]["details"]["path"], "/extra");
// Test the specific edge case: pattern validation failure in a conditional branch
// We filter out FALSE_SCHEMA errors when there are other validation errors
let pattern_failure = json!({
"kind": "checking",
"creating": true,
"account_number": "22", // Too short - will fail pattern validation
"routing_number": "123456789" // Valid, but would be unevaluated - filtered out
});
let result_pattern = validate_json_schema(schema_id_conditional, jsonb(pattern_failure));
let pattern_errors = result_pattern.0["errors"].as_array().unwrap();
// We expect only 1 error: PATTERN_VIOLATED for account_number
// FALSE_SCHEMA for routing_number is filtered out because there's a real validation error
assert_failure_with_json!(result_pattern, 1);
assert_eq!(pattern_errors[0]["code"], "PATTERN_VIOLATED");
assert_eq!(pattern_errors[0]["details"]["path"], "/account_number");
// Test case where both fields have pattern violations
let both_pattern_failures = json!({
"kind": "checking",
"creating": true,
"account_number": "22", // Too short - will fail pattern validation
"routing_number": "123" // Too short - will fail pattern validation
});
let result_both = validate_json_schema(schema_id_conditional, jsonb(both_pattern_failures));
let both_errors = result_both.0["errors"].as_array().unwrap();
// We expect 2 errors: both PATTERN_VIOLATED
assert_failure_with_json!(result_both, 2);
assert!(both_errors.iter().any(|e|
e["code"] == "PATTERN_VIOLATED" &&
e["details"]["path"] == "/account_number"
), "Should have pattern violation for account_number");
assert!(both_errors.iter().any(|e|
e["code"] == "PATTERN_VIOLATED" &&
e["details"]["path"] == "/routing_number"
), "Should have pattern violation for routing_number");
// Test case where there are only FALSE_SCHEMA errors (no other validation errors)
let only_false_schema = json!({
"kind": "checking",
"creating": true,
"account_number": "1234567890", // Valid
"routing_number": "123456789", // Valid
"extra": "not allowed" // Will cause FALSE_SCHEMA
});
let result_only_false = validate_json_schema(schema_id_conditional, jsonb(only_false_schema));
let only_false_errors = result_only_false.0["errors"].as_array().unwrap();
// We expect 1 FALSE_SCHEMA error since there are no other validation errors
assert_failure_with_json!(result_only_false, 1);
assert_eq!(only_false_errors[0]["code"], "FALSE_SCHEMA");
assert_eq!(only_false_errors[0]["details"]["path"], "/extra");
}
#[pg_test]