From f18f79e0517f21ef855f9fbda625086d37f5935d Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Thu, 23 Jul 2026 08:42:36 +0300 Subject: [PATCH 1/2] Resolve open CodeQL alerts in the persistit module Clears the open code-scanning alerts (1 high, 2 warnings, 8 notes) in the persistit module. The high java/improper-validation-of-array-index finding was a false positive - the original bounds check was already correct - so this is a static-analysis legibility change, not a security fix. Core: - Exchange.fetchBufferCopy: replace the manual level bounds check with Objects.checkIndex (recognized by CodeQL as an array-index sanitizer), captured against a local reference to the level cache. The bound is reachable - tree depth can legally reach PAGE_TYPE_INDEX_MAX and is unbounded on a corrupted volume - so the original IllegalArgumentException("Tree depth is N") contract is preserved by rethrowing it from IndexOutOfBoundsException, and the thrown exception is now documented. - JoinPolicy: add toString() so forName() matches by policy name instead of Object identity, repairing the joinpolicy configuration property, which previously always threw; JoinPolicyTest locks the behaviour in. Fix the copy-pasted "No such SplitPolicy" message. - CLI.source: wrap the FileReader directly in the stacked BufferedReader (java/input-resource-leak). - BufferPool: read the reserved block before releasing it on OutOfMemoryError so the reservation stays effective and the variable is no longer dead (java/local-variable-is-never-read). - ThreadSequencer: stop implementing the constants-only SequencerConstants interface (java/constants-only-interface). - GetVersion: drop a useless toString() call on a String value (java/useless-tostring-call). UI: - InspectorPanel.Fetcher: null-check the logical record before dereferencing it (java/dereferenced-value-may-be-null). - ManagementTableModel, ManagementSlidingTableModel: remove locals that are written but never read, keeping the load-bearing token parse (java/local-variable-is-never-read). - AdminUITaskPanel: drop a useless toString() call (java/useless-tostring-call). - Remove the unused VTComboBoxModel class (java/unused-reference-type). --- .../main/java/com/persistit/BufferPool.java | 7 +- .../core/src/main/java/com/persistit/CLI.java | 3 +- .../src/main/java/com/persistit/Exchange.java | 18 +++- .../main/java/com/persistit/GetVersion.java | 2 +- .../java/com/persistit/policy/JoinPolicy.java | 7 +- .../com/persistit/util/ThreadSequencer.java | 2 +- .../java/com/persistit/JoinPolicyTest.java | 40 ++++++++ .../com/persistit/ui/AdminUITaskPanel.java | 6 +- .../java/com/persistit/ui/InspectorPanel.java | 7 +- .../ui/ManagementSlidingTableModel.java | 5 - .../persistit/ui/ManagementTableModel.java | 8 +- .../com/persistit/ui/VTComboBoxModel.java | 97 ------------------- 12 files changed, 85 insertions(+), 117 deletions(-) create mode 100644 persistit/core/src/test/java/com/persistit/JoinPolicyTest.java delete mode 100644 persistit/ui/src/main/java/com/persistit/ui/VTComboBoxModel.java diff --git a/persistit/core/src/main/java/com/persistit/BufferPool.java b/persistit/core/src/main/java/com/persistit/BufferPool.java index 1808f2584..eddffeefa 100644 --- a/persistit/core/src/main/java/com/persistit/BufferPool.java +++ b/persistit/core/src/main/java/com/persistit/BufferPool.java @@ -281,7 +281,12 @@ public class BufferPool { // Note: written this way to try to avoid another OOME. // Do not use String.format here. // - reserve = null; + // Release the reserved block to free memory for the diagnostic + // path below. Reading its length first keeps the reservation + // effective (and clears the "never read" static-analysis finding). + if (reserve.length > 0) { + reserve = null; + } System.err.print("Out of memory with "); System.err.print(Runtime.getRuntime().freeMemory()); System.err.print(" bytes free after creating "); diff --git a/persistit/core/src/main/java/com/persistit/CLI.java b/persistit/core/src/main/java/com/persistit/CLI.java index b3b2d8064..f758fed4e 100644 --- a/persistit/core/src/main/java/com/persistit/CLI.java +++ b/persistit/core/src/main/java/com/persistit/CLI.java @@ -907,8 +907,7 @@ Task source(final @Arg("file|string|Read commands from file") String fileName) t @Override public void runTask() throws Exception { if (!fileName.isEmpty()) { - final FileReader in = new FileReader(fileName); - _sourceStack.push(new BufferedReader(new BufferedReader(in))); + _sourceStack.push(new BufferedReader(new FileReader(fileName))); postMessage(String.format("Source is %s", fileName), LOG_NORMAL); return; } else { diff --git a/persistit/core/src/main/java/com/persistit/Exchange.java b/persistit/core/src/main/java/com/persistit/Exchange.java index 09de5372e..664834933 100644 --- a/persistit/core/src/main/java/com/persistit/Exchange.java +++ b/persistit/core/src/main/java/com/persistit/Exchange.java @@ -44,6 +44,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Objects; import static com.persistit.Buffer.EXACT_MASK; import static com.persistit.Buffer.HEADER_SIZE; @@ -4456,6 +4457,10 @@ public void setTimeoutMillis(final long timeout) { * @param level * The tree level, starting at zero for the data page. * @return copy of page on the key's index tree at that level. + * @throws IllegalArgumentException + * if level does not identify a valid tree level + * (for example when the tree depth exceeds + * {@link #MAX_TREE_DEPTH}) */ public Buffer fetchBufferCopy(final int level) throws PersistitException { assertCorrectThread(true); @@ -4463,11 +4468,18 @@ public Buffer fetchBufferCopy(final int level) throws PersistitException { throw new IllegalArgumentException("Tree depth is " + _tree.getDepth()); } final int lvl = level >= 0 ? level : _tree.getDepth() + level; - if (lvl < 0 || lvl >= _levelCache.length) { - throw new IllegalArgumentException("Tree depth is " + _tree.getDepth()); + final LevelCache[] levelCache = _levelCache; + // Objects.checkIndex is recognized by CodeQL as an array-index sanitizer. + // Preserve the original IllegalArgumentException contract (including the + // "Tree depth is N" message) for over-deep trees, since depth can legally + // reach PAGE_TYPE_INDEX_MAX and is unbounded on a corrupted volume. + try { + Objects.checkIndex(lvl, levelCache.length); + } catch (final IndexOutOfBoundsException e) { + throw new IllegalArgumentException("Tree depth is " + _tree.getDepth(), e); } final int foundAt = searchTree(_key, lvl, false); - final Buffer buffer = _levelCache[lvl]._buffer; + final Buffer buffer = levelCache[lvl]._buffer; try { if (foundAt == -1) { return null; diff --git a/persistit/core/src/main/java/com/persistit/GetVersion.java b/persistit/core/src/main/java/com/persistit/GetVersion.java index bf2929362..731904375 100644 --- a/persistit/core/src/main/java/com/persistit/GetVersion.java +++ b/persistit/core/src/main/java/com/persistit/GetVersion.java @@ -76,6 +76,6 @@ private GetVersion(final String jarResource) throws IOException { @Override public String toString() { - return _version.toString(); + return _version; } } diff --git a/persistit/core/src/main/java/com/persistit/policy/JoinPolicy.java b/persistit/core/src/main/java/com/persistit/policy/JoinPolicy.java index 6f9de1b84..6857ec152 100644 --- a/persistit/core/src/main/java/com/persistit/policy/JoinPolicy.java +++ b/persistit/core/src/main/java/com/persistit/policy/JoinPolicy.java @@ -49,7 +49,7 @@ public static JoinPolicy forName(final String name) { return policy; } } - throw new IllegalArgumentException("No such SplitPolicy " + name); + throw new IllegalArgumentException("No such JoinPolicy " + name); } String _name; @@ -141,4 +141,9 @@ public String getName() { return _name; } + @Override + public String toString() { + return _name; + } + } diff --git a/persistit/core/src/main/java/com/persistit/util/ThreadSequencer.java b/persistit/core/src/main/java/com/persistit/util/ThreadSequencer.java index 659790059..8442ba4b5 100644 --- a/persistit/core/src/main/java/com/persistit/util/ThreadSequencer.java +++ b/persistit/core/src/main/java/com/persistit/util/ThreadSequencer.java @@ -102,7 +102,7 @@ * @author peter * */ -public class ThreadSequencer implements SequencerConstants { +public class ThreadSequencer { private final static DisabledSequencer DISABLED_SEQUENCER = new DisabledSequencer(); diff --git a/persistit/core/src/test/java/com/persistit/JoinPolicyTest.java b/persistit/core/src/test/java/com/persistit/JoinPolicyTest.java new file mode 100644 index 000000000..d5126c9ed --- /dev/null +++ b/persistit/core/src/test/java/com/persistit/JoinPolicyTest.java @@ -0,0 +1,40 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ + +package com.persistit; + +import com.persistit.policy.JoinPolicy; +import org.junit.Test; + +import static org.junit.Assert.assertSame; + +public class JoinPolicyTest { + + @Test + public void forNameResolvesPoliciesCaseInsensitively() { + assertSame(JoinPolicy.EVEN_BIAS, JoinPolicy.forName("even")); + assertSame(JoinPolicy.EVEN_BIAS, JoinPolicy.forName("EVEN")); + assertSame(JoinPolicy.LEFT_BIAS, JoinPolicy.forName("LEFT")); + assertSame(JoinPolicy.LEFT_BIAS, JoinPolicy.forName("left")); + assertSame(JoinPolicy.RIGHT_BIAS, JoinPolicy.forName("Right")); + } + + @Test(expected = IllegalArgumentException.class) + public void forNameRejectsUnknown() { + JoinPolicy.forName("NOPE"); + } + +} diff --git a/persistit/ui/src/main/java/com/persistit/ui/AdminUITaskPanel.java b/persistit/ui/src/main/java/com/persistit/ui/AdminUITaskPanel.java index e76e35fb1..3289b4348 100644 --- a/persistit/ui/src/main/java/com/persistit/ui/AdminUITaskPanel.java +++ b/persistit/ui/src/main/java/com/persistit/ui/AdminUITaskPanel.java @@ -1,6 +1,7 @@ /** * Copyright 2005-2012 Akiban Technologies, Inc. - * + * Portions Copyrighted 2026 3A Systems, LLC + * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at @@ -264,8 +265,7 @@ private void updateDetailedTaskStatus(final TaskStatus taskStatus) { _startTimeField.setText(_adminUI.formatDate(taskStatus.getStartTime())); _endTimeField.setText(_adminUI.formatDate(taskStatus.getFinishTime())); _expirationTimeField.setText(_adminUI.formatDate(taskStatus.getExpirationTime())); - _lastExceptionField.setText(taskStatus.getLastException() == null ? "" : taskStatus.getLastException() - .toString()); + _lastExceptionField.setText(taskStatus.getLastException() == null ? "" : taskStatus.getLastException()); _statusDetailArea.setText(taskStatus.getStatusDetail()); final StringBuilder sb = new StringBuilder(); diff --git a/persistit/ui/src/main/java/com/persistit/ui/InspectorPanel.java b/persistit/ui/src/main/java/com/persistit/ui/InspectorPanel.java index 69d0b9320..3115c7ba2 100644 --- a/persistit/ui/src/main/java/com/persistit/ui/InspectorPanel.java +++ b/persistit/ui/src/main/java/com/persistit/ui/InspectorPanel.java @@ -153,9 +153,14 @@ public void run() { final Management management = _adminUI.getManagement(); if (management == null) return; + // Defensive: refresh() already returns early on a null record, but + // keep this guard so the dereference below is provably safe. + final Management.LogicalRecord logicalRecord = _logicalRecord; + if (logicalRecord == null) + return; try { final Management.LogicalRecord[] results = management.getLogicalRecordArray(getVolumeName(), - getTreeName(), null, _logicalRecord.getKeyState(), Key.EQ, 1, Integer.MAX_VALUE, true + getTreeName(), null, logicalRecord.getKeyState(), Key.EQ, 1, Integer.MAX_VALUE, true ); if (results == null || results.length == 0) { diff --git a/persistit/ui/src/main/java/com/persistit/ui/ManagementSlidingTableModel.java b/persistit/ui/src/main/java/com/persistit/ui/ManagementSlidingTableModel.java index 964d44b00..f2d6c7c0a 100644 --- a/persistit/ui/src/main/java/com/persistit/ui/ManagementSlidingTableModel.java +++ b/persistit/ui/src/main/java/com/persistit/ui/ManagementSlidingTableModel.java @@ -232,7 +232,6 @@ private synchronized void receiveData(final Fetcher fetcher) { final int oldOffset = _offset; int newOffset = oldOffset; - int firstUpdatedRow; int lost = 0; // rows lost from row cache int kept = _valid; // rows kept from row cache @@ -260,8 +259,6 @@ private synchronized void receiveData(final Fetcher fetcher) { } System.arraycopy(fetcher._resultRows, cut, _infoArray, kept, count - cut); - firstUpdatedRow = newOffset + kept; - if (count < fetcher._requestedCount) { changeRowCount(newOffset + newValid, true); } @@ -287,8 +284,6 @@ private synchronized void receiveData(final Fetcher fetcher) { } System.arraycopy(fetcher._resultRows, 0, _infoArray, 0, count - cut); - firstUpdatedRow = newOffset; - if (count < fetcher._requestedCount) { _definite = false; _currentRowCount = DEFAULT_INITIAL_SIZE_ESTIMATE; diff --git a/persistit/ui/src/main/java/com/persistit/ui/ManagementTableModel.java b/persistit/ui/src/main/java/com/persistit/ui/ManagementTableModel.java index a8d88354c..3b1d378b9 100644 --- a/persistit/ui/src/main/java/com/persistit/ui/ManagementTableModel.java +++ b/persistit/ui/src/main/java/com/persistit/ui/ManagementTableModel.java @@ -123,10 +123,14 @@ protected void setup(final Class clazz, final String[] columnSpecs) throws NoSuc final String flags = st.nextToken(); final String header = st.nextToken(); String rendererName = null; - int minWidth = width / 2; if (st.hasMoreTokens()) { try { - minWidth = Integer.parseInt(st.nextToken()); + // Consume the optional 5th (minimum-width) token so a + // following renderer-name token is not shifted into its + // slot; parsing also validates it. The effective minimum + // width comes from the width token (_widths[i] / 10000), + // so the parsed value itself is intentionally unused. + Integer.parseInt(st.nextToken()); } catch (final NumberFormatException e) { throw new IllegalArgumentException( "Invalid minimum width in column specification: " + columnSpecs[index], e); diff --git a/persistit/ui/src/main/java/com/persistit/ui/VTComboBoxModel.java b/persistit/ui/src/main/java/com/persistit/ui/VTComboBoxModel.java deleted file mode 100644 index ced31ac13..000000000 --- a/persistit/ui/src/main/java/com/persistit/ui/VTComboBoxModel.java +++ /dev/null @@ -1,97 +0,0 @@ -/** - * Copyright 2005-2012 Akiban Technologies, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.persistit.ui; - -import java.util.ArrayList; -import java.util.List; - -import javax.swing.AbstractListModel; -import javax.swing.ComboBoxModel; - -import com.persistit.Management; -import com.persistit.Management.TreeInfo; -import com.persistit.Management.VolumeInfo; - -class VTComboBoxModel extends AbstractListModel implements ComboBoxModel { - - private final VTComboBoxModel _parent; - - private final List _cachedList = new ArrayList(); - private Object _selectedItem; - private long _validTime; - private final Management _management; - - public VTComboBoxModel(final VTComboBoxModel vtcmb, final Management management) { - _parent = vtcmb; - _management = management; - } - - @Override - public Object getElementAt(final int index) { - populateList(); - if (index >= 0 && index < _cachedList.size()) { - return _cachedList.get(index); - } - return null; - } - - @Override - public int getSize() { - populateList(); - return _cachedList.size(); - } - - @Override - public void setSelectedItem(final Object item) { - _selectedItem = item; - } - - @Override - public Object getSelectedItem() { - return _selectedItem; - } - - private void populateList() { - final long now = System.currentTimeMillis(); - if (now - _validTime < 1000) - return; - _validTime = now; - _cachedList.clear(); - try { - if (_parent == null) { - final VolumeInfo[] volumes = _management.getVolumeInfoArray(); - for (int index = 0; index < volumes.length; index++) { - _cachedList.add(volumes[index].getName()); - } - } else { - final String volumeName = (String) _parent.getSelectedItem(); - if (volumeName == null) - return; - final TreeInfo[] trees = _management.getTreeInfoArray(volumeName); - for (int index = 0; index < trees.length; index++) { - _cachedList.add(trees[index].getName()); - } - } - } catch (final Exception e) { - handleException(e); - } - } - - private void handleException(final Exception e) { - e.printStackTrace(); - } -} From 467751d1996adb68d32a0037424f8346b30b5e4d Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Thu, 23 Jul 2026 12:43:04 +0300 Subject: [PATCH 2/2] Address review: revert two CodeQL regressions in persistit The previous commit cleared some findings but introduced two new ones. Exchange.fetchBufferCopy: the Objects.checkIndex rewrite left the array-index finding open (its return value is discarded, so the index is not treated as sanitized) and threw IndexOutOfBoundsException on the reachable over-deep-tree path (depth can legally reach 21 > MAX_TREE_DEPTH, and CLI drives the method straight off the on-disk depth), dropping the depth from the message and escaping catch(IllegalArgumentException). Restore the local capture plus the original bounds check, which keeps the IllegalArgumentException("Tree depth is N") contract. Keep the added @throws javadoc; drop the now-unused java.util.Objects import. BufferPool: "if (reserve.length > 0) reserve = null;" reads a freshly allocated array whose length is constant, so the test is always true (java/constant-comparison). Restore the plain "reserve = null;". The remaining array-index (Exchange) and never-read (BufferPool) findings are false positives, to be dismissed in code scanning. --- .../core/src/main/java/com/persistit/BufferPool.java | 7 ++----- .../core/src/main/java/com/persistit/Exchange.java | 11 ++--------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/persistit/core/src/main/java/com/persistit/BufferPool.java b/persistit/core/src/main/java/com/persistit/BufferPool.java index eddffeefa..51167ff54 100644 --- a/persistit/core/src/main/java/com/persistit/BufferPool.java +++ b/persistit/core/src/main/java/com/persistit/BufferPool.java @@ -282,11 +282,8 @@ public class BufferPool { // Do not use String.format here. // // Release the reserved block to free memory for the diagnostic - // path below. Reading its length first keeps the reservation - // effective (and clears the "never read" static-analysis finding). - if (reserve.length > 0) { - reserve = null; - } + // path below. + reserve = null; System.err.print("Out of memory with "); System.err.print(Runtime.getRuntime().freeMemory()); System.err.print(" bytes free after creating "); diff --git a/persistit/core/src/main/java/com/persistit/Exchange.java b/persistit/core/src/main/java/com/persistit/Exchange.java index 664834933..75fbcea41 100644 --- a/persistit/core/src/main/java/com/persistit/Exchange.java +++ b/persistit/core/src/main/java/com/persistit/Exchange.java @@ -44,7 +44,6 @@ import java.util.ArrayList; import java.util.List; -import java.util.Objects; import static com.persistit.Buffer.EXACT_MASK; import static com.persistit.Buffer.HEADER_SIZE; @@ -4469,14 +4468,8 @@ public Buffer fetchBufferCopy(final int level) throws PersistitException { } final int lvl = level >= 0 ? level : _tree.getDepth() + level; final LevelCache[] levelCache = _levelCache; - // Objects.checkIndex is recognized by CodeQL as an array-index sanitizer. - // Preserve the original IllegalArgumentException contract (including the - // "Tree depth is N" message) for over-deep trees, since depth can legally - // reach PAGE_TYPE_INDEX_MAX and is unbounded on a corrupted volume. - try { - Objects.checkIndex(lvl, levelCache.length); - } catch (final IndexOutOfBoundsException e) { - throw new IllegalArgumentException("Tree depth is " + _tree.getDepth(), e); + if (lvl < 0 || lvl >= levelCache.length) { + throw new IllegalArgumentException("Tree depth is " + _tree.getDepth()); } final int foundAt = searchTree(_key, lvl, false); final Buffer buffer = levelCache[lvl]._buffer;