On Fri, 23 Sep 2022 13:19:01 GMT, Prasanta Sadhukhan <[email protected]>
wrote:
> DefaultListSelectionModel.removeIndexInterva accepts `int` value which allows
> it to take in Integer.MAX_VALUE theoratically but it does calculation with
> that value which can results in IOOBE.
> Fix is to make sure the calculation stays within bounds.
Changes requested by aivanov (Reviewer).
src/java.desktop/share/classes/javax/swing/DefaultListSelectionModel.java line
695:
> 693: int rmMaxIndex = Math.max(index0, index1);
> 694: int gapLength = ((rmMaxIndex - rmMinIndex) + 1) > (rmMaxIndex -
> rmMinIndex)
> 695: ? ((rmMaxIndex - rmMinIndex) + 1) : (rmMaxIndex -
> rmMinIndex);
I wonder if the model supports selections of 0 .. Integer.MAX_VALUE, it should
… but it doesn't. If I call
selectionModel.setSelectionInterval(Integer.MAX_VALUE - 1, Integer.MAX_VALUE);
it never returns, it goes into an infinite loop in `changeSelection` because
the condition `i <= Math.max(setMax, clearMax)` is always `true` if either
`setMax` or `clearMax` is `Integer.MAX_VALUE`. Therefore, we should prevent
that from happening. With that in mind, for `removeIndexInterval`, the value of
`Integer.MAX_VALUE` becomes invalid.
What will happen if negative values are passed, in particular
`Integer.MIN_VALUE`?
src/java.desktop/share/classes/javax/swing/DefaultListSelectionModel.java line
705:
> 703: } else {
> 704: setState(i, value.get(gapLength));
> 705: }
~~If `rmMaxIndex == Integer.MAX_VALUE - 1`, the code to remove the interval in
the gap could remain the same. If `rmMaxIndex == Integer.MAX_VALUE`, the value
of `gapLength` could be set to a value as if `rmMaxIndex == Integer.MAX_VALUE -
1`, then one more value is to be moved `setState(Integer.MAX_VALUE - gapLength,
value.get(Integer.MAX_VALUE)` provided that `Integer.MAX_VALUE - gapLength <=
maxIndex`.~~
This has become irrelevant taking into account my comment above that the model
can't support selections up to and including `Integer.MAX_VALUE`.
test/jdk/javax/swing/TestDefListModelExcpn.java line 32:
> 30: import javax.swing.DefaultListSelectionModel;
> 31:
> 32: public class TestDefListModelExcpn {
Why not spell `Exception`?
test/jdk/javax/swing/TestDefListModelExcpn.java line 37:
> 35: selectionModel.setSelectionInterval(0, 1);
> 36: selectionModel.removeIndexInterval(0, Integer.MAX_VALUE);
> 37: }
A comment would help to understand that an exception was thrown and *no
exception is expected*.
`throws Exception` in `main` declaration can be removed to hint at this too.
-------------
PR: https://git.openjdk.org/jdk/pull/10409
