Skip to content

Remove containers whose contents are never read (java/unused-container CodeQL alerts) - #784

Merged
vharseko merged 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:fix-unused-containers
Jul 29, 2026
Merged

Remove containers whose contents are never read (java/unused-container CodeQL alerts)#784
vharseko merged 1 commit into
OpenIdentityPlatform:masterfrom
vharseko:fix-unused-containers

Conversation

@vharseko

Copy link
Copy Markdown
Member

Clears seven java/unused-container alerts (#688, #689, #690, #691, #693, #694, #695). No behaviour changes.

Removed

AlertContainerWhy it is safe
#695ErrorLogAccountStatusNotificationHandler.NOTIFICATION_TYPE_NAMESFilled by a static initialiser, never read anywhere.
#694NodeSearcherQueue.cancelListHad put/remove calls but no reads. The cancellation itself is done by task.cancel(); this map was bookkeeping nobody consulted.
#693GenerateMessageFileMojo.CATEGORY_DESCRIPTIONS24 put calls, zero reads. The LocalizableMessage import became unused and was dropped with it.
#691OnDiskMergeImporter.importedBaseDNsPopulated inside the phase-two loop, never read.
#690NewIndexPanel.configurationAttrNamesSee below.
#689PropertyValueEditor.addedValuesComputed and discarded, together with the previousValues set that only existed to feed it.
#688DSConfig.getPropHandlersThe only one of the five handler maps that is not passed to SubMenuCallback. The surrounding loop is kept, because it also populates the relations set.

A note on NewIndexPanel

The panel classified attributes three ways — standard, configuration, custom — but only rendered the custom and standard categories, so configuration attributes were silently dropped into a set nobody read.

That looks like an oversight at first glance, but AbstractVLVIndexPanel does exactly the same thing: same three-way split, same two categories in the combo box. Two independent panels behaving identically reads as intent — configuration attributes are not offered for indexing — so this change does not add a third category. It only expresses the existing exclusion directly:

elseif (!Utilities.isConfiguration(attr))
{
// Configuration attributes are not offered for indexing.customAttrNames.add(name);
}

If the intent was in fact to offer them, that is a separate, user-visible change and should be its own PR.

Two alerts deliberately left open

#686SubCommandArgumentParser.globalArgumentMap (java/empty-container) is not dead code, it is unfinished code: the map is read in two places but nothing is ever put into it.

  • hasGlobalArgument(name) always returns false, so the conflict check in SubCommand.addArgument() never fires and a subcommand argument can silently shadow a global one.
  • The duplicate check in addGlobalArgument() never fires either. globalLongIDMap covers part of it, but only on the !longArgumentsCaseSensitive() path.

Fixing it — populating the map, or pointing both reads at globalLongIDMapswitches on two validations that are currently dead. If any tool (dsconfig, base64, ...) has such a name clash today, it would start failing at startup with an ArgumentException. That cannot be settled by reading the code; it needs the CLI tools to be exercised. It deserves its own change rather than riding along with a cleanup.

#692InternalConnection.listeners: addConnectionEventListener()/removeConnectionEventListener() work, but nothing ever iterates the list, so internal connections never notify their listeners of a close, an error or an unsolicited notification. The field cannot simply be deleted without breaking the public Connection contract, and wiring up the notifications is a feature, not a cleanup.

Testing

  • mvn -pl opendj-config,opendj-doc-maven-plugin,opendj-server-legacy compile — BUILD SUCCESS
  • mvn -pl opendj-config test — 544 tests run, 0 failures, 0 errors

CodeQL java/unused-container flags seven collections that are populated
but never read:
* ErrorLogAccountStatusNotificationHandler.NOTIFICATION_TYPE_NAMES
* NodeSearcherQueue.cancelList - cancellation itself is performed by
task.cancel(), this map was pure bookkeeping nobody consulted
* GenerateMessageFileMojo.CATEGORY_DESCRIPTIONS
* OnDiskMergeImporter.importedBaseDNs
* NewIndexPanel.configurationAttrNames
* PropertyValueEditor.addedValues, along with the previousValues set
that only fed it
* DSConfig.getPropHandlers - the only one of the five handler maps that
SubMenuCallback is not given; the surrounding loop is kept because it
also populates the relations set
NewIndexPanel collected configuration attributes into a set that was
never rendered, so they were silently excluded from the attribute combo
box. AbstractVLVIndexPanel does exactly the same, so the exclusion is
intended rather than an oversight; it is now expressed directly instead
of through a discarded collection.
No behaviour changes.
@vharsekovharseko added java Pull requests that update java code security Security fixes / CodeQL code-scanning alerts labels Jul 29, 2026
@vharseko
vharseko merged commit 0cf88d8 into OpenIdentityPlatform:masterJul 29, 2026
17 checks passed
@vharseko
vharseko deleted the fix-unused-containers branch July 29, 2026 14:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

javaPull requests that update java codesecuritySecurity fixes / CodeQL code-scanning alerts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@vharseko@maximthomas