RFR: 6187113: DefaultListSelectionModel.removeIndexInterval(0, Integer.MAX_VALUE) fails

[Alexey Ivanov]

On Fri, 23 Sep 2022 13:19:01 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> 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