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();
+ }
}