FOP-1722: Values in PDF Number Trees must be indirect references by Dave Roxburgh

5 files changed, 87 insertions, 17 deletions

diff --git a/fop-core/src/main/java/org/apache/fop/pdf/PDFNumsArray.java b/fop-core/src/main/java/org/apache/fop/pdf/PDFNumsArray.java
index e9e1855b0..aafff37fc 100644
--- a/fop-core/src/main/java/org/apache/fop/pdf/PDFNumsArray.java
+++ b/fop-core/src/main/java/org/apache/fop/pdf/PDFNumsArray.java
@@ -51,12 +51,82 @@ public class PDFNumsArray extends PDFObject {
     }
 
     /**
+     * Determines whether a value object should be converted to an indirect reference for inclusion in a Number Tree
+     * array according to the PDF spec.
+     * PDF1.0 - 1.2 - Spec is silent on this subject (as Number Trees don't exist).
+     * PDF1.3 & 1.4 - Values must be indirect object refs.
+     * PDF1.5       - Recommended: stream, dictionary, array, and string values be indirect object refs.
+     * PDF1.6 - 2.0 - Stream values must be indirect object refs.
+     *                Recommended: dictionary, array, and string values be indirect object refs.
+     * Method signals for values that must be, and those recommended to be indirect object refs.
+     * @param obj The object to be considered.
+     * @return True iff the object should be converted.
+     */
+    private boolean shouldConvertToRef(PDFObject obj) {
+        boolean retval = false;
+        if (getDocument() != null && getDocument().getPDFVersion() != null) {
+            switch (getDocument().getPDFVersion()) {
+                case V1_0: // fall-through
+                case V1_1: // fall-through
+                case V1_2:
+                    log.error("Number Tree used in PDF version " + getDocument().getPDFVersion());
+                    break;
+                case V1_3: // fall-through
+                case V1_4:
+                    retval = true;
+                    break;
+                case V1_5: // fall-through
+                case V1_6: // fall-through
+                case V1_7: // fall-through
+                case V2_0:
+                    if (obj instanceof PDFStream
+                            || obj instanceof PDFDictionary
+                            || obj instanceof PDFArray
+                            || obj instanceof PDFText) {
+                        retval = true;
+                    }
+                    break;
+                default:
+                    log.error("Unrecognised PDF version " + getDocument().getPDFVersion());
+                    break;
+            }
+        }
+        return retval;
+    }
+
+    /**
+     * This method provides conformance with the different PDF specs which require or recommend different types be used
+     * for Number Tree array values. Method indirects objects where indicated.
+     * @param obj The object to be considered for indirection.
+     * @return Either the object or a reference to the object.
+     */
+    private Object indirectIfReq(Object obj) {
+        PDFDocument doc = getDocument();
+        Object retval = obj;
+        if (obj instanceof PDFObject) {
+            PDFObject pdfObj = (PDFObject) obj;
+            PDFObject parent = pdfObj.getParent();
+            if (shouldConvertToRef(pdfObj)) {
+                if (!pdfObj.hasObjectNumber()) { // Needs registering with the doc.
+                    pdfObj.setParent(null); // Can't register if it has a parent.
+                    pdfObj = doc.registerObject(pdfObj);
+                    if (parent != null) {
+                        pdfObj.setParent(parent); // Reinstate original parent.
+                    }
+                }
+                retval = pdfObj.makeReference();
+            }
+        }
+        return retval;
+    }
+
+    /**
      * Sets an entry.
      * @param key the key of the value to set
      * @param obj the new value
      */
     public void put(Integer key, Object obj) {
-        this.map.put(key, obj);
+        this.map.put(key, indirectIfReq(obj));
     }
 
     /**
diff --git a/fop-core/src/main/java/org/apache/fop/render/pdf/PDFLogicalStructureHandler.java b/fop-core/src/main/java/org/apache/fop/render/pdf/PDFLogicalStructureHandler.java
index 318892ba3..12bb7a85c 100644
--- a/fop-core/src/main/java/org/apache/fop/render/pdf/PDFLogicalStructureHandler.java
+++ b/fop-core/src/main/java/org/apache/fop/render/pdf/PDFLogicalStructureHandler.java
@@ -119,14 +119,6 @@ public class PDFLogicalStructureHandler {
      * Receive notification of the end of the current page.
      */
     void endPage() {
-        // TODO
-        // Values in a number tree must be indirect references to the PDF
-        // objects associated to the keys. To enforce that the array is
-        // registered to the PDF document. Unfortunately that can't be done
-        // earlier since a call to PDFContentGenerator.flushPDFDoc can be made
-        // before the array is complete, which would result in only part of it
-        // being output to the PDF.
-        // This should really be handled by PDFNumsArray
         pdfDoc.registerObject(pageParentTreeArray);
         parentTree.addToNums(currentPage.getStructParents(), pageParentTreeArray);
     }
diff --git a/fop-core/src/test/java/org/apache/fop/pdf/PDFLibraryTestSuite.java b/fop-core/src/test/java/org/apache/fop/pdf/PDFLibraryTestSuite.java
index 2f7fccc3a..374540355 100644
--- a/fop-core/src/test/java/org/apache/fop/pdf/PDFLibraryTestSuite.java
+++ b/fop-core/src/test/java/org/apache/fop/pdf/PDFLibraryTestSuite.java
@@ -40,6 +40,7 @@ import org.apache.fop.render.pdf.extensions.PDFEmbeddedFileAttachmentTestCase;
         PDFDocumentTestCase.class,
         PDFNullTestCase.class,
         PDFNumsArrayTestCase.class,
+        PDFNumsArrayPDFV17TestCase.class,
         PDFRectangleTestCase.class,
         PDFReferenceTestCase.class,
         PDFResourcesTestCase.class,
diff --git a/fop-core/src/test/java/org/apache/fop/pdf/PDFNumsArrayTestCase.java b/fop-core/src/test/java/org/apache/fop/pdf/PDFNumsArrayTestCase.java
index d36775cda..035912a95 100644
--- a/fop-core/src/test/java/org/apache/fop/pdf/PDFNumsArrayTestCase.java
+++ b/fop-core/src/test/java/org/apache/fop/pdf/PDFNumsArrayTestCase.java
@@ -25,20 +25,25 @@ import org.junit.Before;
 import org.junit.Test;
 
 /**
- * Test case for {@link PDFNumsArray}.
+ * Test case for {@link PDFNumsArray}. Uses default PDF doc (version 1.4) - requires text values and number values to be
+ * included as refs.
  */
 public class PDFNumsArrayTestCase extends PDFObjectTestCase {
+    private static final String TEST_NAME = "Test name";
+    private static final int TEST_NUMBER = 10;
     private PDFNumsArray numsArray;
-    private String expectedString = "[0 /Test#20name 1 10]";
+    private String expectedString = "[0 1 0 R 1 2 0 R]";
+    private String expectedStringName = "/Test#20name";
+    private String expectedStringNumber = Integer.valueOf(TEST_NUMBER).toString();
 
     @Before
     public void setUp() {
         numsArray = new PDFNumsArray(parent);
-        numsArray.put(0, new PDFName("Test name"));
+        numsArray.setDocument(doc);
+        numsArray.put(0, new PDFName(TEST_NAME));
         PDFNumber num = new PDFNumber();
-        num.setNumber(10);
+        num.setNumber(TEST_NUMBER);
         numsArray.put(1, num);
-        numsArray.setDocument(doc);
 
         pdfObjectUnderTest = numsArray;
     }
@@ -50,5 +55,7 @@ public class PDFNumsArrayTestCase extends PDFObjectTestCase {
     @Test
     public void testOutput() throws IOException {
         testOutputStreams(expectedString, numsArray);
+        testOutputStreams(expectedStringName, doc.objects.get(1));
+        testOutputStreams(expectedStringNumber, doc.objects.get(2));
     }
 }
diff --git a/fop-core/src/test/java/org/apache/fop/pdf/PDFParentTreeTestCase.java b/fop-core/src/test/java/org/apache/fop/pdf/PDFParentTreeTestCase.java
index 6fa5d6a42..acb888e4e 100644
--- a/fop-core/src/test/java/org/apache/fop/pdf/PDFParentTreeTestCase.java
+++ b/fop-core/src/test/java/org/apache/fop/pdf/PDFParentTreeTestCase.java
@@ -24,7 +24,7 @@ import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 
 /**
  * Tests that the nums array in the ParentTree dictionary is correctly being split into
@@ -76,7 +76,7 @@ public class PDFParentTreeTestCase {
      */
     @Test
     public void testOutOfOrderSplit() throws Exception {
-        PDFStructElem structElem = mock(PDFStructElem.class);
+        PDFStructElem structElem = spy(PDFStructElem.class);
         for (int num = 50; num < 53; num++) {
             parentTree.addToNums(num, structElem);
         }
@@ -98,7 +98,7 @@ public class PDFParentTreeTestCase {
      * @throws Exception
      */
     private int getArrayNumber(int elementNumber) throws Exception {
-        PDFStructElem structElem = mock(PDFStructElem.class);
+        PDFStructElem structElem = spy(PDFStructElem.class);
         for (int structParent = 0; structParent < elementNumber; structParent++) {
             parentTree.addToNums(structParent, structElem);
         }