ppkarwasz commented on code in PR #790:
URL: https://github.com/apache/commons-io/pull/790#discussion_r2390105476
##########
src/main/java/org/apache/commons/io/input/NullInputStream.java:
##########
@@ -292,7 +294,8 @@ public int read(final byte[] bytes) throws IOException {
*/
@Override
public int read(final byte[] bytes, final int offset, final int length)
throws IOException {
- if (bytes.length == 0 || length == 0) {
+ IOUtils.checkFromIndexSize(bytes, offset, length);
Review Comment:
I totally agree for the `Reader` implementations: all the implementations I
have seen call `ensureOpen()` **before** arguments validation.
However in `InputStream` I see the inverse convention:
1. The Javadoc states that:
> If `len` is zero, then no bytes are read and `0` is returned;
otherwise, there is an attempt to read at least one byte.
and
> `IOException` - If the **first** byte cannot be read for any reason
other than end of file, or if the input stream has been closed, or if some
other I/O error occurs.
2. Implementations seem to agree that `ensureOpen()` or equivalent is
checked **after** arguments validation:
- `FileInputStream` delegates to the native method `readBytes`
([source](https://github.com/openjdk/jdk/blob/2746c1a555891564963299182b3b0293eaefc901/src/java.base/share/native/libjava/io_util.c#L74)).
- `InputStream.nullInputStream()`
([source](https://github.com/openjdk/jdk/blob/2746c1a555891564963299182b3b0293eaefc901/src/java.base/share/classes/java/io/InputStream.java#L87))
validates **before** checking if it is open.
- The default `InputStream#read(byte[], int, int)` implementation
([source](https://github.com/openjdk/jdk/blob/2746c1a555891564963299182b3b0293eaefc901/src/java.base/share/classes/java/io/InputStream.java#L283))
also validates before calling `read()`.
- The same pattern can be seen in `sun.nio.ch.ChannelInputStream` and
`sun.nio.ch.SocketInputStream`, which are wrappers returned by
`Channels.newInputStream(ReadableByteChannel)`.
- You are right about `BufferedInputStream` and `PushbackInputStream`:
they call an equivalent of `ensureOpen()` **before** arguments validation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
