You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pdfbox.apache.org by ti...@apache.org on 2026/07/24 09:22:55 UTC
svn commit: r1936541 - in pdfbox/branches/2.0/pdfbox/src: main/java/org/apache/pdfbox/pdmodel/graphics/image test/java/org/apache/pdfbox/pdmodel/graphics/image
Author: tilman
Date: Fri Jul 24 09:22:54 2026
New Revision: 1936541
Log:
PDFBOX-5660: fix readlong(), as suggested by Valery Bokov and Claude Code; Sonar fix; closes #488
Modified:
pdfbox/branches/2.0/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/graphics/image/CCITTFactory.java
pdfbox/branches/2.0/pdfbox/src/test/java/org/apache/pdfbox/pdmodel/graphics/image/CCITTFactoryTest.java
Modified: pdfbox/branches/2.0/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/graphics/image/CCITTFactory.java
==============================================================================
--- pdfbox/branches/2.0/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/graphics/image/CCITTFactory.java Fri Jul 24 09:22:50 2026 (r1936540)
+++ pdfbox/branches/2.0/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/graphics/image/CCITTFactory.java Fri Jul 24 09:22:54 2026 (r1936541)
@@ -349,7 +349,7 @@ public final class CCITTFactory
{
int tag = readshort(endianess, reader);
int type = readshort(endianess, reader);
- int count = readlong(endianess, reader);
+ int count = (int) readlong(endianess, reader);
int val;
// Note that when the type is shorter than 4 bytes, the rest can be garbage
// and must be ignored. E.g. short (2 bytes) from "01 00 38 32" (little endian)
@@ -368,7 +368,7 @@ public final class CCITTFactory
reader.read();
break;
default: // long and other types
- val = readlong(endianess, reader);
+ val = (int) readlong(endianess, reader);
break;
}
switch (tag)
@@ -522,13 +522,14 @@ public final class CCITTFactory
return (raf.read() << 8) | raf.read();
}
- private static int readlong(char endianess, RandomAccess raf) throws IOException
+ static long readlong(char endianess, RandomAccess raf) throws IOException
{
+ // TIFF LONG is an unsigned 32-bit value; mask so it widens correctly
if (endianess == 'I')
{
- return raf.read() | (raf.read() << 8) | (raf.read() << 16) | (raf.read() << 24);
+ return (raf.read() | (raf.read() << 8) | (raf.read() << 16) | (raf.read() << 24)) & 0xFFFFFFFFL;
}
- return (raf.read() << 24) | (raf.read() << 16) | (raf.read() << 8) | raf.read();
+ return ((raf.read() << 24) | (raf.read() << 16) | (raf.read() << 8) | raf.read()) & 0xFFFFFFFFL;
}
private static final byte[] fliptable = new byte[]
Modified: pdfbox/branches/2.0/pdfbox/src/test/java/org/apache/pdfbox/pdmodel/graphics/image/CCITTFactoryTest.java
==============================================================================
--- pdfbox/branches/2.0/pdfbox/src/test/java/org/apache/pdfbox/pdmodel/graphics/image/CCITTFactoryTest.java Fri Jul 24 09:22:50 2026 (r1936540)
+++ pdfbox/branches/2.0/pdfbox/src/test/java/org/apache/pdfbox/pdmodel/graphics/image/CCITTFactoryTest.java Fri Jul 24 09:22:54 2026 (r1936541)
@@ -29,8 +29,12 @@ import java.util.Arrays;
import javax.imageio.ImageIO;
import javax.imageio.ImageReader;
import javax.imageio.stream.ImageInputStream;
+
import junit.framework.TestCase;
+
import org.apache.pdfbox.io.IOUtils;
+import org.apache.pdfbox.io.RandomAccess;
+import org.apache.pdfbox.io.RandomAccessBuffer;
import org.apache.pdfbox.pdmodel.PDDocument;
import org.apache.pdfbox.pdmodel.PDPage;
import org.apache.pdfbox.pdmodel.common.PDRectangle;
@@ -40,7 +44,7 @@ import org.apache.pdfbox.pdmodel.graphic
import static org.apache.pdfbox.pdmodel.graphics.image.ValidateXImage.checkIdent;
import static org.apache.pdfbox.pdmodel.graphics.image.ValidateXImage.validate;
-import org.junit.Assert;
+import static org.junit.Assert.assertNotEquals;
/**
* Unit tests for CCITTFactory
@@ -177,7 +181,7 @@ public class CCITTFactoryTest extends Te
{
PDDocument document = new PDDocument();
BufferedImage bim = new BufferedImage(343, 287, BufferedImage.TYPE_BYTE_BINARY);
- Assert.assertNotEquals((bim.getWidth() / 8) * 8, bim.getWidth()); // not mult of 8
+ assertNotEquals((bim.getWidth() / 8) * 8, bim.getWidth()); // not mult of 8
int col = 0;
for (int x = 0; x < bim.getWidth(); ++x)
{
@@ -304,4 +308,38 @@ public class CCITTFactoryTest extends Te
assertEquals(1, document.getNumberOfPages());
document.close();
}
+
+ /**
+ * Tests that CCITTFactory's private readlong() reads a TIFF LONG as an unsigned 32-bit
+ * value. The previous implementation returned a (possibly negative) int, which was then
+ * sign-extended when widened to long, corrupting IFD offsets/counts whose high bit is set
+ * (e.g. 0x80000000 and above).
+ */
+ public void testReadLongIsUnsigned() throws IOException
+ {
+ // all bits set: 0xFFFFFFFF == 4294967295 as an unsigned TIFF LONG.
+ // The buggy code returned the int -1, which as a long is -1, not 4294967295.
+ byte[] allOnes = { (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF };
+ assertReadLongUnsigned('I', allOnes, 0xFFFFFFFFL);
+ assertReadLongUnsigned('M', allOnes, 0xFFFFFFFFL);
+
+ // only the top bit set, in each byte order: 0x80000000 == 2147483648 unsigned.
+ // The buggy code returned the int Integer.MIN_VALUE, which sign-extends to a
+ // large negative long instead of 2147483648.
+ byte[] littleEndianTopBit = { 0x00, 0x00, 0x00, (byte) 0x80 };
+ assertReadLongUnsigned('I', littleEndianTopBit, 0x80000000L);
+
+ byte[] bigEndianTopBit = { (byte) 0x80, 0x00, 0x00, 0x00 };
+ assertReadLongUnsigned('M', bigEndianTopBit, 0x80000000L);
+ }
+
+ private static void assertReadLongUnsigned(char endianess, byte[] bytes,
+ long expected) throws IOException
+ {
+ RandomAccess raf = new RandomAccessBuffer(bytes);
+ long value = CCITTFactory.readlong(endianess, raf);
+ assertEquals(expected, value);
+ assertTrue("TIFF LONG must be read as unsigned, not sign-extended", value >= 0);
+ raf.close();
+ }
}