jdk
jdk copied to clipboard
openjdk
6187113: DefaultListSelectionModel.removeIndexInterval(0, Integer.MAX_VALUE) fails
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.
Progress
- [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
- [ ] Change requires a CSR request to be approved
Issues
- JDK-6187113: DefaultListSelectionModel.removeIndexInterval(0, Integer.MAX_VALUE) fails
- JDK-8295329: DefaultListSelectionModel.removeIndexInterval(0, Integer.MAX_VALUE) fails (CSR)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10409/head:pull/10409
$ git checkout pull/10409
Update a local copy of the PR:
$ git checkout pull/10409
$ git pull https://git.openjdk.org/jdk pull/10409/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 10409
View PR using the GUI difftool:
$ git pr show -t 10409
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10409.diff
:wave: Welcome back psadhukhan! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
![bridgekeeper[bot] avatar](https://avatars.githubusercontent.com/u/41768318?v=4 "Published by a pub.dev verified publisher"){kind=small}
@prsadhuk The following label will be automatically applied to this pull request:
client
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.
Webrevs
- 11: Full - Incremental (eb99ea3b)
- 10: Full - Incremental (da05d20e)
- 09: Full - Incremental (b1857d3c)
- 08: Full - Incremental (fce8c56b)
- 07: Full - Incremental (89b9e86c)
- 06: Full - Incremental (ee02dc45)
- 05: Full - Incremental (0c4b2846)
- 04: Full - Incremental (ee43bd8d)
- 03: Full - Incremental (23d59222)
- 02: Full - Incremental (b70c6ad7)
- 01: Full - Incremental (e42f2afd)
- 00: Full (94fdf905)
There exists at least one other problem because of integer overflow in insertIndexInterval.
This code
selectionModel.setSelectionInterval(Integer.MAX_VALUE - 1, Integer.MAX_VALUE);
selectionModel.insertIndexInterval(Integer.MAX_VALUE - 1, Integer.MAX_VALUE, true);
throws
Exception in thread "main" java.lang.IndexOutOfBoundsException: bitIndex < 0: -2
at java.base/java.util.BitSet.get(BitSet.java:626)
at java.desktop/javax.swing.DefaultListSelectionModel.set(DefaultListSelectionModel.java:311)
at java.desktop/javax.swing.DefaultListSelectionModel.setState(DefaultListSelectionModel.java:629)
at java.desktop/javax.swing.DefaultListSelectionModel.insertIndexInterval(DefaultListSelectionModel.java:657)
at SelectionModelTest.main(SelectionModelTest.java)
Would you like to include it as part of this fix? Shall I submit a new bug for this?
Thanks for your suggestive code.. I guess you are now ok with handling Integer.MAX_VALUE in the code rather than only in javadoc.
BTW, your code change resulted in JCK failure so I have used a mix of what I had in previous iterations and your code and this satisfied JCK as well as the unit tests.
Regarding insertIndexInterval I guess it's better to address in this PR as it has same issue in same file, so I have added a fix for it also in this PR.
With the current code in the PR, six (6) test cases still fail.
The failure of test10 is expected: you adjust the value of length so that insMaxIndex can't be greater than Integer.MAX_VALUE. It is a valid failure, the test needs changing: lead should be become Integer.MAX_VALUE + 1 = -2147483648 (0x80000000).
Other five failures are valid ones, minIndex and maxIndex in the object don't have the expected values.
@prsadhuk This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@prsadhuk This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
The CSR was approved two days ago, yet the bots didn't update the status of the issue. It should be ready for integration.
@prsadhuk
The label csr is not a valid label.
These labels are valid:
serviceabilityhotspothotspot-compileride-supportkullai18nshenandoahjdkjavadocsecurityhotspot-runtimejmxbuildnioclientcore-libscompilernethotspot-gchotspot-jfr
@prsadhuk usage: /csr [needed|unneeded], requires that the issue the pull request refers to links to an approved CSR request.
@prsadhuk This change now passes all automated pre-integration checks.
ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.
After integration, the commit message for the final commit will be:
6187113: DefaultListSelectionModel.removeIndexInterval(0, Integer.MAX_VALUE) fails
Co-authored-by: Alexey Ivanov <[email protected]>
Reviewed-by: aivanov
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.
At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.
➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.
@prsadhuk has indicated that a compatibility and specification (CSR) request is needed for this pull request.
@prsadhuk please create a CSR request for issue JDK-6187113 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.
@prsadhuk Pushed as commit c2ebd179388cac5d6e10f98aab9a7ea909f8bc6b.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.