[
https://issues.apache.org/jira/browse/FINERACT-2709?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Adam Saghy resolved FINERACT-2709.
----------------------------------
Resolution: Fixed
> 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
> Assignee: Farooq Ayoade
> Priority: Minor
> Fix For: 1.16.0
>
>
> ---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)
