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

[Roger Riggs]

On Wed, 26 Apr 2023 11:55:23 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.
>
> Volker Simonis has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8306461
>  - Simplified exception message and fixed test to check the correct message
>  - Addresed review comments of @turbanoff, @shipilev and @RogerRiggs
>  - 8306461: ObjectInputStream::readObject() should handle negative array sizes without throwing NegativeArraySizeExceptions

A bit more investigation is needed.   
I noticed that ObjectInputStream.checkArray throws NegativeArraySizeException if length is < 0 before calling filterCheck.
The checkArray method is called via SharedSecrets to check array sizes on some of the collection classes during deserialization.
Likely, the change should be extended to cover negative lengths in those cases too.

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

Changes requested by rriggs (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13540#pullrequestreview-1401913022