RFR: 8377486: com.sun.jndi.ldap.sasl.SaslOutputStream.write() throws NullPointerException if it is already closed
Can I please get a review of this change which proposes to prevent the `NullPointerException` thrown from `com.sun.jndi.ldap.sasl.SaslOutputStream.write()` methods when the stream is already closed? This addresses https://bugs.openjdk.org/browse/JDK-8377486. As noted in that issue, `OutputStream.write(...)` is specified to throw an `IOException` in such cases. The change in this PR updates the `SaslOutputStream.write()` methods to throw an `IOException` when closed. A new test has been introduced to verify this change. ------------- Commit messages: - add a test - 8377486: com.sun.jndi.ldap.sasl.SaslOutputStream.write() throws NullPointerException if it is already closed Changes: https://git.openjdk.org/jdk/pull/29647/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=29647&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8377486 Stats: 151 lines in 2 files changed: 145 ins; 1 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/29647.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/29647/head:pull/29647 PR: https://git.openjdk.org/jdk/pull/29647
On Tue, 10 Feb 2026 10:05:31 GMT, Jaikiran Pai <jpai@openjdk.org> wrote:
Can I please get a review of this change which proposes to prevent the `NullPointerException` thrown from `com.sun.jndi.ldap.sasl.SaslOutputStream.write()` methods when the stream is already closed? This addresses https://bugs.openjdk.org/browse/JDK-8377486.
As noted in that issue, `OutputStream.write(...)` is specified to throw an `IOException` in such cases. The change in this PR updates the `SaslOutputStream.write()` methods to throw an `IOException` when closed.
A new test has been introduced to verify this change.
Looks reasonable to me. However, is it possible that close() is called asynchronously? If not then the proposed fix looks fine. ------------- PR Review: https://git.openjdk.org/jdk/pull/29647#pullrequestreview-3814402630
On Tue, 17 Feb 2026 14:48:46 GMT, Daniel Fuchs <dfuchs@openjdk.org> wrote:
However, is it possible that close() is called asynchronously?
The `SaslOutputStream` instance gets used by the `com.sun.jndi.ldap.Connection`. That `outStream` instance is `close()`d when that `Connection` is being closed (`cleanup()` calls `flushAndCloseOutputStream()`). Similarly, the `Connection` instance issues `outStream.write(...)` calls in a few places. Both the `outStream.close()` and `outStream.write(...)` calls are issued when holding the (same) `lock`. So there doesn't appear to be any concurrent calls to `close()` and `write(...)` on the `SaslOutputStream` instance. Other than the `Connection` class, from what I can see in the code, the `SaslOutputStream` stream instance doesn't get closed or written to. ------------- PR Comment: https://git.openjdk.org/jdk/pull/29647#issuecomment-3915829386
On Tue, 10 Feb 2026 10:05:31 GMT, Jaikiran Pai <jpai@openjdk.org> wrote:
Can I please get a review of this change which proposes to prevent the `NullPointerException` thrown from `com.sun.jndi.ldap.sasl.SaslOutputStream.write()` methods when the stream is already closed? This addresses https://bugs.openjdk.org/browse/JDK-8377486.
As noted in that issue, `OutputStream.write(...)` is specified to throw an `IOException` in such cases. The change in this PR updates the `SaslOutputStream.write()` methods to throw an `IOException` when closed.
A new test has been introduced to verify this change.
Marked as reviewed by dfuchs (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/29647#pullrequestreview-3815656501
On Tue, 10 Feb 2026 10:05:31 GMT, Jaikiran Pai <jpai@openjdk.org> wrote:
Can I please get a review of this change which proposes to prevent the `NullPointerException` thrown from `com.sun.jndi.ldap.sasl.SaslOutputStream.write()` methods when the stream is already closed? This addresses https://bugs.openjdk.org/browse/JDK-8377486.
As noted in that issue, `OutputStream.write(...)` is specified to throw an `IOException` in such cases. The change in this PR updates the `SaslOutputStream.write()` methods to throw an `IOException` when closed.
A new test has been introduced to verify this change.
Thank you Daniel for the review. ------------- PR Comment: https://git.openjdk.org/jdk/pull/29647#issuecomment-3918707931
On Tue, 10 Feb 2026 10:05:31 GMT, Jaikiran Pai <jpai@openjdk.org> wrote:
Can I please get a review of this change which proposes to prevent the `NullPointerException` thrown from `com.sun.jndi.ldap.sasl.SaslOutputStream.write()` methods when the stream is already closed? This addresses https://bugs.openjdk.org/browse/JDK-8377486.
As noted in that issue, `OutputStream.write(...)` is specified to throw an `IOException` in such cases. The change in this PR updates the `SaslOutputStream.write()` methods to throw an `IOException` when closed.
A new test has been introduced to verify this change.
This pull request has now been integrated. Changeset: 1d713b2b Author: Jaikiran Pai <jpai@openjdk.org> URL: https://git.openjdk.org/jdk/commit/1d713b2bbe4daddc8a9b1648296b59412e683186 Stats: 151 lines in 2 files changed: 145 ins; 1 del; 5 mod 8377486: com.sun.jndi.ldap.sasl.SaslOutputStream.write() throws NullPointerException if it is already closed Reviewed-by: dfuchs ------------- PR: https://git.openjdk.org/jdk/pull/29647
participants (2)
-
Daniel Fuchs
-
Jaikiran Pai