[BUG] ByteArrayIndexInput::readBytes can read beyond valid boundary of the slice

[parasjain1]

Describe the bug
When using ByteArrayIndexInput::readBytes on a sliced ByteArrayIndexInput, the position validation check seems to be incorrect and allows reading beyond the valid boundary of the slice.

This is due to incorrect validation check in ByteArrayIndexInput::readBytes() method -

if (pos + len > this.offset + length) {
    throw new EOFException("seek past EOF");
}

The above code pointer exists since a very long time. The scenario of readBytes() called on a slice was not covered in a UT and hence this may have been missed. Please let me know if this does not seem to be a bug and rather is some misunderstanding wrt how slices are supposed to work. If this looks legit, I'll raise a PR with the fix and UT.

To Reproduce
Below is a UT that can be added to ByteArrayIndexInputTests to reproduce the behaviour. The UT will fail for the current implementation of readBytes().

public void testReadBytesWithSlice() throws IOException {
        int inputLength = randomIntBetween(100, 1000);

        byte[] input = randomUnicodeOfLength(inputLength).getBytes(StandardCharsets.UTF_8);
        IndexInput indexInput = getIndexInput(input);

        int sliceOffset = randomIntBetween(1, inputLength - 10);
        int sliceLength = randomIntBetween(1, inputLength - 10 - sliceOffset);
        IndexInput slice = indexInput.slice("slice", sliceOffset, sliceLength);

        // read a byte from sliced index input and verify if the read value is correct
        assertEquals(input[sliceOffset], slice.readByte());

        // read few more bytes into a byte array
        int bytesToRead = randomIntBetween(1, sliceLength - 1);
        slice.readBytes(new byte[bytesToRead], 0, bytesToRead);

        // now try to read beyond the boundary of the slice, but within the
        // boundary of the original IndexInput. We've already read few bytes
        // so this is expected to fail
        assertThrows(EOFException.class, () -> slice.readBytes(new byte[sliceLength], 0, sliceLength));
    }

Expected behavior
ByteArrayIndexInput::readBytes should not allow reads beyond the boundary of the slice and throw an EOFException.
Proposal is to fix the validation check as below -

if (pos + len > length) {
    throw new EOFException("seek past EOF");
}

Additional context
Attaching a manual dry-run example below -
We consider a basic scenario where we have a slice of length 4 and per the current implementation can call readBytes with len = 5 without raising an EOFException.

        /**
         * --- Original IndexInput of length 10 ---
         * 0 1 2 3 4 5 6 7 8 9 10
         *       ^     ^
         *       3 4 5 6
         *---- ^^^  slice ^^^ ------
         * offset = 3
         * length = 4
         * pos = 0
         *--------------------------
         * Sample call - readBytes(b, 0, 5)
         * 
         * len = 5
         * pos + len = 0 + 5
         * offset + length = 7
         *
         * Current Validation check : pos + len > offset + length  : 5 > 7 : false : does not raise EOFException
         *
         * Proposed validation check : pos + len > length : 5 > 4 : true :  raises EOFException
         *
         */

[andrross]

@parasjain1 Looks like a bug. Can you put up a PR?

[parasjain1]

The behaviour is similar for other methods too as there's a common check ByteArrayIndexInput::validatePos. Raising a PR to fix validatePos.

[parasjain1]

Have raised a PR against main. We'll also need to backport this.