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

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

Alert Container Why it is safe
#695 ErrorLogAccountStatusNotificationHandler.NOTIFICATION_TYPE_NAMES Filled by a static initialiser, never read anywhere.
#694 NodeSearcherQueue.cancelList Had put/remove calls but no reads. The cancellation itself is done by task.cancel(); this map was bookkeeping nobody consulted.
#693 GenerateMessageFileMojo.CATEGORY_DESCRIPTIONS 24 put calls, zero reads. The LocalizableMessage import became unused and was dropped with it.
#691 OnDiskMergeImporter.importedBaseDNs Populated inside the phase-two loop, never read.
#690 NewIndexPanel.configurationAttrNames See below.
#689 PropertyValueEditor.addedValues Computed and discarded, together with the previousValues set that only existed to feed it.
#688 DSConfig.getPropHandlers The 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:

else if (!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

#686 — SubCommandArgumentParser.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 globalLongIDMap — switches 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.

#692 — InternalConnection.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.
@vharseko
vharseko requested a review from maximthomas July 29, 2026 07:49
@vharseko vharseko added java security Security fixes / CodeQL code-scanning alerts labels Jul 29, 2026
@vharseko
vharseko merged commit 0cf88d8 into OpenIdentityPlatform:master Jul 29, 2026
17 checks passed
@vharseko
vharseko deleted the fix-unused-containers branch July 29, 2026 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security Security fixes / CodeQL code-scanning alerts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants