RFR: 8263509: LdapSchemaParser.readNextTag checks array length incorrectly
SonarCloud rightfully says: The length of "values" is always ">=0", so update this test to either "==0" or ">0". // make sure at least one value was returned if(values.length < 0) { // <--- here throw new InvalidAttributeValueException("no values for " + "attribute "" + tagName + """); } There is a subsequent access to values[0], which means the failure would throw `AIOOB`, not `InvalidAttributeValueException`. Additional testing: - [x] Linux x86_64 fastdebug, `com/sun/jndi` ------------- Commit messages: - 8263509: LdapSchemaParser.readNextTag checks array length incorrectly Changes: https://git.openjdk.java.net/jdk/pull/2968/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2968&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263509 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2968.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2968/head:pull/2968 PR: https://git.openjdk.java.net/jdk/pull/2968
On Fri, 12 Mar 2021 13:25:31 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
SonarCloud rightfully says: The length of "values" is always ">=0", so update this test to either "==0" or ">0".
// make sure at least one value was returned if(values.length < 0) { // <--- here throw new InvalidAttributeValueException("no values for " + "attribute "" + tagName + """); }
There is a subsequent access to values[0], which means the failure would throw `AIOOB`, not `InvalidAttributeValueException`.
Additional testing: - [x] Linux x86_64 fastdebug, `com/sun/jndi`
This looks right but I'm surprised it isn't caught by tests (@AlekseiEfimov - do you have suggests for tests that would be useful to add here?) ------------- PR: https://git.openjdk.java.net/jdk/pull/2968
On Fri, 12 Mar 2021 13:25:31 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
SonarCloud rightfully says: The length of "values" is always ">=0", so update this test to either "==0" or ">0".
// make sure at least one value was returned if(values.length < 0) { // <--- here throw new InvalidAttributeValueException("no values for " + "attribute "" + tagName + """); }
There is a subsequent access to values[0], which means the failure would throw `AIOOB`, not `InvalidAttributeValueException`.
Additional testing: - [x] Linux x86_64 fastdebug, `com/sun/jndi`
Seems fine. Lets hope no caller relies on this throwing AIOOBE. ..Thomas ------------- Marked as reviewed by stuefe (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2968
On Fri, 12 Mar 2021 15:28:03 GMT, Thomas Stuefe <stuefe@openjdk.org> wrote:
SonarCloud rightfully says: The length of "values" is always ">=0", so update this test to either "==0" or ">0".
// make sure at least one value was returned if(values.length < 0) { // <--- here throw new InvalidAttributeValueException("no values for " + "attribute "" + tagName + """); }
There is a subsequent access to values[0], which means the failure would throw `AIOOB`, not `InvalidAttributeValueException`.
Additional testing: - [x] Linux x86_64 fastdebug, `com/sun/jndi`
Seems fine. Lets hope no caller relies on this throwing AIOOBE.
..Thomas
This looks right but I'm surprised it isn't caught by tests (@AlekseiEfimov - do you have suggests for tests that would be useful to add here?)
Can we go without adding tests here? This seems quite trivial. ------------- PR: https://git.openjdk.java.net/jdk/pull/2968
On Tue, 16 Mar 2021 08:56:29 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
Seems fine. Lets hope no caller relies on this throwing AIOOBE.
..Thomas
This looks right but I'm surprised it isn't caught by tests (@AlekseiEfimov - do you have suggests for tests that would be useful to add here?)
Can we go without adding tests here? This seems quite trivial.
Hi @shipilev, We do not have tests for LDAP schema parser, and creating one wouldn't be trivial: environment with real LDAP server will be needed for, at least, to collect LDAP message dumps with required schema value. Therefore, I think it is reasonable to continue without a test here. -Aleksei ------------- PR: https://git.openjdk.java.net/jdk/pull/2968
On Fri, 12 Mar 2021 13:25:31 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
SonarCloud rightfully says: The length of "values" is always ">=0", so update this test to either "==0" or ">0".
// make sure at least one value was returned if(values.length < 0) { // <--- here throw new InvalidAttributeValueException("no values for " + "attribute "" + tagName + """); }
There is a subsequent access to values[0], which means the failure would throw `AIOOB`, not `InvalidAttributeValueException`.
Additional testing: - [x] Linux x86_64 fastdebug, `com/sun/jndi`
Marked as reviewed by aefimov (Committer). ------------- PR: https://git.openjdk.java.net/jdk/pull/2968
On Tue, 16 Mar 2021 10:52:25 GMT, Aleksei Efimov <aefimov@openjdk.org> wrote:
SonarCloud rightfully says: The length of "values" is always ">=0", so update this test to either "==0" or ">0".
// make sure at least one value was returned if(values.length < 0) { // <--- here throw new InvalidAttributeValueException("no values for " + "attribute "" + tagName + """); }
There is a subsequent access to values[0], which means the failure would throw `AIOOB`, not `InvalidAttributeValueException`.
Additional testing: - [x] Linux x86_64 fastdebug, `com/sun/jndi`
Marked as reviewed by aefimov (Committer).
Thanks! ------------- PR: https://git.openjdk.java.net/jdk/pull/2968
On Fri, 12 Mar 2021 13:25:31 GMT, Aleksey Shipilev <shade@openjdk.org> wrote:
SonarCloud rightfully says: The length of "values" is always ">=0", so update this test to either "==0" or ">0".
// make sure at least one value was returned if(values.length < 0) { // <--- here throw new InvalidAttributeValueException("no values for " + "attribute "" + tagName + """); }
There is a subsequent access to values[0], which means the failure would throw `AIOOB`, not `InvalidAttributeValueException`.
Additional testing: - [x] Linux x86_64 fastdebug, `com/sun/jndi`
This pull request has now been integrated. Changeset: 83a9a029 Author: Aleksey Shipilev <shade@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/83a9a029 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8263509: LdapSchemaParser.readNextTag checks array length incorrectly Reviewed-by: stuefe, aefimov ------------- PR: https://git.openjdk.java.net/jdk/pull/2968
participants (4)
-
Alan Bateman
-
Aleksei Efimov
-
Aleksey Shipilev
-
Thomas Stuefe