Re: RFR: 8373660: Add explicit null check in DataOutputStream.writeBytes
On Wed, 17 Dec 2025 12:56:14 GMT, Eunbin Son wrote:
> ## 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
> - [x] Test addition/modification
> - [x] 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"
At least [this one
comment](https://github.com/openjdk/jdk/pull/28869#issuecomment-3669945336) but
possibly more of them are almost certainly LLM-generated.
-
PR Comment: https://git.openjdk.org/jdk/pull/28869#issuecomment-3734668245
Re: RFR: 8373660: Add explicit null check in DataOutputStream.writeBytes
On Thu, 18 Dec 2025 12:41:16 GMT, Alan Bateman wrote:
>> @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?
>
>> 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?
Thank you very much for the review. Every line of your feedback is very
valuable to me, @AlanBateman @liach @RogerRiggs
and I've learned a lot from this experience.
Please close the PR. I'm happy to follow your guidance.
-
PR Comment: https://git.openjdk.org/jdk/pull/28869#issuecomment-3677378492
Re: RFR: 8373660: Add explicit null check in DataOutputStream.writeBytes
On Wed, 17 Dec 2025 12:56:14 GMT, eunbin son wrote:
> ## 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
> - [x] Test addition/modification
> - [x] 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"
Please close this PR. It does not add enough value.
-
PR Comment: https://git.openjdk.org/jdk/pull/28869#issuecomment-3676490828
Re: RFR: 8373660: Add explicit null check in DataOutputStream.writeBytes
On Wed, 17 Dec 2025 12:56:14 GMT, eunbin son wrote:
> ## 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
> - [x] Test addition/modification
> - [x] 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"
As far as I see, this patch provides little to no value - null checks is
already given by the package specification. The extra `requireNonNull` call
inflates bytecode size and harms JIT compilers. The unit test added is way too
narrow and restricts the NPE message freedom.
Note that when the bug triage moves a submitted ticket to JDK, it does not mean
the ticket is always a good idea - in particular, the "problem" section of this
PR and the "motivation" section in the JBS issue are complete nonsense.
-
Changes requested by liach (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/28869#pullrequestreview-3600044234
Re: RFR: 8373660: Add explicit null check in DataOutputStream.writeBytes
On Thu, 18 Dec 2025 22:46:00 GMT, Roger Riggs wrote:
>> ## 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
>> - [x] Test addition/modification
>> - [x] 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"
>
> $0.02, The original exception message is complete and accurate without adding
> extra bytecodes or an extra line of source.
> I don't think the addition is worthwhile.
@RogerRiggs Thank you for the review!
I've reviewed the OpenJDK codebase and found that most code uses just the
parameter name (e.g., `Objects.requireNonNull(out, "out")`), which is what I've
used here.
However, the Objects.java Javadoc examples use more descriptive messages like
`"bar must not be null"`.
Using `Objects.requireNonNull` at the method entry point allows us to
immediately identify the problematic parameter and makes the API contract
explicit. By using a more descriptive message like `"s must not be null"`, we
can provide a complete and accurate error message that clearly indicates which
parameter is null and why it cannot be null, while still maintaining the
benefits of fail-fast validation at method entry.
I'm open to changing it to a more descriptive message if that's the preferred
approach.
Would you like to share your opinion on this approach?
- `Objects.requireNonNull(s, "s must not be null")`
I'm happy to follow the project's conventions and your guidance on this.
Thank you for take time to review this.
-
PR Comment: https://git.openjdk.org/jdk/pull/28869#issuecomment-3672625364
Re: RFR: 8373660: Add explicit null check in DataOutputStream.writeBytes
On Wed, 17 Dec 2025 12:56:14 GMT, eunbin son wrote:
> ## 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
> - [x] Test addition/modification
> - [x] 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"
$0.02, The original exception message is complete and accurate without adding
extra bytecodes or an extra line of source.
I don't think the addition is worthwhile.
-
PR Comment: https://git.openjdk.org/jdk/pull/28869#issuecomment-3672567334
Re: RFR: 8373660: Add explicit null check in DataOutputStream.writeBytes
On Thu, 18 Dec 2025 12:41:16 GMT, Alan Bateman wrote:
>> @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?
>
>> 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?
Of course. Thank you for the review, @AlanBateman!
Here are the NPE messages:
**Existing NPE message (JEP 358 automatic, without Objects.requireNonNull):**
java.lang.NullPointerException: Cannot invoke "String.length()" because "s" is
null
**New NPE message (with Objects.requireNonNull):**
java.lang.NullPointerException: s
The JEP 358 message provides more context (shows which method call failed),
while Objects.requireNonNull provides a simpler message and fails fast at
method entry.
-
PR Comment: https://git.openjdk.org/jdk/pull/28869#issuecomment-3672535570
Re: RFR: 8373660: Add explicit null check in DataOutputStream.writeBytes
On Thu, 18 Dec 2025 12:00:01 GMT, eunbin son wrote: > 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? - PR Comment: https://git.openjdk.org/jdk/pull/28869#issuecomment-3670098310
Re: RFR: 8373660: Add explicit null check in DataOutputStream.writeBytes
On Wed, 17 Dec 2025 13:37:17 GMT, Alan Bateman wrote:
>> ## 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
>> - [x] Test addition/modification
>> - [x] 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"
>
>> 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.
@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?
-
PR Comment: https://git.openjdk.org/jdk/pull/28869#issuecomment-3669945336
Re: RFR: 8373660: Add explicit null check in DataOutputStream.writeBytes
On Wed, 17 Dec 2025 12:56:14 GMT, eunbin son wrote: > 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. - PR Comment: https://git.openjdk.org/jdk/pull/28869#issuecomment-3665396706
