On Wed, 11 Nov 2026 08:01:41 GMT, Jaikiran Pai <[email protected]> wrote:
> Can I please get a review of this change which proposes to address the issue > noted in https://bugs.openjdk.org/browse/JDK-8396120? > > The implementation of `LdapCtx.getSchema(name)` searches for the > `subschemasubentry` attribute for the given name. The response to that search > is allowed to return the attribute with no values associated with it. Because > of what appears to be a typo in the current implementation of > `LdapCtx.getSchema(...)`, for such cases where the attribute has no values, > the implementation throws an unspecified `NoSuchElementException`. The commit > in this PR addresses it by correctly detecting the absence of attribute > values. > > A new jtreg test has been introduced to reproduce the issue and verify the > fix. Existing tests in tier1, tier2, tier3 continue to pass too. > > I'm unsure if this change would require a CSR. The fix updates the > implementation to match the specification, unlike previously where it would > throw the unspecified exception. Should we file one? > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). test/jdk/com/sun/jndi/ldap/LdapCtxGetSchemaTest.java line 1: > 1: /* *Nit:* There is an `LdapCtx` folder in the same directory. This test can be better suited there. test/jdk/com/sun/jndi/ldap/LdapCtxGetSchemaTest.java line 102: > 100: .scheme("ldap") > 101: .host(server.getInetAddress().getHostAddress()) > 102: .port(server.getPort()) The test uses an empty base DN. If you introduce a non-empty base DN: Suggestion: .port(server.getPort()) .path("/dc=openjdk,dc=org") and run the test again, you bump to a stack overflow due to recursion at `LdapCtx.java:1748`. test/jdk/com/sun/jndi/ldap/LdapCtxGetSchemaTest.java line 118: > 116: */ > 117: @Test > 118: void testGetSchema() throws Exception { `throws Exception` is redundant: Suggestion: void testGetSchema() { test/jdk/com/sun/jndi/ldap/LdapCtxGetSchemaTest.java line 130: > 128: */ > 129: @Test > 130: void testGetSchemaString() throws Exception { `throws Exception` is redundant: Suggestion: void testGetSchemaString() { ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/33838#discussion_r4539701025 PR Review Comment: https://git.openjdk.org/jdk/pull/33838#discussion_r4539979812 PR Review Comment: https://git.openjdk.org/jdk/pull/33838#discussion_r4539705753 PR Review Comment: https://git.openjdk.org/jdk/pull/33838#discussion_r4539708126
