Skip to content

8364135: JPEGImageReader.getImageTypes() should throw exception for negative image index #26522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,12 @@ private void checkTablesOnly() throws IOException {
tablesOnlyChecked = true;
}

private void verifyImageIndex(int imageIndex) {
if (imageIndex < minIndex) {
throw new IndexOutOfBoundsException("imageIndex < " + minIndex);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we irrespectively throw IOBE or check for seekForwardOnly before throwing the exception

* <p> The {@code seekForwardOnly} parameter controls whether
* the value returned by {@code getMinIndex} will be
* increased as each image (or thumbnail, or image metadata) is
* read. If {@code seekForwardOnly} is true, then a call to
* {@code read(index)} will throw an
* {@code IndexOutOfBoundsException} if {@code index < this.minIndex};
* otherwise, the value of
* {@code minIndex} will be set to {@code index}. If

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If seekForwardOnly is set to true and if we we have moved to n image, then getting details for n-1 image should throw IOOBE. minIndex will be updated accordingly when seekForwardOnly is set to true, so nothing extra needs to be done from our side.

Naming the function as checkNegativeImageIndex is not right, so i have updated the function name.

}
}

@Override
public int getNumImages(boolean allowSearch) throws IOException {
setThreadLock();
Expand Down Expand Up @@ -497,9 +503,7 @@ private void gotoImage(int imageIndex) throws IOException {
if (iis == null) {
throw new IllegalStateException("Input not set");
}
if (imageIndex < minIndex) {
throw new IndexOutOfBoundsException();
}
verifyImageIndex(imageIndex);
if (!tablesOnlyChecked) {
checkTablesOnly();
}
Expand Down Expand Up @@ -840,6 +844,7 @@ private void setImageData(int width,

@Override
public int getWidth(int imageIndex) throws IOException {
verifyImageIndex(imageIndex);
setThreadLock();
Comment on lines +847 to 848
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether accessing minIndex inside verifyImageIndex requires a lock.

try {
if (currentImage != imageIndex) {
Expand All @@ -854,6 +859,7 @@ public int getWidth(int imageIndex) throws IOException {

@Override
public int getHeight(int imageIndex) throws IOException {
verifyImageIndex(imageIndex);
setThreadLock();
try {
if (currentImage != imageIndex) {
Expand Down Expand Up @@ -884,6 +890,7 @@ private ImageTypeProducer getImageType(int code) {
@Override
public ImageTypeSpecifier getRawImageType(int imageIndex)
throws IOException {
verifyImageIndex(imageIndex);
setThreadLock();
try {
if (currentImage != imageIndex) {
Expand All @@ -902,6 +909,7 @@ public ImageTypeSpecifier getRawImageType(int imageIndex)
@Override
public Iterator<ImageTypeSpecifier> getImageTypes(int imageIndex)
throws IOException {
verifyImageIndex(imageIndex);
setThreadLock();
try {
return getImageTypesOnThread(imageIndex);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/**
/*

Let us not use the javadoc comment syntax for jtreg tags.

* @test
* @bug 8364135
* @summary Test verifies that jpeg image reader throws
* IndexOutOfBoundsException when "-1" image index is used.
* @run main JpegNegativeImageIndexTest
*/

import java.io.IOException;
import java.util.Iterator;
import java.util.Objects;

import javax.imageio.ImageIO;
import javax.imageio.ImageReader;
import javax.imageio.ImageTypeSpecifier;

public class JpegNegativeImageIndexTest {

static boolean passed;

public static void main(String[] args) throws IOException {
Iterator<ImageReader> readers =
ImageIO.getImageReadersByFormatName("jpeg");
if (!readers.hasNext()) {
throw new RuntimeException("No jpeg image readers found");
}

ImageReader ir = readers.next();
try {
// Iterate through all functions where we don't have sufficient
// checks for -1 index
Iterator<ImageTypeSpecifier> types = ir.getImageTypes(-1);
int width = ir.getWidth(-1);
int height = ir.getHeight(-1);
ImageTypeSpecifier specifier = ir.getRawImageType(-1);
} catch (IndexOutOfBoundsException e) {
if (Objects.equals(e.getMessage(), "imageIndex < 0")) {
Comment on lines +58 to +60
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't iterate over all readers but only the first one, throw the error before the catch block.

Inside the catch block, if the message isn't equal to the expected one, throw the error right away.

This allows removing indirection of using the passed field.

passed = true;
}
}
if (!passed) {
throw new RuntimeException("JpegImageReader didn't throw required"
+ " IndexOutOfBoundsException for -1 image index");
}
}
}