Change PdfDocument id generation process

 - Add separate fields for original and modified ids to PdfDocument
to aid conversion to .xdf format.
 - Generate new modified id every time PdfReader is present without PdfWriter.
 - Id values in WriterProperties take precedence over id values specified
in PdfReader if both PdfReader and PdfWriter is present.
 - Remove id generation inside PdfReader getters.
 - Move id generation process to PdfDocument.
 - Standard encryption now uses originalDocumentId as a parameter
 instead of generating new id every time.
 - Change PdfDocumentId and PdfEncryption tests to reflect new behavior.

Author: BlackEgoist

io/src/main/java/com/itextpdf/io/LogMessageConstant.java

@@ -78,6 +78,7 @@ public final class LogMessageConstant {
     public static final String DIRECTONLY_OBJECT_CANNOT_BE_INDIRECT = "DirectOnly object cannot be indirect";
     public static final String DOCFONT_HAS_ILLEGAL_DIFFERENCES = "Document Font has illegal differences array. Entry {0} references a glyph ID over 255 and will be ignored.";
     public static final String DOCUMENT_ALREADY_HAS_FIELD = "The document already has field {0}. Annotations of the fields with this name will be added to the existing one as children. If you want to have separate fields, please, rename them manually before copying.";
+    public static final String DOCUMENT_IDS_ARE_CORRUPTED = "The document original and/or modified id is corrupted";
     public static final String DOCUMENT_SERIALIZATION_EXCEPTION_RAISED = "Unhandled exception while serialization";
     public static final String ELEMENT_DOES_NOT_FIT_AREA = "Element does not fit current area. {0}";
     public static final String ELEMENT_WAS_FORCE_PLACED_KEEP_WITH_NEXT_WILL_BE_IGNORED = "Element was placed in a forced way. Keep with next property will be ignored";

kernel/findbugs-filter.xml

@@ -465,4 +465,9 @@
         </Or>
         <Bug pattern="WEAK_MESSAGE_DIGEST_MD5"/>
     </Match>
+    <Match>
+        <Class name="com.itextpdf.kernel.pdf.PdfDocument"/>
+        <Method name="open"/>
+        <Bug pattern="PRMC_POSSIBLY_REDUNDANT_METHOD_CALLS"/>
+    </Match>
 </FindBugsFilter>

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

@@ -163,9 +163,14 @@ public class PdfDocument implements IEventDispatcher, Closeable, Serializable {
     protected PdfVersion pdfVersion = PdfVersion.PDF_1_7;
 
     /**
-     * The original second id when the document is read initially.
+     * The original (first) id when the document is read initially.
      */
-    private PdfString originalModifiedDocumentId;
+    private PdfString originalDocumentId;
+
+    /**
+     * The original modified (second) id when the document is read initially.
+     */
+    private PdfString modifiedDocumentId;
 
     /**
      * List of indirect objects used in the document.
@@ -633,6 +638,29 @@ public PdfDocumentInfo getDocumentInfo() {
         return info;
     }
 
+    /**
+     * Gets original document id
+     *
+     * In order to set originalDocumentId  {@link WriterProperties#setInitialDocumentId} should be used
+     *
+     * @return original dccument id
+     */
+    public PdfString getOriginalDocumentId() {
+        return originalDocumentId;
+    }
+
+
+    /**
+     * Gets modified document id
+     *
+     * In order to set modifiedDocumentId {@link WriterProperties#setModifiedDocumentId} should be used
+     *
+     * @return modified document id
+     */
+    public PdfString getModifiedDocumentId() {
+        return modifiedDocumentId;
+    }
+
     /**
      * Gets default page size.
      *
@@ -803,8 +831,6 @@ public void close() {
                 }
                 checkIsoConformance();
 
-                PdfObject fileId = getFileId();
-
                 PdfObject crypto = null;
                 Set<PdfIndirectReference> forbiddenToFlush = new HashSet<>();
                 if (properties.appendMode) {
@@ -918,6 +944,11 @@ public void close() {
                 trailer.put(PdfName.Root, catalog.getPdfObject());
                 trailer.put(PdfName.Info, info.getPdfObject());
 
+
+                //By this time original and modified document ids should always be not null due to initializing in
+                // either writer properties, or in the writer init section on document open or from pdfreader. So we shouldn't worry about it being null next
+                PdfObject fileId = PdfEncryption.createInfoId(ByteUtils.getIsoBytes(originalDocumentId.getValue()),
+                        ByteUtils.getIsoBytes(modifiedDocumentId.getValue()));
                 xref.writeXrefTableAndTrailer(this, fileId, crypto);
                 writer.flush();
                 for (ICounter counter : getCounters()) {
@@ -951,44 +982,6 @@ public void close() {
         closed = true;
     }
 
-    private PdfObject getFileId() {
-        boolean documentIsModified = false;
-        byte[] originalFileId = null;
-
-        if (writer.properties.initialDocumentId != null) {
-            originalFileId = ByteUtils.getIsoBytes(writer.properties.initialDocumentId.getValue());
-        }
-        if (originalFileId == null && !properties.appendMode && writer.crypto != null) { // TODO why only not in append mode when encrypted?
-            originalFileId = writer.crypto.getDocumentId();
-        }
-        if (originalFileId == null && getReader() != null) {
-            originalFileId = getReader().getOriginalFileId();
-            documentIsModified = true;
-        }
-        if (originalFileId == null) {
-            originalFileId = PdfEncryption.generateNewDocumentId();
-        }
-
-        byte[] secondId = null;
-        if (writer.properties.modifiedDocumentId != null) {
-            secondId = ByteUtils.getIsoBytes(writer.properties.modifiedDocumentId.getValue());
-        }
-        if (secondId == null && originalModifiedDocumentId != null) {
-            PdfString newModifiedId = reader.trailer.getAsArray(PdfName.ID).getAsString(1);
-
-            if (!originalModifiedDocumentId.equals(newModifiedId)) {
-                secondId = ByteUtils.getIsoBytes(newModifiedId.getValue());
-            } else {
-                secondId = PdfEncryption.generateNewDocumentId();
-            }
-        }
-        if (secondId == null) {
-            secondId = (documentIsModified) ? PdfEncryption.generateNewDocumentId() : originalFileId;
-        }
-
-        return PdfEncryption.createInfoId(originalFileId, secondId);
-    }
-
     /**
      * Gets close status of the document.
      *
@@ -1833,7 +1826,15 @@ protected void open(PdfVersion newPdfVersion) {
                 PdfArray id = reader.trailer.getAsArray(PdfName.ID);
 
                 if (id != null) {
-                    originalModifiedDocumentId = id.getAsString(1);
+                   if (id.size() == 2) {
+                       originalDocumentId = id.getAsString(0);
+                       modifiedDocumentId = id.getAsString(1);
+                   }
+
+                   if (originalDocumentId == null || modifiedDocumentId == null) {
+                       Logger logger = LoggerFactory.getLogger(PdfDocument.class);
+                       logger.error(LogMessageConstant.DOCUMENT_IDS_ARE_CORRUPTED);
+                   }
                 }
 
                 catalog = new PdfCatalog((PdfDictionary) trailer.get(PdfName.Root, true));
@@ -1892,6 +1893,32 @@ protected void open(PdfVersion newPdfVersion) {
                         trailer.put(PdfName.ID, reader.trailer.get(PdfName.ID));
                     }
                 }
+
+                if (writer.properties != null) {
+                    PdfString readerModifiedId = modifiedDocumentId;
+                    if (writer.properties.initialDocumentId != null &&
+                            !(reader != null && reader.decrypt != null && (properties.appendMode || properties.preserveEncryption))) {
+                        originalDocumentId = writer.properties.initialDocumentId;
+                    }
+                    if (writer.properties.modifiedDocumentId != null) {
+                        modifiedDocumentId = writer.properties.modifiedDocumentId;
+                    }
+                    if (originalDocumentId == null && modifiedDocumentId != null) {
+                        originalDocumentId = modifiedDocumentId;
+                    }
+                    if (modifiedDocumentId == null) {
+                        if (originalDocumentId == null) {
+                            originalDocumentId = new PdfString(PdfEncryption.generateNewDocumentId());
+                        }
+                        modifiedDocumentId = originalDocumentId;
+                    }
+                    if(writer.properties.modifiedDocumentId == null && modifiedDocumentId.equals(readerModifiedId)) {
+                        modifiedDocumentId = new PdfString(PdfEncryption.generateNewDocumentId());
+                    }
+                }
+
+                assert originalDocumentId != null;
+                assert modifiedDocumentId != null;
             }
             if (properties.appendMode) {       // Due to constructor reader and writer not null.
                 assert reader != null;

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

@@ -506,17 +506,34 @@ public byte[] computeUserPassword() {
     }
 
     /**
-     * Gets file ID, either {@link PdfName#ID} key of trailer or a newly generated id.
+     * Gets original file ID, the first element in {@link PdfName#ID} key of trailer.
+     * If the size of ID array does not equal 2, an empty array will be returned.
      *
-     * @return byte array represents file ID.
-     * @see PdfEncryption#generateNewDocumentId()
+     * @return byte array represents original file ID.
+     * @see PdfDocument#getOriginalDocumentId(). The ultimate document id should be taken from PdfDocument
      */
     public byte[] getOriginalFileId() {
         PdfArray id = trailer.getAsArray(PdfName.ID);
-        if (id != null) {
+        if (id != null && id.size() == 2) {
             return ByteUtils.getIsoBytes(id.getAsString(0).getValue());
         } else {
-            return PdfEncryption.generateNewDocumentId();
+            return new byte[0];
+        }
+    }
+
+    /**
+     * Gets modified file ID, the second element in {@link PdfName#ID} key of trailer.
+     * If the size of ID array does not equal 2, an empty array will be returned.
+     *
+     * @return byte array represents modified file ID.
+     * @see PdfDocument#getModifiedDocumentId()
+     */
+    public byte[] getModifiedFileId() {
+        PdfArray id = trailer.getAsArray(PdfName.ID);
+        if (id != null && id.size() == 2) {
+            return ByteUtils.getIsoBytes(id.getAsString(1).getValue());
+        } else {
+            return new byte[0];
         }
     }
 

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

@@ -45,6 +45,7 @@ This file is part of the iText (R) project.
 
 import com.itextpdf.io.LogMessageConstant;
 import com.itextpdf.io.source.ByteArrayOutputStream;
+import com.itextpdf.io.source.ByteUtils;
 import com.itextpdf.io.util.FileUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -276,7 +277,7 @@ protected void initCryptoIfSpecified(PdfVersion version) {
         EncryptionProperties encryptProps = properties.encryptionProperties;
         if (properties.isStandardEncryptionUsed()) {
             crypto = new PdfEncryption(encryptProps.userPassword, encryptProps.ownerPassword, encryptProps.standardEncryptPermissions,
-                    encryptProps.encryptionAlgorithm, PdfEncryption.generateNewDocumentId(), version);
+                    encryptProps.encryptionAlgorithm, ByteUtils.getIsoBytes(this.document.getOriginalDocumentId().toString()), version);
         } else if (properties.isPublicKeyEncryptionUsed()) {
             crypto = new PdfEncryption(encryptProps.publicCertificates,
                     encryptProps.publicKeyEncryptPermissions, encryptProps.encryptionAlgorithm, version);

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

@@ -223,7 +223,6 @@ public WriterProperties setModifiedDocumentId(PdfString modifiedDocumentId) {
         this.modifiedDocumentId = modifiedDocumentId;
         return this;
     }
-
     /**
      * This activates debug mode with pdfDebug tool.
      * It causes additional overhead of duplicating document bytes into memory, so use it careful.