[jira] [Assigned] (FINERACT-2709) 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

["Adam Saghy (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/FINERACT-2709?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Adam Saghy reassigned FINERACT-2709:
------------------------------------

    Assignee: Farooq Ayoade

> 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
>
> ---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)