RFR: 8306461: ObjectInputStream::readObject() should handle negative array sizes without throwing NegativeArraySizeExceptions

[Roger Riggs]

On Wed, 19 Apr 2023 16:47:33 GMT, Volker Simonis <simonis at openjdk.org> wrote:

> This issue was reported by: Yakov Shafranovich ([yakovsh at amazon.com](mailto:yakovsh at amazon.com))
> 
> Currently, `ObjectInputStream::readObject()` doesn't explicitly checks for a negative array length in the deserialization stream. Instead it calls `j.l.r.Array::newInstance(..)` with the negative length which results in a `NegativeArraySizeException`. NegativeArraySizeException is an unchecked exception which is neither declared in the signature of `ObjectInputStream::readObject()` nor mentioned in its API specification. It is therefore not obvious for users of `ObjectInputStream::readObject()` that they may have to handle `NegativeArraySizeException`s. It would therefor be better if a negative array length in the deserialization stream would be automatically wrapped in an `InvalidClassException` which is a checked exception (derived from `IOException` via `ObjectStreamException`) and declared in the signature of `ObjectInputStream::readObject()`.
> 
> If we do the negative array length check in `ObjectInputStream::readObject()` before filtering, this will then also fix `ObjectInputFilter.FilterInfo::arrayLength()` which is defined as:
> 
> Returns:
> the non-negative number of array elements when deserializing an array of the class, otherwise -1
> 
> but currently returns a negative value if the array length is negative.

test/jdk/java/io/ObjectInputStream/NegativeArraySizeTest.java line 59:

> 57:             if (serializedData[i] == 0x78) {
> 58:                 firstPos = i;
> 59:                 break;

Move setting the length code (lines 64-67) here and return immediately.

test/jdk/java/io/ObjectInputStream/NegativeArraySizeTest.java line 62:

> 60:             }
> 61:         }
> 62: 

The test should fail if it falls through without finding the BLOCKDATA. Just a double guard against the unexpected.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13540#discussion_r1172973558
PR Review Comment: https://git.openjdk.org/jdk/pull/13540#discussion_r1172974579