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

[Oliver Kopp]

On Fri, 19 Apr 2024 17:55:12 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> 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 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?

Smalltalk 1: I thought this was easier to understand than some byte code generation in the JVM. 😅 

Smalltalk 2: Sometimes, the code for "Calculate a + b, but return c at most" is pretty hard to craft.

Smalltalk 3: The whole checks stem from possible "out of range" values, especially from the other functions mentioned at https://github.com/openjdk/jfx/pull/1442/#discussion_r1570948582. That was too defensive, as only `requestedSteps` AKA `length` can be out of range.

OK, I seem to have understood "Also, this doesn't follow the usual pattern of checking for integer overflow" by Kevin wrong. I googled the Java way of the usual pattern for Integer overflow. Since Java 8, there is `Math.addExact`. I thought, that this was meant. -- I found it from https://stackoverflow.com/a/3001879/873282. (Inlining the code of `Math.addExact` seems to have negative performance impact.)

The proposed code works OKish if strings are not in length area of Integer.MAX_VALUE. I think, we can safely assume that. - It however returns more characters if start is greater then 0. Example: I request start 2, length of 5, but maximum end index  of 3. Then 3 should be returned, not 7.

---

I changed the code accordingly. Also added a comment when an overflow might happen. From the discussion here, it seems, we can ignore these cases.

Note that the old code returned `0` if `start` was negative. New might return some negative value if `start` is negative enough. However, did not seem to happen, because otherwise, IndexOutOfBounds exceptions might have been seen.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1442#discussion_r1573308526