This is an automated email from the ASF dual-hosted git repository. desruisseaux pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
commit 7799cafa4aff6be0a393fb2f05496879d1aa6821 Author: Martin Desruisseaux <[email protected]> AuthorDate: Mon Jul 28 15:46:02 2025 +0200 Make the GeoTIFF writer more robust to malformed rasters. --- .../apache/sis/storage/geotiff/GeoTiffStore.java | 4 +-- .../org/apache/sis/storage/geotiff/writer/ZIP.java | 26 ++++++++++++-- .../org/apache/sis/storage/geotiff/WriterTest.java | 41 +++++++++++++++++----- .../apache/sis/io/stream/HyperRectangleWriter.java | 15 +++++--- .../sis/io/stream/SubsampledRectangleWriter.java | 26 +++++++------- 5 files changed, 80 insertions(+), 32 deletions(-) diff --git a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/GeoTiffStore.java b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/GeoTiffStore.java index fc8937d54b..7871b0c6c5 100644 --- a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/GeoTiffStore.java +++ b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/GeoTiffStore.java @@ -743,9 +743,9 @@ public class GeoTiffStore extends DataStore implements Aggregate { reader.offsetOfWrittenIFD(offsetIFD); } index = writer.imageIndex++; - } catch (RasterFormatException | ArithmeticException e) { + } catch (RasterFormatException | ArithmeticException | IllegalArgumentException e) { throw new IncompatibleResourceException(cannotWrite(), e).addAspect("raster"); - } catch (IOException e) { + } catch (RuntimeException | IOException e) { throw new DataStoreException(cannotWrite(), e); } if (components != null) { diff --git a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/writer/ZIP.java b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/writer/ZIP.java index 291877e50c..2befb2f272 100644 --- a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/writer/ZIP.java +++ b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/writer/ZIP.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.util.zip.Deflater; import org.apache.sis.io.stream.ChannelDataOutput; +import org.apache.sis.storage.geotiff.base.Resources; /** @@ -74,15 +75,34 @@ final class ZIP extends CompressionChannel { * But providing this information in advance may hopefully help the deflater. The condition * can be true when raster data are sent directly to this compressor, without `this.buffer`. */ - if (deflater.getBytesRead() >= length - remaining) { + final long numBytes = deflater.getBytesRead() + remaining; + if (numBytes >= length) { + /* + * To be strict, we shoud raise an exception if `deflater.finished()` is true. + * But it is safer to do this check indirectly in the following loop instead. + */ deflater.finish(); } while (remaining > 0) { assert !deflater.needsInput(); output.ensureBufferAccepts(Math.min(remaining, target.capacity())); target.limit(target.capacity()); // Allow the use of all available space. - deflater.deflate(target); - target.limit(target.position()); // Bytes after the position are not valid. + try { + if (deflater.deflate(target) == 0) { + /* + * According Javadoc, it may occur if `deflater.needsInput()` needs to be checked. + * But we already know that its value should be `false`. The other possibility is + * that `deflater.finished()` is true. The latter may happen if we determined that + * a call to this `write(…)` should be the last one, but this method is nevertheless + * invoked again. It may happen when there is inconsistency in the computation using + * pixel stride and scanline stride, sometime because of malformed sample model. + */ + throw new IOException(Resources.forLocale(null) + .getString(Resources.Keys.UnexpectedTileLength_2, length, numBytes)); + } + } finally { + target.limit(target.position()); // Bytes after the position are not valid. + } remaining = source.remaining(); } return source.position() - start; // Number from caller's perspective (it doesn't know about compression). diff --git a/endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/WriterTest.java b/endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/WriterTest.java index ab44f35b7f..0eb61e9e77 100644 --- a/endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/WriterTest.java +++ b/endorsed/src/org.apache.sis.storage.geotiff/test/org/apache/sis/storage/geotiff/WriterTest.java @@ -70,6 +70,11 @@ public final class WriterTest extends TestCase { */ private static final int TILE_WIDTH = 7, TILE_HEIGHT = 5; + /** + * Minimal buffer size. Shall be smaller than {@code TILE_WIDTH * TILE_HEIGHT}. + */ + private static final int MIN_BUFFER_SIZE = 20; + /** * The image to write. */ @@ -111,7 +116,7 @@ public final class WriterTest extends TestCase { /** * Initializes the test with a tiled image and a GeoTIFF writer. * The image is created with some random properties (pixel and tile coordinates) but verifiable pixel values. - * The writer uses a buffer of random size. + * The writer uses a buffer of random size, potentially smaller than the tile size but not always. * * @param dataType sample data type as one of the {@link java.awt.image.DataBuffer} constants. * @param order whether to use little endian or big endian byte order. @@ -142,11 +147,11 @@ public final class WriterTest extends TestCase { image.validate(); image.initializeAllTiles(); output = new ByteArrayChannel(new byte[image.getWidth() * image.getHeight() * numBands * type.bytes() + 800], false); - var d = new ChannelDataOutput("TIFF", output, ByteBuffer.allocate(random.nextInt(128) + 20).order(order)); - var c = new StorageConnector(d); - c.setOption(FormatModifier.OPTION_KEY, options); - c.setOption(Compression.OPTION_KEY, Compression.NONE); - store = new GeoTiffStore(null, c); + final var buffer = ByteBuffer.allocate(random.nextInt(128) + MIN_BUFFER_SIZE).order(order); + final var connector = new StorageConnector(new ChannelDataOutput("TIFF", output, buffer)); + connector.setOption(FormatModifier.OPTION_KEY, options); + connector.setOption(Compression.OPTION_KEY, Compression.NONE); + store = new GeoTiffStore(null, connector); data = output.toBuffer().order(order); } @@ -231,7 +236,7 @@ public final class WriterTest extends TestCase { } /** - * Tests the writing an RGB image made of a single tile with pixels on (8,8,8) bits. + * Tests the writing an <abbr>RGB</abbr> image made of a single tile with pixels on (8,8,8) bits. * * @throws IOException should never happen since the tests are writing in memory. * @throws DataStoreException if the image is incompatible with writer capability. @@ -249,6 +254,19 @@ public final class WriterTest extends TestCase { store.close(); } + /** + * Tests the writing an RGB image made of a single tile with pixels on (8,8,8) bits. + * + * @throws IOException should never happen since the tests are writing in memory. + * @throws DataStoreException if the image is incompatible with writer capability. + */ + @Test + public void testUntiledMultiBandsOfDoubles() throws IOException, DataStoreException { + initialize(DataType.DOUBLE, ByteOrder.LITTLE_ENDIAN, false, 2, 1, 1, FormatModifier.ANY_TILE_SIZE); + writeImage(); + verifyHeader(false, IOBase.LITTLE_ENDIAN); + } + /** * Tests writing an image with GeoTIFF data. * @@ -408,7 +426,11 @@ public final class WriterTest extends TestCase { * This is necessary for allowing the {@code isShort & isBigEndian} test to work. */ private static Object compact(final short[] array) { - return (array.length == 1) ? array[0] : array; + switch (array.length) { + default: return array; + case 1: return array[0]; + case 2: throw new UnsupportedOperationException("Not supported by current tests."); + } } /** @@ -454,8 +476,9 @@ public final class WriterTest extends TestCase { /** * Verifies the sample values. Expected values are of the form "BTYX" where B is the band (starting with 1), * T is the tile index (starting with 1), and X and Y are pixel coordinates starting with 0. + * This method assumes that the sample values are stored as bytes. * - * @param numBands number of bands in each tile. + * @param numBands number of bands in each tile. */ private void verifySampleValues(final int numBands) { @SuppressWarnings("LocalVariableHidesMemberVariable") diff --git a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/HyperRectangleWriter.java b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/HyperRectangleWriter.java index 2929424305..61aa85eeb3 100644 --- a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/HyperRectangleWriter.java +++ b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/HyperRectangleWriter.java @@ -248,13 +248,18 @@ public class HyperRectangleWriter { regionLower[0] = Math.floorDiv(regionLower[0] * pixelBitStride, dataSize); regionUpper[0] = JDK18.ceilDiv(regionUpper[0] * pixelBitStride, dataSize); } - var subset = new Region(sourceSize, regionLower, regionUpper, new long[] {1,1}); + final var subset = new Region(sourceSize, regionLower, regionUpper, new long[] {1,1}); length = subset.length; - if (bandOffsets == null || (bandOffsets.length == pixelStride && ArraysExt.isRange(0, bandOffsets))) { - return new HyperRectangleWriter(subset); - } else { - return new SubsampledRectangleWriter(subset, bandOffsets, pixelStride); + if (bandOffsets != null) { + final int numBands = bandOffsets.length; + if (numBands != pixelStride || !ArraysExt.isRange(0, bandOffsets)) { + if (numBands != 1) { + ArgumentChecks.ensureBetween("pixelStride", numBands, scanlineStride, pixelStride); + } + return new SubsampledRectangleWriter(subset, bandOffsets, pixelStride); + } } + return new HyperRectangleWriter(subset); } /** diff --git a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/SubsampledRectangleWriter.java b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/SubsampledRectangleWriter.java index 89c00f3712..72d2c12397 100644 --- a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/SubsampledRectangleWriter.java +++ b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/SubsampledRectangleWriter.java @@ -48,7 +48,7 @@ final class SubsampledRectangleWriter extends HyperRectangleWriter { * @param pixelStride number of bands in a pixel. * @throws ArithmeticException if the region is too large. */ - public SubsampledRectangleWriter(final Region region, final int[] bandOffsets, final int pixelStride) { + SubsampledRectangleWriter(final Region region, final int[] bandOffsets, final int pixelStride) { super(region); this.bandOffsets = bandOffsets; this.pixelStride = pixelStride; @@ -142,7 +142,7 @@ final class SubsampledRectangleWriter extends HyperRectangleWriter { public void write(final ChannelDataOutput output, final byte[] data, int offset, final boolean direct) throws IOException { if (direct) throw new UnsupportedOperationException(); write(output, offset, Byte.BYTES, new Data() { - /** Fill the buffer with pixels made of a single sample value. */ + /** Fills the buffer with pixels made of a single sample value. */ @Override int fill(final ByteBuffer target, int index, final int stride) { while (target.hasRemaining()) { target.put(data[index]); @@ -151,7 +151,7 @@ final class SubsampledRectangleWriter extends HyperRectangleWriter { return index; } - /** Fill the buffer with pixels made of multiple sample values. */ + /** Fills the buffer with pixels made of multiple sample values. */ @Override int fill(final ByteBuffer target, int index, final int[] bands, final int stride) { while (target.hasRemaining()) { for (int b : bands) { @@ -175,7 +175,7 @@ final class SubsampledRectangleWriter extends HyperRectangleWriter { @Override public void write(final ChannelDataOutput output, final short[] data, int offset) throws IOException { write(output, offset, Short.BYTES, new Data() { - /** Fill the buffer with pixels made of a single sample value. */ + /** Fills the buffer with pixels made of a single sample value. */ @Override int fill(final ByteBuffer target, int index, final int stride) { while (target.hasRemaining()) { target.putShort(data[index]); @@ -184,7 +184,7 @@ final class SubsampledRectangleWriter extends HyperRectangleWriter { return index; } - /** Fill the buffer with pixels made of multiple sample values. */ + /** Fills the buffer with pixels made of multiple sample values. */ @Override int fill(final ByteBuffer target, int index, final int[] bands, final int stride) { while (target.hasRemaining()) { for (int b : bands) { @@ -208,7 +208,7 @@ final class SubsampledRectangleWriter extends HyperRectangleWriter { @Override public void write(final ChannelDataOutput output, final int[] data, int offset) throws IOException { write(output, offset, Integer.BYTES, new Data() { - /** Fill the buffer with pixels made of a single sample value. */ + /** Fills the buffer with pixels made of a single sample value. */ @Override int fill(final ByteBuffer target, int index, final int stride) { while (target.hasRemaining()) { target.putInt(data[index]); @@ -217,7 +217,7 @@ final class SubsampledRectangleWriter extends HyperRectangleWriter { return index; } - /** Fill the buffer with pixels made of multiple sample values. */ + /** Fills the buffer with pixels made of multiple sample values. */ @Override int fill(final ByteBuffer target, int index, final int[] bands, final int stride) { while (target.hasRemaining()) { for (int b : bands) { @@ -241,7 +241,7 @@ final class SubsampledRectangleWriter extends HyperRectangleWriter { @Override public void write(final ChannelDataOutput output, final long[] data, int offset) throws IOException { write(output, offset, Long.BYTES, new Data() { - /** Fill the buffer with pixels made of a single sample value. */ + /** Fills the buffer with pixels made of a single sample value. */ @Override int fill(final ByteBuffer target, int index, final int stride) { while (target.hasRemaining()) { target.putLong(data[index]); @@ -250,7 +250,7 @@ final class SubsampledRectangleWriter extends HyperRectangleWriter { return index; } - /** Fill the buffer with pixels made of multiple sample values. */ + /** Fills the buffer with pixels made of multiple sample values. */ @Override int fill(final ByteBuffer target, int index, final int[] bands, final int stride) { while (target.hasRemaining()) { for (int b : bands) { @@ -274,7 +274,7 @@ final class SubsampledRectangleWriter extends HyperRectangleWriter { @Override public void write(final ChannelDataOutput output, final float[] data, int offset) throws IOException { write(output, offset, Float.BYTES, new Data() { - /** Fill the buffer with pixels made of a single sample value. */ + /** Fills the buffer with pixels made of a single sample value. */ @Override int fill(final ByteBuffer target, int index, final int stride) { while (target.hasRemaining()) { target.putFloat(data[index]); @@ -283,7 +283,7 @@ final class SubsampledRectangleWriter extends HyperRectangleWriter { return index; } - /** Fill the buffer with pixels made of multiple sample values. */ + /** Fills the buffer with pixels made of multiple sample values. */ @Override int fill(final ByteBuffer target, int index, final int[] bands, final int stride) { while (target.hasRemaining()) { for (int b : bands) { @@ -307,7 +307,7 @@ final class SubsampledRectangleWriter extends HyperRectangleWriter { @Override public void write(final ChannelDataOutput output, final double[] data, int offset) throws IOException { write(output, offset, Double.BYTES, new Data() { - /** Fill the buffer with pixels made of a single sample value. */ + /** Fills the buffer with pixels made of a single sample value. */ @Override int fill(final ByteBuffer target, int index, final int stride) { while (target.hasRemaining()) { target.putDouble(data[index]); @@ -316,7 +316,7 @@ final class SubsampledRectangleWriter extends HyperRectangleWriter { return index; } - /** Fill the buffer with pixels made of multiple sample values. */ + /** Fills the buffer with pixels made of multiple sample values. */ @Override int fill(final ByteBuffer target, int index, final int[] bands, final int stride) { while (target.hasRemaining()) { for (int b : bands) {
