naman2627 opened a new pull request, #6560:
URL: https://github.com/apache/fineract/pull/6560
## Problem
`DataValidatorBuilder.validateForBooleanValue()` throws a
`NullPointerException` when the value is null and `ignoreIfNull()` was not
called, so validators that pass a null value through it fail with an unhandled
exception instead of a validation error — e.g. `JobDetailDataValidator`
(`{"active": null}` or `{"active": ""}`), which calls
`.notBlank().validateForBooleanValue()`.
## Root cause
The private `validateStringFor(...)` returned early for null only when
`ignoreNullValue` was set; otherwise it proceeded to `this.value.toString()`
(`DataValidatorBuilder.java:962`). If null had been intended as a validation
error, the method would add to `dataValidationErrors`, as its non-null
rejection path does, rather than throw an NPE — so the null case was simply
unhandled. Skipping it matches the class's value-format checks (e.g.
`validateCronExpression`, `inMinMaxRange`, `positiveAmount`), which leave
missing-value reporting to `notNull()`/`notBlank()`; callers that require the
field already chain `notBlank()` first, so reporting null here as well would
duplicate that error.
## Fix
Return early from `validateStringFor` whenever the value is null (one-line
change). The only behavioural change is for a null value without
`ignoreIfNull()`, which previously threw an NPE and is now a no-op; null with
`ignoreIfNull()` and all non-null values behave exactly as before.
`ignoreNullValue` remains in use by the other checks in the class.
## Tests
Added `DataValidatorBuilderBooleanValueTest` (fineract-core) covering
accepted boolean strings, rejection of a non-boolean value, a null value on its
own, and `notBlank()` + `validateForBooleanValue()` on null (expects exactly
one `cannot.be.blank` error). The two null cases fail on develop with the NPE
above and pass with the fix.
## Verification
- `./gradlew :fineract-core:test --no-parallel` — 413 tests, 0 failures, 0
errors, 0 skipped.
- `./gradlew spotlessApply --no-parallel` — no formatting changes beyond
this PR.
- **Before the fix, on a running server (observed):** against a local devRun
instance built before this change, `PUT /v1/jobs/1` with `{"active": null}`
returned **HTTP 500**, and with `{"active": ""}` returned **HTTP 500**.
- **After the fix, on a running server: not yet confirmed.** That these
requests now return HTTP 400 with the `cannot.be.blank` error is verified only
at unit level by the test above; the running instance predates the fix.
- **Not run:** the full `./gradlew build` (runs out of memory on my machine)
and integration tests. The code-value (`isActive`) and entity-datatable-check
(`systemDefined`) paths were not exercised on a running server.
## Related
- Relates to FINERACT-2694 — same defect family (a queued validation error
bypassed by a null dereference, producing a 500 instead of a 400), different
call path (`.intValue()`/`fromInt()`, not `validateForBooleanValue()`).
- FINERACT-1013 (closed "Not A Problem") argued this method is effectively a
no-op because `extractBooleanNamed()` pre-parses via `Boolean.parseBoolean()`,
so invalid strings never reach it. That doesn't apply here:
- It only covers non-null strings: an explicit JSON `null` is a
`JsonNull`, so `extractBooleanNamed()` returns null without parsing, and the
null reaches `this.value.toString()`.
- The job and entity-datatable-check validators use
`extractStringNamed()`, not `extractBooleanNamed()`, so nothing pre-parses: a
value like `"yes"` reaches the method and is rejected, so it is not dead code
on these paths. (The new test confirms the method rejects `"yes"`; that it
arrives unparsed on these paths is from reading the code.)
- `extractStringNamed()` also returns null for `""` and whitespace-only
values. **Observed:** `PUT /v1/jobs/1` with `{"active": ""}` returned **HTTP
500** on a running pre-fix instance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
