Skip to content

Conversation

@thswlsqls
Copy link

@thswlsqls thswlsqls commented Dec 17, 2025

Summary

Add explicit null check using Objects.requireNonNull in
DataOutputStream.writeBytes(String s) to provide a clearer error message
and make the API contract explicit.

Problem

The writeBytes method currently throws a NullPointerException when
null is passed, but the error message may not be clear in all contexts.
While JDK 14+ provides helpful NullPointerException messages through
JEP 358 (Helpful NullPointerExceptions), adding an explicit null check
using Objects.requireNonNull makes the API contract more explicit and
provides consistent error messaging across different JDK versions.

Solution

Added Objects.requireNonNull(s, "s") at the beginning of the
writeBytes(String s) method. This ensures:

  • A clear error message ("s") is provided when null is passed
  • The API contract explicitly states that null is not allowed
  • The method fails fast with a descriptive exception

Issue

Fixes JDK-8373660

JBS Issue Link: https://bugs.java.com/bugdatabase/view_bug?bug_id=JDK-8373660

Type of Change

  • Test addition/modification
  • Bug fix
  • New feature
  • Documentation improvement
  • Refactoring

Testing

A new JUnit test has been added to verify the null check behavior:

# Run the specific JUnit test file make test TEST="jtreg:test/jdk/java/io/DataOutputStream/DataOutputStreamTest.java" # Or run all tests in the DataOutputStream directory make test TEST="jtreg:test/jdk/java/io/DataOutputStream" <!-- Anything below this marker will be automatically updated, please do not edit manually! --> --------- ### Progress - [ ] Change must be properly reviewed (1 review required, with at least 1 [Reviewer](https://openjdk.org/bylaws#reviewer)) - [x] Change must not contain extraneous whitespace - [x] Commit message must refer to an issue ### Integration blocker &nbsp;⚠️ Title mismatch between PR and JBS for issue [JDK-8373660](https://bugs.openjdk.org/browse/JDK-8373660) ### Issue * [JDK-8373660](https://bugs.openjdk.org/browse/JDK-8373660): Add explicit null check to DataOutputStream.writeBytes (**Enhancement** - P4) ⚠️ Title mismatch between PR and JBS. ### Reviewing <details><summary>Using <code>git</code></summary> Checkout this PR locally: \ `$ git fetch https://git.openjdk.org/jdk.git pull/28869/head:pull/28869` \ `$ git checkout pull/28869` Update a local copy of the PR: \ `$ git checkout pull/28869` \ `$ git pull https://git.openjdk.org/jdk.git pull/28869/head` </details> <details><summary>Using Skara CLI tools</summary> Checkout this PR locally: \ `$ git pr checkout 28869` View PR using the GUI difftool: \ `$ git pr show -t 28869` </details> <details><summary>Using diff file</summary> Download this PR as a diff file: \ <a href="https://git.openjdk.org/jdk/pull/28869.diff">https://git.openjdk.org/jdk/pull/28869.diff</a> </details> <details><summary>Using Webrev</summary> [Link to Webrev Comment](https://git.openjdk.org/jdk/pull/28869#issuecomment-3665269098) </details>
Added Objects.requireNonNull check to provide clearer error message when null is passed to writeBytes method. This improves defensive programming and makes the API contract more explicit.
@bridgekeeper
Copy link

bridgekeeper bot commented Dec 17, 2025

👋 Welcome back thswlsqls! 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.

@openjdk
Copy link

openjdk bot commented Dec 17, 2025

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

@openjdk openjdk bot added the core-libs core-libs-dev@openjdk.org label Dec 17, 2025
@openjdk
Copy link

openjdk bot commented Dec 17, 2025

@thswlsqls The following label will be automatically applied to this pull request:

  • core-libs

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.

@openjdk openjdk bot added the rfr Pull request is ready for review label Dec 17, 2025
@mlbridge
Copy link

mlbridge bot commented Dec 17, 2025

Webrevs

@AlanBateman
Copy link
Contributor

Add explicit null check using Objects.requireNonNull in DataOutputStream.writeBytes(String s) to provide a clearer error message and make the API contract explicit.

Can you paste in the existing NPE message vs. the new NPE message? With Helpful NPEs (JEP 358), there is a less need to make changes like this.

@thswlsqls
Copy link
Author

thswlsqls commented Dec 18, 2025

@AlanBateman Thank you for the feedback!

Analysis:

JEP 358 message advantages:

  • More detailed: Shows which method call failed ("String.length()")
  • More informative: Explains the context of the failure

Objects.requireNonNull advantages:

  • Explicit API contract: Makes it clear at method entry that null is not allowed
  • Fail-fast: Exception occurs at method entry, not deep in the method body
  • Consistent across JDK versions: Works the same way in JDK 8, 11, 14, 17, etc.
  • Simpler message: Just the parameter name

I understand that with JEP 358, there's less need for explicit null checks in many cases. However, I believe Objects.requireNonNull still provides value by:

  1. Making the API contract explicit at the method signature level
  2. Failing fast before any method body execution
  3. Providing consistent behavior across all JDK versions

I'm open to feedback on whether this change is appropriate, or if we should rely on JEP 358's automatic messages instead. What would you recommend?

@AlanBateman
Copy link
Contributor

I'm open to feedback on whether this change is appropriate, or if we should rely on JEP 358's automatic messages instead. What would you recommend?

Would you mind pasting in the existing NPE message and the new NPE message?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core-libs core-libs-dev@openjdk.org rfr Pull request is ready for review

2 participants