On Fri, 19 Apr 2024 12:43:33 GMT, Oliver Kopp <d...@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
