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

[Volker Simonis]

On Thu, 20 Apr 2023 11:58:50 GMT, Aleksey Shipilev <shade 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 109:
> 
>> 107:                 throw new Exception("ObjectInputStream::readObject() should catch NegativeArraySizeExceptions before filtering", ice);
>> 108:             }
>> 109:             // OK, because a NegativeArraySizeException should be converted into a ObjectStreamException *before* filtering
> 
> Sounds like we should really test for ICE with "Array length ... is negative" here. This would cover the case when filter rejected the class before that exception is thrown.

The test for the `InvalidClassException` created by the object input filter is just above the comment, but I added another check for the correct exception message just for the simple test without filter.

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

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