RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v9]

[Andy Goryachev]

On Fri, 19 Apr 2024 12:43:33 GMT, Oliver Kopp <duke at openjdk.org> wrote:

>> Fixes https://bugs.openjdk.org/browse/JDK-8330462.
>> 
>> If the parameter `maxLength` is larger than `Integer.MAX_VALUE - start`, then an addition of `start` to it leads to a negative value. This is "fixed" by using `Math.max` comparing the `maxLength` and `maxLength + start`.
>
> Oliver Kopp has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix test

modules/javafx.graphics/src/main/java/com/sun/glass/ui/win/WinTextRangeProvider.java line 2:

> 1: /*
> 2:  * Copyright (c) 2014, 2022, 2024, Oracle and/or its affiliates. All rights reserved.

this needs the following form:

Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights reserved.

modules/javafx.graphics/src/main/java/com/sun/glass/ui/win/WinTextRangeProvider.java line 116:

> 114:             return fixedMaxEnd;
> 115:         }
> 116:     }

Frankly, I have hard time understanding this code (maybe a comment describing what the method does and why might help).

It looks to me that all we need to do is to guard against a very large maxLength (which for some reason called here 'requestedSteps' which does not seem right - should the last two arguments be swapped?)


public static int getEndIndex(int start, int length, int maxLength) {
  if(length > maxLength) {
    length = maxLength;
  }
  return start + length;
}


That is, I assume we don't have to worry about start + fixedLength overflowing, we just need to make sure we don't go beyond maxLength.  Or is my assumption wrong and start can be negative, or start+fixedLength might overflow?

modules/javafx.graphics/src/main/java/com/sun/glass/ui/win/WinTextRangeProvider.java line 378:

> 376:         if (text == null) return null;
> 377:         validateRange(text);
> 378:         int endOffset = getValidStringIndex(start, maxLength, end);

If I presume the arguments are (start, length, maxLength), the last two arguments on this line only need to be swapped.  They seem to be correct on LL 394, 498.

modules/javafx.graphics/src/test/java/test/com/sun/glass/ui/win/WinTextRangeProviderTest.java line 29:

> 27: import org.junit.Test;
> 28: import static org.junit.Assert.*;
> 29: import com.sun.glass.ui.win.WinTextRangeProvider;

since it's a new test, would it be possible to convert it to junit5?

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1442#discussion_r1572691354
PR Review Comment: https://git.openjdk.org/jfx/pull/1442#discussion_r1572723728
PR Review Comment: https://git.openjdk.org/jfx/pull/1442#discussion_r1572725583
PR Review Comment: https://git.openjdk.org/jfx/pull/1442#discussion_r1572692653