Farooq Ayoade created FINERACT-2709:
---------------------------------------
Summary: Bulk-import chart-of-accounts download template:
setNames() null-checks the wrong variable, so the Tags named-range guard is
dead and a null lookup-index entry would NullPointerException
Key: FINERACT-2709
URL: https://issues.apache.org/jira/browse/FINERACT-2709
Project: Apache Fineract
Issue Type: Bug
Components: DataImportTool
Reporter: Farooq Ayoade
---Observed behavior
In ChartOfAccountsWorkbook.setNames(), the block that builds the
per-account-type Tags named range guards the wrong
variable:
{code:java}
Integer[] tagValueBeginEndIndexes =
accountTypeToBeginEndIndexesofAccountNames.get(i); // may return null
if (accountTypeToBeginEndIndexesofAccountNames != null) {
// guards the MAP, not the array
setSanitized(tags, "Tags_" + accountTypesNoDuplicatesList.get(i));
tags.setRefersToFormula(... + tagValueBeginEndIndexes[0] + ... +
tagValueBeginEndIndexes[1]); // NPE if the entry is null
}
{code}
accountTypeToBeginEndIndexesofAccountNames is an instance field that was
already dereferenced on the line above
(.get(i)); it is never null at this point, so the guard is always true and
protects nothing. The value that can actually
be null is tagValueBeginEndIndexes (a Map.get miss), and it is dereferenced
unguarded at tagValueBeginEndIndexes[0].
The very next block — building the AccountName named range — demonstrates the
intended pattern and guards correctly:
{code:java}
Integer[] accountNamesBeginEndIndexes =
accountTypeToBeginEndIndexesofAccountNames.get(i);
if (accountNamesBeginEndIndexes != null) { // correct: guards the array it
dereferences
...
}
{code}
Expected behavior
The Tags block guards the same variable it dereferences
(tagValueBeginEndIndexes), matching the sibling AccountName
block, so a missing lookup-index entry simply skips the Tags named range
instead of risking an NPE.
Reachability
With well-formed GL-account data the null branch is not reached:
accountTypesNoDuplicatesList and the index map are both
derived from the same glAccounts, and setLookupTable() only skips a put
(leaving a null entry) for a type whose
account-name list is empty — which cannot happen for a type that came from an
actual account. This is therefore a dead
guard / latent NPE, not a reproducible HTTP 500. The fix is a low-risk
correctness cleanup that makes the guard do what it
already appears to do.
Root cause
Copy-paste of the AccountName block into the Tags block without switching the
guarded variable from the map to the
per-iteration array.
Proposed fix
{code:java}
Integer[] tagValueBeginEndIndexes =
accountTypeToBeginEndIndexesofAccountNames.get(i);
if (tagValueBeginEndIndexes != null) { // was:
accountTypeToBeginEndIndexesofAccountNames != null
setSanitized(tags, "Tags_" + accountTypesNoDuplicatesList.get(i));
tags.setRefersToFormula(... tagValueBeginEndIndexes[0] ...
tagValueBeginEndIndexes[1]);
}
{code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
