From 5b1765f25dc24d89046d2acf08859b68301d145e Mon Sep 17 00:00:00 2001 From: Brian Burkhalter Date: Fri, 11 Dec 2015 11:38:18 -0800 Subject: [PATCH] 8144245: [PIT] javax/imageio/plugins/shared/WriteAfterAbort.java Reset stream position after abort; change IAEs to NPEs. Reviewed-by: prr, serb --- .../com/sun/imageio/plugins/tiff/TIFFIFD.java | 9 +- .../imageio/plugins/tiff/TIFFImageWriter.java | 56 ++- jdk/test/ProblemList.txt | 2 - .../plugins/shared/WriteAfterAbort.java | 16 +- .../tiff/WriteToSequenceAfterAbort.java | 376 ++++++++++++++++++ 5 files changed, 449 insertions(+), 10 deletions(-) create mode 100644 jdk/test/javax/imageio/plugins/tiff/WriteToSequenceAfterAbort.java diff --git a/jdk/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFIFD.java b/jdk/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFIFD.java index 6803f634762..7c14f8daf85 100644 --- a/jdk/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFIFD.java +++ b/jdk/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFIFD.java @@ -472,6 +472,13 @@ public class TIFFIFD extends TIFFDirectory { // Read tag number, value type, and value count. int tagNumber = stream.readUnsignedShort(); int type = stream.readUnsignedShort(); + int sizeOfType; + try { + sizeOfType = TIFFTag.getSizeOfType(type); + } catch (IllegalArgumentException e) { + throw new IIOException("Illegal type " + type + + " for tag number " + tagNumber, e); + } int count = (int)stream.readUnsignedInt(); // Get the associated TIFFTag. @@ -510,7 +517,7 @@ public class TIFFIFD extends TIFFDirectory { } } - int size = count*TIFFTag.getSizeOfType(type); + int size = count*sizeOfType; if (size > 4 || tag.isIFDPointer()) { // The IFD entry value is a pointer to the actual field value. long offset = stream.readUnsignedInt(); diff --git a/jdk/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFImageWriter.java b/jdk/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFImageWriter.java index 08c1f0ecb9a..6a90e0677c9 100644 --- a/jdk/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFImageWriter.java +++ b/jdk/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFImageWriter.java @@ -205,6 +205,10 @@ public class TIFFImageWriter extends ImageWriter { // Next available space. private long nextSpace = 0L; + private long prevStreamPosition; + private long prevHeaderPosition; + private long prevNextSpace; + // Whether a sequence is being written. private boolean isWritingSequence = false; private boolean isInsertingEmpty = false; @@ -2300,7 +2304,14 @@ public class TIFFImageWriter extends ImageWriter { public void write(IIOMetadata sm, IIOImage iioimage, ImageWriteParam p) throws IOException { + if (stream == null) { + throw new IllegalStateException("output == null!"); + } + markPositions(); write(sm, iioimage, p, true, true); + if (abortRequested()) { + resetPositions(); + } } private void writeHeader() throws IOException { @@ -2333,7 +2344,7 @@ public class TIFFImageWriter extends ImageWriter { throw new IllegalStateException("output == null!"); } if (iioimage == null) { - throw new NullPointerException("image == null!"); + throw new IllegalArgumentException("image == null!"); } if(iioimage.hasRaster() && !canWriteRasters()) { throw new UnsupportedOperationException @@ -2767,7 +2778,7 @@ public class TIFFImageWriter extends ImageWriter { throw new IllegalStateException("Output not set!"); } if (image == null) { - throw new NullPointerException("image == null!"); + throw new IllegalArgumentException("image == null!"); } // Locate the position of the old IFD (ifd) and the location @@ -2779,9 +2790,16 @@ public class TIFFImageWriter extends ImageWriter { // imageIndex is < -1 or is too big thereby satisfying the spec. locateIFD(imageIndex, ifdpos, ifd); + markPositions(); + // Seek to the position containing the pointer to the old IFD. stream.seek(ifdpos[0]); + // Save the previous pointer value in case of abort. + stream.mark(); + long prevPointerValue = stream.readUnsignedInt(); + stream.reset(); + // Update next space pointer in anticipation of next write. if(ifdpos[0] + 4 > nextSpace) { nextSpace = ifdpos[0] + 4; @@ -2805,6 +2823,12 @@ public class TIFFImageWriter extends ImageWriter { // Update the new IFD to point to the old IFD. stream.writeInt((int)ifd[0]); // Don't need to update nextSpace here as already done in write(). + + if (abortRequested()) { + stream.seek(ifdpos[0]); + stream.writeInt((int)prevPointerValue); + resetPositions(); + } } // ----- BEGIN insert/writeEmpty methods ----- @@ -2834,7 +2858,7 @@ public class TIFFImageWriter extends ImageWriter { } if(imageType == null) { - throw new NullPointerException("imageType == null!"); + throw new IllegalArgumentException("imageType == null!"); } if(width < 1 || height < 1) { @@ -2891,6 +2915,10 @@ public class TIFFImageWriter extends ImageWriter { IIOMetadata imageMetadata, List thumbnails, ImageWriteParam param) throws IOException { + if (stream == null) { + throw new IllegalStateException("output == null!"); + } + checkParamsEmpty(imageType, width, height, thumbnails); this.isWritingEmpty = true; @@ -2901,8 +2929,12 @@ public class TIFFImageWriter extends ImageWriter { 0, 0, emptySM.getWidth(), emptySM.getHeight(), emptySM, imageType.getColorModel()); + markPositions(); write(streamMetadata, new IIOImage(emptyImage, null, imageMetadata), param, true, false); + if (abortRequested()) { + resetPositions(); + } } public void endInsertEmpty() throws IOException { @@ -3015,7 +3047,7 @@ public class TIFFImageWriter extends ImageWriter { throw new IllegalStateException("Output not set!"); } if (region == null) { - throw new NullPointerException("region == null!"); + throw new IllegalArgumentException("region == null!"); } if (region.getWidth() < 1) { throw new IllegalArgumentException("region.getWidth() < 1!"); @@ -3200,7 +3232,7 @@ public class TIFFImageWriter extends ImageWriter { } if (image == null) { - throw new NullPointerException("image == null!"); + throw new IllegalArgumentException("image == null!"); } if (!inReplacePixelsNest) { @@ -3559,6 +3591,20 @@ public class TIFFImageWriter extends ImageWriter { // ----- END replacePixels methods ----- + // Save stream positions for use when aborted. + private void markPositions() throws IOException { + prevStreamPosition = stream.getStreamPosition(); + prevHeaderPosition = headerPosition; + prevNextSpace = nextSpace; + } + + // Reset to positions saved by markPositions(). + private void resetPositions() throws IOException { + stream.seek(prevStreamPosition); + headerPosition = prevHeaderPosition; + nextSpace = prevNextSpace; + } + public void reset() { super.reset(); diff --git a/jdk/test/ProblemList.txt b/jdk/test/ProblemList.txt index 51abaacbf53..2a18c6adef2 100644 --- a/jdk/test/ProblemList.txt +++ b/jdk/test/ProblemList.txt @@ -310,8 +310,6 @@ javax/sound/midi/Gervill/SoftProvider/GetDevice.java generic-all # jdk_imageio -javax/imageio/plugins/shared/WriteAfterAbort.java generic-all - ############################################################################ # jdk_swing diff --git a/jdk/test/javax/imageio/plugins/shared/WriteAfterAbort.java b/jdk/test/javax/imageio/plugins/shared/WriteAfterAbort.java index 4b503d2fe6f..f1be068cdfe 100644 --- a/jdk/test/javax/imageio/plugins/shared/WriteAfterAbort.java +++ b/jdk/test/javax/imageio/plugins/shared/WriteAfterAbort.java @@ -130,13 +130,25 @@ public final class WriteAfterAbort implements IIOWriteProgressListener { ImageWriterSpi.class, provider -> true, true); // Validates all supported ImageWriters + int numFailures = 0; while (iter.hasNext()) { final WriteAfterAbort writeAfterAbort = new WriteAfterAbort(); final ImageWriter writer = iter.next().createWriterInstance(); System.out.println("ImageWriter = " + writer); - writeAfterAbort.test(writer); + try { + writeAfterAbort.test(writer); + } catch (Exception e) { + System.err.println("Test failed for \"" + + writer.getOriginatingProvider().getFormatNames()[0] + + "\" format."); + numFailures++; + } + } + if (numFailures == 0) { + System.out.println("Test passed."); + } else { + throw new RuntimeException("Test failed."); } - System.out.println("Test passed"); } // Callbacks diff --git a/jdk/test/javax/imageio/plugins/tiff/WriteToSequenceAfterAbort.java b/jdk/test/javax/imageio/plugins/tiff/WriteToSequenceAfterAbort.java new file mode 100644 index 00000000000..55461f36f24 --- /dev/null +++ b/jdk/test/javax/imageio/plugins/tiff/WriteToSequenceAfterAbort.java @@ -0,0 +1,376 @@ +/* + * Copyright (c) 2015, 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. + */ + +import java.awt.Color; +import java.awt.Graphics2D; +import java.awt.Rectangle; +import java.awt.image.BufferedImage; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; + +import javax.imageio.ImageIO; +import javax.imageio.ImageWriter; +import javax.imageio.event.IIOWriteProgressListener; +import javax.imageio.stream.ImageOutputStream; + +import static java.awt.image.BufferedImage.TYPE_BYTE_BINARY; +import java.awt.image.ColorModel; +import java.awt.image.Raster; +import java.awt.image.RenderedImage; +import java.awt.image.SampleModel; +import java.awt.image.WritableRaster; +import java.util.Vector; +import javax.imageio.IIOImage; +import javax.imageio.ImageReader; +import javax.imageio.stream.ImageInputStream; + +/** + * @test + * @bug 8144245 + * @summary Ensure aborting write works properly for a TIFF sequence. + */ +public final class WriteToSequenceAfterAbort implements IIOWriteProgressListener { + + private volatile boolean abortFlag = true; + private volatile boolean isAbortCalled; + private volatile boolean isCompleteCalled; + private volatile boolean isProgressCalled; + private volatile boolean isStartedCalled; + private static final int WIDTH = 100; + private static final int HEIGHT = 100; + private static final int NUM_TILES_XY = 3; + + private class TiledImage implements RenderedImage { + private final BufferedImage tile; + private final BufferedImage image; + private final int numXTiles, numYTiles; + private boolean isImageInitialized = false; + + TiledImage(BufferedImage tile, int numXTiles, int numYTiles) { + this.tile = tile; + this.numXTiles = numXTiles; + this.numYTiles = numYTiles; + image = new BufferedImage(getWidth(), getHeight(), tile.getType()); + } + + @Override + public Vector getSources() { + return null; + } + + @Override + public Object getProperty(String string) { + return java.awt.Image.UndefinedProperty; + } + + @Override + public String[] getPropertyNames() { + return new String[0]; + } + + @Override + public ColorModel getColorModel() { + return tile.getColorModel(); + } + + @Override + public SampleModel getSampleModel() { + return tile.getSampleModel(); + } + + @Override + public int getWidth() { + return numXTiles*tile.getWidth(); + } + + @Override + public int getHeight() { + return numYTiles*tile.getHeight(); + } + + @Override + public int getMinX() { + return 0; + } + + @Override + public int getMinY() { + return 0; + } + + @Override + public int getNumXTiles() { + return numXTiles; + } + + @Override + public int getNumYTiles() { + return numYTiles; + } + + @Override + public int getMinTileX() { + return 0; + } + + @Override + public int getMinTileY() { + return 0; + } + + @Override + public int getTileWidth() { + return tile.getWidth(); + } + + @Override + public int getTileHeight() { + return tile.getHeight(); + } + + @Override + public int getTileGridXOffset() { + return 0; + } + + @Override + public int getTileGridYOffset() { + return 0; + } + + @Override + public Raster getTile(int x, int y) { + WritableRaster r = tile.getRaster(); + return r.createWritableTranslatedChild(x*tile.getWidth(), + y*tile.getHeight()); + } + + @Override + public Raster getData() { + return getAsBufferedImage().getData(); + } + + @Override + public Raster getData(Rectangle r) { + return getAsBufferedImage().getData(r); + } + + @Override + public WritableRaster copyData(WritableRaster wr) { + return getAsBufferedImage().copyData(wr); + } + + public BufferedImage getAsBufferedImage() { + synchronized (image) { + if (!isImageInitialized) { + int tx0 = getMinTileX(), ty0 = getMinTileY(); + int txN = tx0 + getNumXTiles(), tyN = ty0 + getNumYTiles(); + for (int j = ty0; j < tyN; j++) { + for (int i = tx0; i < txN; i++) { + image.setData(getTile(i, j)); + } + } + } + isImageInitialized = true; + } + return image; + } + } + + private void test(final ImageWriter writer) throws IOException { + String suffix = writer.getOriginatingProvider().getFileSuffixes()[0]; + + // Image initialization + BufferedImage imageUpperLeft = + new BufferedImage(WIDTH, HEIGHT, TYPE_BYTE_BINARY); + Graphics2D g = imageUpperLeft.createGraphics(); + g.setColor(Color.WHITE); + g.fillRect(0, 0, WIDTH/2, HEIGHT/2); + g.dispose(); + BufferedImage imageLowerRight = + new BufferedImage(WIDTH, HEIGHT, TYPE_BYTE_BINARY); + g = imageLowerRight.createGraphics(); + g.setColor(Color.WHITE); + g.fillRect(WIDTH/2, HEIGHT/2, WIDTH/2, HEIGHT/2); + g.dispose(); + TiledImage[] images = new TiledImage[] { + new TiledImage(imageUpperLeft, NUM_TILES_XY, NUM_TILES_XY), + new TiledImage(imageUpperLeft, NUM_TILES_XY, NUM_TILES_XY), + new TiledImage(imageLowerRight, NUM_TILES_XY, NUM_TILES_XY), + new TiledImage(imageLowerRight, NUM_TILES_XY, NUM_TILES_XY) + }; + + // File initialization + File file = File.createTempFile("temp", "." + suffix); + file.deleteOnExit(); + FileOutputStream fos = new SkipWriteOnAbortOutputStream(file); + ImageOutputStream ios = ImageIO.createImageOutputStream(fos); + writer.setOutput(ios); + writer.addIIOWriteProgressListener(this); + + writer.prepareWriteSequence(null); + boolean[] abortions = new boolean[] {true, false, true, false}; + for (int i = 0; i < 4; i++) { + abortFlag = abortions[i]; + isAbortCalled = false; + isCompleteCalled = false; + isProgressCalled = false; + isStartedCalled = false; + + TiledImage image = images[i]; + if (abortFlag) { + // This write will be aborted, and file will not be touched + writer.writeToSequence(new IIOImage(image, null, null), null); + if (!isStartedCalled) { + throw new RuntimeException("Started should be called"); + } + if (!isProgressCalled) { + throw new RuntimeException("Progress should be called"); + } + if (!isAbortCalled) { + throw new RuntimeException("Abort should be called"); + } + if (isCompleteCalled) { + throw new RuntimeException("Complete should not be called"); + } + } else { + // This write should be completed successfully and the file should + // contain correct image data. + writer.writeToSequence(new IIOImage(image, null, null), null); + if (!isStartedCalled) { + throw new RuntimeException("Started should be called"); + } + if (!isProgressCalled) { + throw new RuntimeException("Progress should be called"); + } + if (isAbortCalled) { + throw new RuntimeException("Abort should not be called"); + } + if (!isCompleteCalled) { + throw new RuntimeException("Complete should be called"); + } + } + } + + writer.endWriteSequence(); + writer.dispose(); + ios.close(); + + // Validates content of the file. + ImageReader reader = ImageIO.getImageReader(writer); + ImageInputStream iis = ImageIO.createImageInputStream(file); + reader.setInput(iis); + for (int i = 0; i < 2; i++) { + System.out.println("Testing image " + i); + BufferedImage imageRead = reader.read(i); + BufferedImage imageWrite = images[2 * i].getAsBufferedImage(); + for (int x = 0; x < WIDTH; ++x) { + for (int y = 0; y < HEIGHT; ++y) { + if (imageRead.getRGB(x, y) != imageWrite.getRGB(x, y)) { + throw new RuntimeException("Test failed for image " + i); + } + } + } + } + } + + public static void main(final String[] args) throws IOException { + WriteToSequenceAfterAbort writeAfterAbort = new WriteToSequenceAfterAbort(); + ImageWriter writer = ImageIO.getImageWritersByFormatName("TIFF").next(); + writeAfterAbort.test(writer); + System.out.println("Test passed."); + } + + // Callbacks + + @Override + public void imageComplete(ImageWriter source) { + isCompleteCalled = true; + } + + @Override + public void imageProgress(ImageWriter source, float percentageDone) { + isProgressCalled = true; + if (percentageDone > 50 && abortFlag) { + source.abort(); + } + } + + @Override + public void imageStarted(ImageWriter source, int imageIndex) { + isStartedCalled = true; + } + + @Override + public void writeAborted(final ImageWriter source) { + isAbortCalled = true; + } + + @Override + public void thumbnailComplete(ImageWriter source) { + } + + @Override + public void thumbnailProgress(ImageWriter source, float percentageDone) { + } + + @Override + public void thumbnailStarted(ImageWriter source, int imageIndex, + int thumbnailIndex) { + } + + /** + * We need to skip writes on abort, because content of the file after abort + * is undefined. + */ + private class SkipWriteOnAbortOutputStream extends FileOutputStream { + + SkipWriteOnAbortOutputStream(File file) throws FileNotFoundException { + super(file); + } + + @Override + public void write(int b) throws IOException { + if (!abortFlag) { + super.write(b); + } + } + + @Override + public void write(byte[] b) throws IOException { + if (!abortFlag) { + super.write(b); + } + } + + @Override + public void write(byte[] b, int off, int len) throws IOException { + if (!abortFlag) { + super.write(b, off, len); + } + } + } +} +