RFR: 8325304: Several classes in java.util.jar and java.util.zip don't specify the behaviour for null arguments [v5]

Alan Bateman alanb at openjdk.org
Wed Feb 7 11:33:53 UTC 2024


On Wed, 7 Feb 2024 11:14:06 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>>> These 2 classes, the `CheckedInputStream` and the `CheckedOutputStream` are slightly different from the rest of the classes in this changeset. This javadoc here is for the constructor of the `CheckedInputStream`. The implementation of the constructor just blindly assigns the passed argument to an internal member field and doesn't do any null check on the passed arguments (any subsequent usage of these instance fields within that class, then leads to a NullPointerException). 
>> 
>> The superclasses FilterInputStream and FilterOutputStream have a a protected field for the underlying stream so it's possible for a subclass to set the underlying stream lazily. I can't recall seeing code in the wild availing of that but it is possible.
>> 
>> CheckedInputStream and CheckedOutputStream date from JDK 1.1 and it's not clear what the intention was. My guess is that it was an oversight/bug to not check for null when constructing directly. Fixing this will help catch buggy code but it does mean a behavior change. I think we have to keep existing behavior for the subclassing case because it is possible for the subclass to set the stream lazily.
>
> Given that subclasses could set these fields lazily (however remote the case might be), do you think we should then not specify the `NullPointerException` for the read methods on these 2 classes. In which case I can exclude these 2 classes from this PR.

It would be simpler to drop CheckedInputStream and CheckedOutputStream and deal with them separately. It's unfortunate that this current effort has run into this but that comes with touching some of the JDK 1.0/1.1 era classes.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17728#discussion_r1481333286


More information about the core-libs-dev mailing list