This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
commit c40700c0c774e7dbe299a2420ffa35e034fd3111 Author: Martin Desruisseaux <[email protected]> AuthorDate: Thu Oct 8 16:12:17 2026 +0200 Fix: invalid TIFF image written when the header contains short texts (2 characters in classical TIFF or 6 characters in BigTIFF). --- .../org/apache/sis/storage/geotiff/Writer.java | 33 ++++++++++++---------- .../apache/sis/storage/geotiff/reader/Type.java | 18 +++++++++++- .../org/apache/sis/storage/geotiff/WriterTest.java | 24 ++++++++++++---- 3 files changed, 53 insertions(+), 22 deletions(-) diff --git a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/Writer.java b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/Writer.java index 14c54d4c84..c31d21cb7c 100644 --- a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/Writer.java +++ b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/Writer.java @@ -76,7 +76,7 @@ import org.opengis.coverage.CannotEvaluateException; * because they are not useful for geospatial applications. This restriction does not reduce the set * of Java2D images that this writer can encode.</p> * - * <p>The TIFF format specification version 6.0 (June 3, 1992) is available + * <p>The <abbr>TIFF</abbr> format specification version 6.0 (June 3, 1992) is available * <a href="https://partners.adobe.com/public/developer/en/tiff/TIFF6.pdf";>here</a>.</p> * * @author Erwan Roussel (Geomatys) @@ -98,7 +98,7 @@ final class Writer extends IOBase implements OverviewIterator, Flushable { static final short TIFF_ULONG = 16; /** - * Sizes of a few TIFF tags used in this writer. + * Sizes of a few <abbr>TIFF</abbr> tags used in this writer, in number of bytes. * * @see #writeTag(short, short, int[]) * @see #writeTag(short, short, double[]) @@ -190,7 +190,7 @@ final class Writer extends IOBase implements OverviewIterator, Flushable { private final Queue<TagValue> largeTagData = new ArrayDeque<>(); /** - * Number of TIFF tag entries in the image being written. + * Number of <abbr>TIFF</abbr> tag entries in the image being written. * This is a temporary information used during the writing of an Image File Directory (IFD). */ private int numberOfTags; @@ -367,7 +367,7 @@ final class Writer extends IOBase implements OverviewIterator, Flushable { largeTagData.clear(); // For making sure that there is no memory retention. } tiles.writeRasters(output); - wordAlign(output); + wordAlign(); tiles.writeOffsetsAndLengths(output); flush(); currentIFD = tiles.nextIFD; // Set only after the operation succeeded. @@ -388,11 +388,11 @@ final class Writer extends IOBase implements OverviewIterator, Flushable { } /** - * Writes the Image File Directory (IFD) of the given image at the current {@link #output} position. + * Writes the Image File Directory (<abbr>IFD</abbr>) of the given image at the current {@link #output} position. * This method does not write the pixel values. Those values must be written by the caller. * This separation makes possible to write directories in any order compared to pixel data. * - * @param image the image for which to write the IFD. + * @param image the image for which to write the <abbr>IFD</abbr>. * @param grid mapping from pixel coordinates to "real world" coordinates, or {@code null} if none. * @param metadata title, author and other information, or {@code null} if none. * @param oveverview whether the image is an overview of another image. @@ -530,7 +530,10 @@ final class Writer extends IOBase implements OverviewIterator, Flushable { tiling.nextIFD = writeOffset(0); for (final TagValue tag : largeTagData) { UpdatableWrite<?> offset = tag.writeHere(output); - if (offset != null) deferredWrites.add(offset); + wordAlign(); + if (offset != null) { + deferredWrites.add(offset); + } } return tiling; } @@ -558,7 +561,7 @@ final class Writer extends IOBase implements OverviewIterator, Flushable { } /** - * Writes a 32-bits or 64-bits offset, depending on whether the format is classic TIFF or BigTIFF. + * Writes a 32-bits or 64-bits offset, depending on whether the format is classic <abbr>TIFF</abbr> or BigTIFF. * * @param offset an initial guess of the offset value. * @return a handler for updating later the offset with its actual value. @@ -572,12 +575,12 @@ final class Writer extends IOBase implements OverviewIterator, Flushable { /** * Forces 16-bits word alignment. - * The TIFF specification requires that tag values are aligned. + * The <abbr>TIFF</abbr> specification requires that tag values are aligned. * * @param channel the channel on which to apply 16-bits word alignment. * @throws IOException if an error occurred while writing to the output stream. */ - private static void wordAlign(final ChannelDataOutput output) throws IOException { + private void wordAlign() throws IOException { if ((output.getStreamPosition() & 1) != 0) { output.writeByte(0); } @@ -646,7 +649,7 @@ final class Writer extends IOBase implements OverviewIterator, Flushable { } /** - * Writes a tag value which is potentially too large for fitting in the IFD entry. + * Writes a tag value which is potentially too large for fitting in the <abbr>IFD</abbr> entry. * * @param tag the code of the tag to write, usually a constant defined by the TIFF specification. * @param type one of the {@link TIFFTag} constants such as {@code TIFF_SHORT} or {@code TIFF_LONG}. @@ -670,7 +673,7 @@ final class Writer extends IOBase implements OverviewIterator, Flushable { * Writes the color map tag. * * @param cm color model from which to read color values. - * @param count number of colors to write, <strong>not</strong> multiplied by 3 for the RGB bands. + * @param count number of colors to write, <strong>not</strong> multiplied by 3 for the <abbr>RGB</abbr> bands. * @throws IOException if an error occurred while writing to the output. */ private void writeColorPalette(final IndexColorModel cm, final long count) throws IOException { @@ -696,8 +699,8 @@ final class Writer extends IOBase implements OverviewIterator, Flushable { } /** - * Writes a tag with string values stored as ASCII characters. - * The list of valid tag code is defined by TIFF specification. + * Writes a tag with string values stored as <abbr>ASCII</abbr> characters. + * The list of valid tag codes is defined by <abbr>TIFF</abbr> specification. * * @param tag the code of the tag to write, usually a constant defined by the TIFF specification. * @param values the values to write, or {@code null} if none. @@ -732,9 +735,9 @@ final class Writer extends IOBase implements OverviewIterator, Flushable { if (c != null) { output.write(c); output.writeByte(0); - wordAlign(output); } } + // Do not invoke `wordAlign()` because the number of bytes written must be exactly `count`. } }); } diff --git a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/reader/Type.java b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/reader/Type.java index 9f45f09b1e..db214e7a58 100644 --- a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/reader/Type.java +++ b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/reader/Type.java @@ -333,15 +333,19 @@ public enum Type { * 8-bits byte that contains a 7-bit ASCII code. In a string of ASCII characters, the last byte must be NUL * (binary zero). The string length (including the NUL byte) is the {@code count} field before the string. * NUL bytes may also appear in the middle of the string for separating its content into multi-strings. + * NUL byte shall appear only once, not counting the padding byte after all strings. + * * <ul> * <li>TIFF name: {@code ASCII}</li> * <li>TIFF code: 2</li> * </ul> + * + * Non-standard extension: if NUL is missing, all remaining characters are taken anyway. */ ASCII(TIFFTag.TIFF_ASCII, Byte.BYTES, false) { @Override public String[] readAsStrings(final ChannelDataInput input, final long length, final Charset charset) throws IOException { final byte[] chars = input.readBytes(Math.toIntExact(length)); - String[] lines = new String[1]; // We will usually have exactly one string. + String[] lines = new String[1]; // We will usually have exactly one string. int count = 0, lower = 0; for (int i=0; i<chars.length; i++) { if (chars[i] == 0) { @@ -349,6 +353,18 @@ public enum Type { lines = Arrays.copyOf(lines, 2*count); } lines[count++] = new String(chars, lower, i-lower, charset); + lower = i + 1; + } + } + // Non-standard extension: take remaining characters if any. + final int n = chars.length - lower; + if (n > 0) { + final String remaining = new String(chars, lower, n, charset).trim(); + if (!remaining.isBlank()) { + if (count >= lines.length) { + return ArraysExt.append(lines, remaining); + } + lines[count++] = remaining; } } return ArraysExt.resize(lines, count); 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 c5779a49e5..f92e6243c3 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 @@ -43,6 +43,9 @@ import org.apache.sis.coverage.grid.GridGeometry; import org.apache.sis.coverage.grid.GridOrientation; import org.apache.sis.image.DataType; import org.apache.sis.image.internal.shared.ColorModelBuilder; +import org.apache.sis.metadata.iso.DefaultMetadata; +import org.apache.sis.metadata.iso.citation.DefaultCitation; +import org.apache.sis.metadata.iso.identification.DefaultDataIdentification; import org.apache.sis.geometry.Envelope2D; // Test dependencies @@ -176,10 +179,19 @@ public final class WriterTest extends TestCase { * @throws DataStoreException if the image is incompatible with writer capability. */ private void writeImage() throws IOException, DataStoreException { - store.append(image, gridGeometry, null); + final var metadata = new DefaultMetadata(); + final var id = new DefaultDataIdentification(); + id.setCitation(new DefaultCitation("abcd")); // Short enough for being stored directly in BigTIFF slot. + assertTrue(metadata.getIdentificationInfo().add(id)); + store.append(image, gridGeometry, metadata); data.clear().limit(Math.toIntExact(output.size())); } + /** + * Common number of tags which will be written, including the metadata added by {@link #writeImage()}. + */ + private static final int COMMON_NUMBER_OF_TAGS = Writer.COMMON_NUMBER_OF_TAGS + 1; + /** * Tests the writing a gray scale image made of a single tile with pixels on 8 bits. * This is the simplest type of image. @@ -193,7 +205,7 @@ public final class WriterTest extends TestCase { FormatModifier.ANY_TILE_SIZE); writeImage(); verifyHeader(false, IOBase.BIG_ENDIAN); - verifyImageFileDirectory(Writer.COMMON_NUMBER_OF_TAGS - 1, // One less tag because stripped layout. + verifyImageFileDirectory(COMMON_NUMBER_OF_TAGS - 1, // One less tag because stripped layout. PHOTOMETRIC_INTERPRETATION_BLACK_IS_ZERO, new short[] {Byte.SIZE}, false); verifySampleValues(1); @@ -212,7 +224,7 @@ public final class WriterTest extends TestCase { FormatModifier.ANY_TILE_SIZE, FormatModifier.BIG_TIFF); writeImage(); verifyHeader(true, IOBase.LITTLE_ENDIAN); - verifyImageFileDirectory(Writer.COMMON_NUMBER_OF_TAGS - 1, // One less tag because stripped layout. + verifyImageFileDirectory(COMMON_NUMBER_OF_TAGS - 1, // One less tag because stripped layout. PHOTOMETRIC_INTERPRETATION_BLACK_IS_ZERO, new short[] {Byte.SIZE}, false); verifySampleValues(1); @@ -231,7 +243,7 @@ public final class WriterTest extends TestCase { initialize(DataType.BYTE, ByteOrder.LITTLE_ENDIAN, false, 1, 3, 4, FormatModifier.ANY_TILE_SIZE); writeImage(); verifyHeader(false, IOBase.LITTLE_ENDIAN); - verifyImageFileDirectory(Writer.COMMON_NUMBER_OF_TAGS, + verifyImageFileDirectory(COMMON_NUMBER_OF_TAGS, PHOTOMETRIC_INTERPRETATION_BLACK_IS_ZERO, new short[] {Byte.SIZE}, true); verifySampleValues(1); @@ -250,7 +262,7 @@ public final class WriterTest extends TestCase { image.setColorModel(new ColorModelBuilder().createRGB(image.getSampleModel())); writeImage(); verifyHeader(false, IOBase.LITTLE_ENDIAN); - verifyImageFileDirectory(Writer.COMMON_NUMBER_OF_TAGS - 1, // One less tag because stripped layout. + verifyImageFileDirectory(COMMON_NUMBER_OF_TAGS - 1, // One less tag because stripped layout. PHOTOMETRIC_INTERPRETATION_RGB, new short[] {Byte.SIZE, Byte.SIZE, Byte.SIZE}, false); verifySampleValues(3); @@ -287,7 +299,7 @@ public final class WriterTest extends TestCase { * So the test cannot expects an exact number of tags. */ int tagCount = data.getShort(data.position()); - assertTrue(tagCount >= Writer.COMMON_NUMBER_OF_TAGS + 3 - 1); // 3 more for RGB, 1 less for strips. + assertTrue(tagCount >= COMMON_NUMBER_OF_TAGS + 3 - 1); // 3 more for RGB, 1 less for strips. verifyImageFileDirectory(tagCount, PHOTOMETRIC_INTERPRETATION_BLACK_IS_ZERO, new short[] {Byte.SIZE}, false); verifySampleValues(1); store.close();
