On Fri, 3 Oct 2025 19:35:21 GMT, Phil Race <[email protected]> wrote:
>> Updates the specification of Raster.createPackedRaster(..) methods and also
>> the implementation of these to explicitly check the specified conditions
>> rather than relying on them being passed up from some other API.
>>
>> Also one each of the createInterleavedRaster(..) and createBandedRaster(..)
>> methods have an update
>>
>> createInterleavedRaster(...) to address that even if WxH does not exceed
>> Integer.MAX_VALUE, the storage needed still might.
>>
>> createBandedRaster(..) to address
>> - a not explicitly specified or tested case that WxH must not overflow
>> - a not explicitly tested NPE case if bandMasks is null. This was relying on
>> a de-reference and the actual check for it being null would have thrown the
>> wrong exception type (AIOBE) - meaning not the one NPE actually specified
>> and thrown.
>> There's an unfortunate oddity with this that of bankIndices is null it
>> specifies and throws AIOBE, but I didn't think it worth changing either of
>> these to make them consistent.
>>
>> The existing CreateRasterExceptionTest.java is updated to verify all these
>> assertions.
>> In almost all cases the behaviour is unchanged, it is just now properly
>> specified and verified up front.
>>
>> There are only 2 sub-tests which would fail on JDK 25
>> The test can also be run (and pass) there without this fix because it
>> explicitly handles those 2 cases which are reasonable behavioral changes
>> - One for the createInterleavedRaster() update where the unchecked case
>> could end up causing a NegativeArraySizeException. This seems like a merited
>> change.
>> - One for an unlikely case where if an app specified 0 app to
>> createPackedRaster, divide by zero would happen.
>>
>> The CSR is now ready for review https://bugs.openjdk.org/browse/JDK-8369131
>
> Phil Race has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8369129
src/java.desktop/share/classes/java/awt/image/Raster.java line 310:
> 308: throw new IllegalArgumentException("size too large to store
> image");
> 309: }
> 310: int size = (int)lsz;
Isn't it strange that in this method we reject "empty" images by requiring w
and h to be greater than 0, but at the same time accept 0 as scanlineStride and
pixelStride? This may result in empty image as well (size == 0), I think it
will be rejected later but still should we check it here as well?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27627#discussion_r2418526427
