RFR: 8323108: BufferedImage.setData(Raster) should not cast float and double values to integers
Martin Desruisseaux
duke at openjdk.org
Sun Jan 7 23:32:53 UTC 2024
On Mon, 8 May 2023 18:44:00 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:
>> I do not know why `WritableRaster.setRect(Raster)` computes intersection with its own code in the method body instead of delegating to `Rectangle.intersection(Rectangle)` in the same way as `BufferedImage.setData(Raster)` does. For now I see no technical reason. I'm tempted to add a commit to this pull request for replacing the intersection calculation of `WritableRaster.setRect(Raster)`, but abstained for now because I though that reviewers or sponsors may want to minimize the changes. However I believe that this replacement would be desirable at least for the extra robustness of `Rectangle` regarding underflow and overflow.
>
> Can we prove the overflow issue there by some test? If yes then probably it would be useful to fix it as well, probably even as separate bug which can be later backported.
In theory the following code would demonstrate the overflow:
var sm = new BandedSampleModel(DataBuffer.TYPE_BYTE, 100, 100, 1);
var r1 = Raster.createWritableRaster(sm, new Point(Integer.MAX_VALUE - 150, 0));
var r2 = Raster.createWritableRaster(sm, new Point(Integer.MAX_VALUE - 50, 0)); // Overflow on X.
r1.setRect(r2);
It practice it does not happen so easily because the `Raster` constructor detects in advance the overflow and throws a `RasterFormatException` saying _"overflow condition for X coordinates of Raster"_. However because the `Raster.minX` field is protected and non-final, the constructor check can be bypassed as below:
var r2 = new WritableRaster(sm, new Point()) {
{ // Constructor
minX = Integer.MAX_VALUE - 50;
sampleModelTranslateX = minX;
}
};
In which case the overflow happens and manifest itself with an `ArrayIndexOutOfBoundsException`. I do not know if this is considered a too convolved case for being worth a fix. However using the `BufferedImage` code, in addition of being robust to this case, would also be simpler.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13797#discussion_r1192681645
More information about the client-libs-dev
mailing list