Introduce ForbidRelease and ReadOnly flags. Make PdfObject#release() less prone to errors

DEVSIX-480

Author: Snipx

forms/src/main/java/com/itextpdf/forms/PdfAcroForm.java

@@ -108,10 +108,8 @@ public PdfAcroForm(PdfDictionary pdfObject) {
      * @param fields a {@link PdfArray} of {@link PdfDictionary} objects
      */
     public PdfAcroForm(PdfArray fields) {
-        super(new PdfDictionary());
-        put(PdfName.Fields, fields);
-        getFormFields();
-        xfaForm = new XfaForm();
+        this(createAcroFormDictionaryByFields(fields));
+        setForbidRelease();
     }
 
     /**
@@ -986,6 +984,25 @@ public String getXfaFieldValue(String name) {
         return null;
     }
 
+    /**
+     * Releases underlying pdf object and other pdf entities used by wrapper.
+     * This method should be called instead of direct call to {@link PdfObject#release()} if the wrapper is used.
+     */
+    public void release() {
+        unsetForbidRelease();
+        getPdfObject().release();
+        for (PdfFormField field : fields.values()) {
+            field.release();
+        }
+        fields = null;
+    }
+
+    private static PdfDictionary createAcroFormDictionaryByFields(PdfArray fields) {
+        PdfDictionary dictionary = new PdfDictionary();
+        dictionary.put(PdfName.Fields, fields);
+        return dictionary;
+    }
+
     private PdfPage getFieldPage(PdfDictionary annotDic) {
         PdfDictionary pageDic = annotDic.getAsDictionary(PdfName.P);
         if (pageDic != null) {

forms/src/main/java/com/itextpdf/forms/fields/PdfFormField.java

@@ -129,6 +129,7 @@ public class PdfFormField extends PdfObjectWrapper<PdfDictionary> {
     public PdfFormField(PdfDictionary pdfObject) {
         super(pdfObject);
         ensureObjectIsAddedToDocument(pdfObject);
+        setForbidRelease();
     }
 
     /**
@@ -1787,6 +1788,64 @@ public <T extends PdfFormField> T setAppearance(PdfName appearanceType, String a
         return (T) this;
     }
 
+    /**
+     * Releases underlying pdf object and other pdf entities used by wrapper.
+     * This method should be called instead of direct call to {@link PdfObject#release()} if the wrapper is used.
+     */
+    public void release() {
+        unsetForbidRelease();
+        getPdfObject().release();
+    }
+
+    protected static Object[] splitDAelements(String da) {
+        PdfTokenizer tk = new PdfTokenizer(new RandomAccessFileOrArray(new RandomAccessSourceFactory().createSource(PdfEncodings.convertToBytes(da, null))));
+        List<String> stack = new ArrayList<>();
+        Object ret[] = new Object[3];
+        try {
+            while (tk.nextToken()) {
+                if (tk.getTokenType() == PdfTokenizer.TokenType.Comment)
+                    continue;
+                if (tk.getTokenType() == PdfTokenizer.TokenType.Other) {
+                    String operator = tk.getStringValue();
+                    if (operator.equals("Tf")) {
+                        if (stack.size() >= 2) {
+                            ret[DA_FONT] = stack.get(stack.size() - 2);
+                            ret[DA_SIZE] = new Integer(stack.get(stack.size() - 1));
+                        }
+                    } else if (operator.equals("g")) {
+                        if (stack.size() >= 1) {
+                            float gray = new Float(stack.get(stack.size() - 1));
+                            if (gray != 0) {
+                                ret[DA_COLOR] = new DeviceGray(gray);
+                            }
+                        }
+                    } else if (operator.equals("rg")) {
+                        if (stack.size() >= 3) {
+                            float red = new Float(stack.get(stack.size() - 3));
+                            float green = new Float(stack.get(stack.size() - 2));
+                            float blue = new Float(stack.get(stack.size() - 1));
+                            ret[DA_COLOR] = new DeviceRgb(red, green, blue);
+                        }
+                    } else if (operator.equals("k")) {
+                        if (stack.size() >= 4) {
+                            float cyan = new Float(stack.get(stack.size() - 4));
+                            float magenta = new Float(stack.get(stack.size() - 3));
+                            float yellow = new Float(stack.get(stack.size() - 2));
+                            float black = new Float(stack.get(stack.size() - 1));
+                            ret[DA_COLOR] = new DeviceCmyk(cyan, magenta, yellow, black);
+                        }
+                    }
+                    stack.clear();
+                } else {
+                    stack.add(tk.getStringValue());
+                }
+            }
+        } catch (IOException e) {
+
+        }
+        return ret;
+    }
+
     @Override
     protected boolean isWrappedObjectMustBeIndirect() {
         return true;
@@ -1887,55 +1946,6 @@ protected Object[] getFontAndSize(PdfDictionary asNormal) throws IOException {
         return fontAndSize;
     }
 
-    public static Object[] splitDAelements(String da) {
-        PdfTokenizer tk = new PdfTokenizer(new RandomAccessFileOrArray(new RandomAccessSourceFactory().createSource(PdfEncodings.convertToBytes(da, null))));
-        List<String> stack = new ArrayList<>();
-        Object ret[] = new Object[3];
-        try {
-            while (tk.nextToken()) {
-                if (tk.getTokenType() == PdfTokenizer.TokenType.Comment)
-                    continue;
-                if (tk.getTokenType() == PdfTokenizer.TokenType.Other) {
-                    String operator = tk.getStringValue();
-                    if (operator.equals("Tf")) {
-                        if (stack.size() >= 2) {
-                            ret[DA_FONT] = stack.get(stack.size() - 2);
-                            ret[DA_SIZE] = new Integer(stack.get(stack.size() - 1));
-                        }
-                    } else if (operator.equals("g")) {
-                        if (stack.size() >= 1) {
-                            float gray = new Float(stack.get(stack.size() - 1));
-                            if (gray != 0) {
-                                ret[DA_COLOR] = new DeviceGray(gray);
-                            }
-                        }
-                    } else if (operator.equals("rg")) {
-                        if (stack.size() >= 3) {
-                            float red = new Float(stack.get(stack.size() - 3));
-                            float green = new Float(stack.get(stack.size() - 2));
-                            float blue = new Float(stack.get(stack.size() - 1));
-                            ret[DA_COLOR] = new DeviceRgb(red, green, blue);
-                        }
-                    } else if (operator.equals("k")) {
-                        if (stack.size() >= 4) {
-                            float cyan = new Float(stack.get(stack.size() - 4));
-                            float magenta = new Float(stack.get(stack.size() - 3));
-                            float yellow = new Float(stack.get(stack.size() - 2));
-                            float black = new Float(stack.get(stack.size() - 1));
-                            ret[DA_COLOR] = new DeviceCmyk(cyan, magenta, yellow, black);
-                        }
-                    }
-                    stack.clear();
-                } else {
-                    stack.add(tk.getStringValue());
-                }
-            }
-        } catch (IOException e) {
-
-        }
-        return ret;
-    }
-
     /**
      * Draws the visual appearance of text in a form field.
      *

kernel/src/main/java/com/itextpdf/kernel/PdfException.java

@@ -45,6 +45,7 @@ public class PdfException extends RuntimeException {
     public static final String CannotSetDataToPdfstreamWhichWasCreatedByInputstream = "cannot.set.data.to.pdfstream.which.was.created.by.inputstream";
     public static final String CannotSplitDocumentThatIsBeingWritten = "cannot.split.document.that.is.being.written";
     public static final String CannotWritePdfStream = "cannot.write.pdf.stream";
+    public static final String CannotWriteObjectAfterItWasReleased = "Cannot write object after it was released. In normal situation the object must be read once again before being written";
     public static final String CantDecodePkcs7SigneddataObject = "can.t.decode.pkcs7signeddata.object";
     public static final String CantFindSigningCertificateWithSerial1 = "can.t.find.signing.certificate.with.serial {0}";
     public static final String CfNotFoundEncryption = "cf.not.found.encryption";

kernel/src/main/java/com/itextpdf/kernel/pdf/PdfCatalog.java

@@ -2,15 +2,16 @@
 
 import com.itextpdf.kernel.PdfException;
 import com.itextpdf.kernel.pdf.action.PdfAction;
-import com.itextpdf.kernel.pdf.annot.PdfAnnotation;
-import com.itextpdf.kernel.pdf.annot.PdfWidgetAnnotation;
 import com.itextpdf.kernel.pdf.collection.PdfCollection;
 import com.itextpdf.kernel.pdf.layer.PdfOCProperties;
 import com.itextpdf.kernel.pdf.navigation.PdfDestination;
 import com.itextpdf.kernel.pdf.navigation.PdfExplicitDestination;
 import com.itextpdf.kernel.pdf.navigation.PdfStringDestination;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 
 public class PdfCatalog extends PdfObjectWrapper<PdfDictionary> {
 

kernel/src/main/java/com/itextpdf/kernel/pdf/PdfDocumentInfo.java

@@ -11,6 +11,7 @@ public PdfDocumentInfo(PdfDictionary pdfObject, PdfDocument pdfDocument) {
         if (pdfDocument.getWriter() != null) {
             this.getPdfObject().makeIndirect(pdfDocument);
         }
+        setForbidRelease();
     }
 
     public PdfDocumentInfo(PdfDictionary pdfObject) {

kernel/src/main/java/com/itextpdf/kernel/pdf/PdfObject.java

@@ -1,8 +1,12 @@
 package com.itextpdf.kernel.pdf;
 
 import com.itextpdf.kernel.PdfException;
+
 import java.io.IOException;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 abstract public class PdfObject {
 
     static public final byte Array = 1;
@@ -22,30 +26,38 @@ abstract public class PdfObject {
     protected PdfIndirectReference indirectReference = null;
 
     // Indicates if the object has been flushed.
-    protected static final byte Flushed = 1;
+    protected static final short Flushed = 1;
     // Indicates that the indirect reference of the object could be reused or have to be marked as free.
-    protected static final byte Free = 2;
+    protected static final short Free = 2;
     // Indicates that definition of the indirect reference of the object still not found (e.g. keys in XRefStm).
-    protected static final byte Reading = 4;
+    protected static final short Reading = 4;
     // Indicates that object changed (using in stamp mode).
-    protected static final byte Modified = 8;
+    protected static final short Modified = 8;
     // Indicates that the indirect reference of the object represents ObjectStream from original document.
     // When PdfReader read ObjectStream reference marked as OriginalObjectStream
     // to avoid further reusing.
-    protected static final byte OriginalObjectStream = 16;
+    protected static final short OriginalObjectStream = 16;
     // For internal usage only. Marks objects that shall be written to the output document.
     // Option is needed to build the correct PDF objects tree when closing the document.
     // As a result it avoids writing unused (removed) objects.
-    protected static final byte MustBeFlushed = 32;
+    protected static final short MustBeFlushed = 32;
     // Indicates that the object shall be indirect when it is written to the document.
     // It is used to postpone the creation of indirect reference for the objects that shall be indirect,
     // so it is possible to create such objects without PdfDocument instance.
-    protected static final byte MustBeIndirect = 64;
+    protected static final short MustBeIndirect = 64;
+    // Indicates that the object is highly sensitive and we do not want to release it even if release() is called.
+    // This flag can be set in stamping mode in object wrapper constructors and is automatically set when setModified
+    // flag is set (we do not want to release changed objects).
+    // The flag is set automatically for some wrappers that need document even in reader mode (FormFields etc).
+    protected static final short ForbidRelease = 128;
+    // Indicates that we do not want this object to be ever written into the resultant document
+    // (because of multiple objects read from the same reference inconsistency).
+    protected static final short ReadOnly = 256;
 
     /**
      * Indicate same special states of PdfIndirectObject or PdfObject like @see Free, @see Reading, @see Modified.
      */
-    private byte state;
+    private short state;
 
     public PdfObject() {
 
@@ -154,15 +166,15 @@ public <T extends PdfObject> T makeIndirect(PdfDocument document) {
      * @param state special flag to check
      * @return true if the state was set.
      */
-    protected boolean checkState(byte state) {
+    protected boolean checkState(short state) {
         return (this.state & state) == state;
     }
 
     /**
      * Sets special states of current object.
      * @param state special flag of current object
      */
-    protected <T extends PdfObject> T setState(byte state) {
+    protected <T extends PdfObject> T setState(short state) {
         this.state |= state;
         return (T) this;
     }
@@ -171,8 +183,8 @@ protected <T extends PdfObject> T setState(byte state) {
      * Clear state of the flag of current object.
      * @param state special flag state to clear
      */
-    protected <T extends PdfObject> T clearState(byte state) {
-        this.state &= 0xFF^state;
+    protected <T extends PdfObject> T clearState(short state) {
+        this.state &= ~state;
         return (T) this;
     }
 
@@ -289,17 +301,26 @@ protected <T extends PdfObject> T setIndirectReference(PdfIndirectReference indi
 
     //TODO comment! Add note about flush, modified flag and xref.
     public void setModified() {
-        if (indirectReference != null)
+        if (indirectReference != null) {
             indirectReference.setState(Modified);
+            setState(ForbidRelease);
+        }
     }
 
     public void release() {
-        if (indirectReference != null && indirectReference.getReader() != null
-                && !indirectReference.checkState(Flushed)) {
-            indirectReference.refersTo = null;
-            indirectReference = null;
+        // In case ForbidRelease flag is set, release will not be performed.
+        if (checkState(ForbidRelease)) {
+            Logger logger = LoggerFactory.getLogger(PdfObject.class);
+            logger.warn("ForbidRelease flag is set and release is called. Releasing will not be performed.");
+        } else {
+            if (indirectReference != null && indirectReference.getReader() != null
+                    && !indirectReference.checkState(Flushed)) {
+                indirectReference.refersTo = null;
+                indirectReference = null;
+                setState(ReadOnly);
+            }
+            //TODO log reasonless call of method
         }
-        //TODO log reasonless call of method
     }
 
     /**

kernel/src/main/java/com/itextpdf/kernel/pdf/PdfObjectWrapper.java

@@ -144,6 +144,18 @@ protected void setPdfObject(T pdfObject){
         this.pdfObject = pdfObject;
     }
 
+    protected void setForbidRelease() {
+        if (pdfObject != null) {
+            pdfObject.setState(PdfObject.ForbidRelease);
+        }
+    }
+
+    protected void unsetForbidRelease() {
+        if (pdfObject != null) {
+            pdfObject.clearState(PdfObject.ForbidRelease);
+        }
+    }
+
     protected static void markObjectAsIndirect(PdfObject pdfObject) {
         if (pdfObject.getIndirectReference() == null) {
             pdfObject.setState(PdfObject.MustBeIndirect);

kernel/src/main/java/com/itextpdf/kernel/pdf/PdfOutputStream.java

@@ -133,6 +133,9 @@ public PdfOutputStream write(PdfObject pdfObject) {
             pdfObject.makeIndirect(document);
             pdfObject = pdfObject.getIndirectReference();
         }
+        if (pdfObject.checkState(PdfObject.ReadOnly)) {
+            throw new PdfException(PdfException.CannotWriteObjectAfterItWasReleased);
+        }
         switch (pdfObject.getType()) {
             case PdfObject.Array:
                 write((PdfArray) pdfObject);

kernel/src/main/java/com/itextpdf/kernel/pdf/PdfPage.java

@@ -46,12 +46,12 @@ public class PdfPage extends PdfObjectWrapper<PdfDictionary> {
 
     protected PdfPage(PdfDictionary pdfObject) {
         super(pdfObject);
+        setForbidRelease();
         ensureObjectIsAddedToDocument(pdfObject);
     }
 
     protected PdfPage(PdfDocument pdfDocument, PageSize pageSize) {
-        super(new PdfDictionary());
-        makeIndirect(pdfDocument);
+        this(new PdfDictionary().makeIndirect(pdfDocument));
         PdfStream contentStream = new PdfStream().makeIndirect(pdfDocument);
         getPdfObject().put(PdfName.Contents, contentStream);
         getPdfObject().put(PdfName.Type, PdfName.Page);

kernel/src/main/java/com/itextpdf/kernel/pdf/PdfPages.java

@@ -11,6 +11,7 @@ public PdfPages(int from, PdfDocument pdfDocument, PdfPages parent) {
         if (pdfDocument.getWriter() != null) {
             getPdfObject().makeIndirect(pdfDocument);
         }
+        setForbidRelease();
         this.from = from;
         this.count = new PdfNumber(0);
         this.kids = new PdfArray();