Fix behavior for finding /Q, /DA, and /DR for form fields

If not found in the field hierarchy, /Q and /DA are supposed to be looked up in the document-level form dictionary. /DR is supposed to only come from the document dictionary.

Fix behavior for finding /Q, /DA, and /DR for form fields
If not found in the field hierarchy, /Q and /DA are supposed to be looked up in the document-level form dictionary. /DR is supposed to only come from the document dictionary.
Jay Berkenbilt
1 parent 5207c3da
Showing 11 changed files with 2837 additions and 24 deletions
ChangeLog
include/qpdf/QPDFFormFieldObjectHelper.hh
libqpdf/QPDFFormFieldObjectHelper.cc
libqpdf/QPDFPageDocumentHelper.cc
manual/qpdf-manual.xml
qpdf/qpdf.testcov
qpdf/qtest/qpdf.test
qpdf/qtest/qpdf/form-document-defaults.pdf
qpdf/qtest/qpdf/form-filled-by-acrobat-out.pdf
qpdf/qtest/qpdf/form-form-document-defaults.out
qpdf/qtest/qpdf/sample-form-out.pdf
+2021-02-26  Jay Berkenbilt  <ejb@ql.org>
+
+	* Bug fix: QPDFFormFieldObjectHelper was mis-handling /DA, /Q, and
+	/DR in ways that usually didn't matter but were still wrong. /DA
+	and /Q were being found in the field hierarchy, but if not found,
+	the default values in the /AcroForm dictionary were not being
+	used. /DR was being treated as an inherited field in the field
+	dictionary, which is wrong. It is actually supposed to come from
+	the /AcroForm dictionary. We were getting away with this since
+	many popular form writers seem to copy it to the field as well,
+	even though the spec makes no mention of doing this. To support
+	this, QPDFFormFieldObjectHelper::getDefaultResources was added.
+
 2021-02-25  Jay Berkenbilt  <ejb@ql.org>
  
 	* Update StreamDataProvider examples to use copyStream() when they
@@ -121,12 +121,22 @@ class QPDFFormFieldObjectHelper: public QPDFObjectHelper
     // the field tree into account. Returns the empty string if the
     // default appearance string is not available (because it's
     // erroneously absent or because this is not a variable text
-    // field).
+    // field). If not found in the field hierarchy, look in /AcroForm.
     QPDF_DLL
     std::string getDefaultAppearance();
  
+    // Return the default resource dictionary for the field. This
+    // comes not from the field but from the document-level /AcroForm
+    // dictionary. While several PDF generates put a /DR key in the
+    // form field's dictionary, experimentation suggests that many
+    // popular readers, including Adobe Acrobat and Acrobat Reader,
+    // ignore any /DR item on the field.
+    QPDF_DLL
+    QPDFObjectHandle getDefaultResources();
+
     // Return the quadding value, taking inheritance from the field
-    // tree into account. Returns 0 if quadding is not specified.
+    // tree into account. Returns 0 if quadding is not specified. Look
+    // in /AcroForm if not found in the field hierarchy.
     QPDF_DLL
     int getQuadding();
  
@@ -200,6 +210,7 @@ class QPDFFormFieldObjectHelper: public QPDFObjectHelper
     void generateAppearance(QPDFAnnotationObjectHelper&);
  
   private:
+    QPDFObjectHandle getFieldFromAcroForm(std::string const& name);
     void setRadioButtonValue(QPDFObjectHandle name);
     void setCheckBoxValue(bool value);
     void generateTextAppearance(QPDFAnnotationObjectHelper&);
@@ -63,6 +63,24 @@ QPDFFormFieldObjectHelper::getTopLevelField(bool* is_different)
 }
  
 QPDFObjectHandle
+QPDFFormFieldObjectHelper::getFieldFromAcroForm(std::string const& name)
+{
+    QPDFObjectHandle result = QPDFObjectHandle::newNull();
+    // Fields are supposed to be indirect, so this should work.
+    QPDF* q = this->oh.getOwningQPDF();
+    if (! q)
+    {
+        return result;
+    }
+    auto acroform = q->getRoot().getKey("/AcroForm");
+    if (! acroform.isDictionary())
+    {
+        return result;
+    }
+    return acroform.getKey(name);
+}
+
+QPDFObjectHandle
 QPDFFormFieldObjectHelper::getInheritableFieldValue(std::string const& name)
 {
     QPDFObjectHandle node = this->oh;
@@ -203,20 +221,47 @@ QPDFFormFieldObjectHelper::getDefaultValueAsString()
     return getInheritableFieldValueAsString("/DV");
 }
  
+QPDFObjectHandle
+QPDFFormFieldObjectHelper::getDefaultResources()
+{
+    return getFieldFromAcroForm("/DR");
+}
+
 std::string
 QPDFFormFieldObjectHelper::getDefaultAppearance()
 {
-    return getInheritableFieldValueAsString("/DA");
+    auto value = getInheritableFieldValue("/DA");
+    bool looked_in_acroform = false;
+    if (! value.isString())
+    {
+        value = getFieldFromAcroForm("/DA");
+        looked_in_acroform = true;
+    }
+    std::string result;
+    if (value.isString())
+    {
+        QTC::TC("qpdf", "QPDFFormFieldObjectHelper DA present",
+                looked_in_acroform ? 0 : 1);
+        result = value.getUTF8Value();
+    }
+    return result;
 }
  
 int
 QPDFFormFieldObjectHelper::getQuadding()
 {
-    int result = 0;
     QPDFObjectHandle fv = getInheritableFieldValue("/Q");
+    bool looked_in_acroform = false;
+    if (! fv.isInteger())
+    {
+        fv = getFieldFromAcroForm("/Q");
+        looked_in_acroform = true;
+    }
+    int result = 0;
     if (fv.isInteger())
     {
-        QTC::TC("qpdf", "QPDFFormFieldObjectHelper Q present");
+        QTC::TC("qpdf", "QPDFFormFieldObjectHelper Q present",
+                looked_in_acroform ? 0 : 1);
         result = QIntC::to_int(fv.getIntValue());
     }
     return result;
@@ -920,7 +965,7 @@ QPDFFormFieldObjectHelper::generateTextAppearance(
         QPDFObjectHandle font = getFontFromResource(resources, font_name);
         if (! font.isInitialized())
         {
-            QPDFObjectHandle dr = getInheritableFieldValue("/DR");
+            QPDFObjectHandle dr = getDefaultResources();
             font = getFontFromResource(dr, font_name);
         }
         if (font.isInitialized() &&
@@ -148,8 +148,7 @@ QPDFPageDocumentHelper::flattenAnnotationsForPage(
                         "/Resources", as_resources.shallowCopy());
                     as_resources = as.getDict().getKey("/Resources");
                 }
-                as_resources.mergeResources(
-                    ff.getInheritableFieldValue("/DR"));
+                as_resources.mergeResources(ff.getDefaultResources());
             }
             else
             {
@@ -5061,7 +5061,7 @@ print &quot;\n&quot;;
    </varlistentry>
 -->
    <varlistentry>
-    <term>XXX 10.2.1: Month dd, YYYY</term>
+    <term>XXX 10.3.0: Month dd, YYYY</term>
     <listitem>
      <itemizedlist>
       <listitem>
@@ -5081,6 +5081,16 @@ print &quot;\n&quot;;
           rarely used method call.
          </para>
         </listitem>
+        <listitem>
+         <para>
+          Fix form field handling code to look for default
+          appearances, quadding, and default resources in the right
+          places. The code was not looking for things in the
+          document-level interactive form dictionary that it was
+          supposed to be finding there. This required adding a few new
+          methods to <classname>QPDFFormFieldObjectHelper</classname>.
+         </para>
+        </listitem>
        </itemizedlist>
       </listitem>
      </itemizedlist>
@@ -337,7 +337,8 @@ QPDFFormFieldObjectHelper TU present 0
 QPDFFormFieldObjectHelper TM present 0
 QPDFFormFieldObjectHelper TU absent 0
 QPDFFormFieldObjectHelper TM absent 0
-QPDFFormFieldObjectHelper Q present 0
+QPDFFormFieldObjectHelper Q present 1
+QPDFFormFieldObjectHelper DA present 1
 QPDFAnnotationObjectHelper AS present 0
 QPDFAnnotationObjectHelper AS absent 0
 QPDFAnnotationObjectHelper AP stream 0
@@ -335,6 +335,7 @@ my @form_tests = (
     'form-filled-with-atril',
     'form-bad-fields-array',
     'form-errors',
+    'form-document-defaults',
     );
  
 $n_tests += scalar(@form_tests) + 6;
+iterating over form fields
+Field: 4 0 R
+  Parent: none
+  Fully qualified name: Text Box 1
+  Partial name: Text Box 1
+  Alternative name: Text Box 1
+  Mapping name: Text Box 1
+  Field type: /Tx
+  Value: <feff>
+  Value as string: 
+  Default value: <feff>
+  Default value as string: 
+  Default appearance: 0.18039 0.20392 0.21176 rg /F2 12 Tf
+  Quadding: 1
+  Annotation: 83 0 R
+Field: 6 0 R
+  Parent: none
+  Fully qualified name: Check Box 1
+  Partial name: Check Box 1
+  Alternative name: Check Box 1
+  Mapping name: Check Box 1
+  Field type: /Btn
+  Value: /Off
+  Value as string: 
+  Default value: /Off
+  Default value as string: 
+  Default appearance: 0.18039 0.20392 0.21176 rg /ZaDi 0 Tf
+  Quadding: 1
+  Annotation: 6 0 R
+Field: 7 0 R
+  Parent: none
+  Fully qualified name: Check Box 2
+  Partial name: Check Box 2
+  Alternative name: Check Box 2
+  Mapping name: Check Box 2
+  Field type: /Btn
+  Value: /Yes
+  Value as string: 
+  Default value: /Yes
+  Default value as string: 
+  Default appearance: 0.18039 0.20392 0.21176 rg /ZaDi 0 Tf
+  Quadding: 1
+  Annotation: 7 0 R
+Field: 8 0 R
+  Parent: none
+  Fully qualified name: Check Box 3
+  Partial name: Check Box 3
+  Alternative name: Check Box 3
+  Mapping name: Check Box 3
+  Field type: /Btn
+  Value: /Off
+  Value as string: 
+  Default value: /Off
+  Default value as string: 
+  Default appearance: 0.18039 0.20392 0.21176 rg /ZaDi 0 Tf
+  Quadding: 1
+  Annotation: 8 0 R
+Field: 10 0 R
+  Parent: none
+  Fully qualified name: Text Box 2
+  Partial name: Text Box 2
+  Alternative name: Text Box 2
+  Mapping name: Text Box 2
+  Field type: /Tx
+  Value: <feff00730061006c00610064002003c002ac>
+  Value as string: salad πʬ
+  Default value: <feff00730061006c00610064002003c002ac>
+  Default value as string: salad πʬ
+  Default appearance: 0.18039 0.20392 0.21176 rg /F2 12 Tf
+  Quadding: 1
+  Annotation: 10 0 R
+Field: 16 0 R
+  Parent: 5 0 R
+  Parent: none
+  Fully qualified name: r1.choice1
+  Partial name: choice1
+  Alternative name: chice 1
+  Mapping name: choice 1 TM
+  Field type: /Btn
+  Value: /1
+  Value as string: 
+  Default value: /1
+  Default value as string: 
+  Default appearance: 0.18039 0.20392 0.21176 rg /ZaDi 0 Tf
+  Quadding: 1
+  Annotation: 16 0 R
+Field: 17 0 R
+  Parent: 5 0 R
+  Parent: none
+  Fully qualified name: r1
+  Partial name: 
+  Alternative name: r1
+  Mapping name: r1
+  Field type: /Btn
+  Value: /1
+  Value as string: 
+  Default value: /1
+  Default value as string: 
+  Default appearance: 0.18039 0.20392 0.21176 rg /ZaDi 0 Tf
+  Quadding: 1
+  Annotation: 17 0 R
+Field: 18 0 R
+  Parent: 5 0 R
+  Parent: none
+  Fully qualified name: r1
+  Partial name: 
+  Alternative name: r1
+  Mapping name: r1
+  Field type: /Btn
+  Value: /1
+  Value as string: 
+  Default value: /1
+  Default value as string: 
+  Default appearance: 0.18039 0.20392 0.21176 rg /ZaDi 0 Tf
+  Quadding: 1
+  Annotation: 18 0 R
+Field: 32 0 R
+  Parent: 9 0 R
+  Parent: none
+  Fully qualified name: r2
+  Partial name: 
+  Alternative name: r2
+  Mapping name: r2
+  Field type: /Btn
+  Value: /2
+  Value as string: 
+  Default value: /2
+  Default value as string: 
+  Default appearance: 0.18039 0.20392 0.21176 rg /ZaDi 0 Tf
+  Quadding: 1
+  Annotation: 32 0 R
+Field: 33 0 R
+  Parent: 9 0 R
+  Parent: none
+  Fully qualified name: r2
+  Partial name: 
+  Alternative name: r2
+  Mapping name: r2
+  Field type: /Btn
+  Value: /2
+  Value as string: 
+  Default value: /2
+  Default value as string: 
+  Default appearance: 0.18039 0.20392 0.21176 rg /ZaDi 0 Tf
+  Quadding: 1
+  Annotation: 33 0 R
+Field: 34 0 R
+  Parent: 9 0 R
+  Parent: none
+  Fully qualified name: r2
+  Partial name: 
+  Alternative name: r2
+  Mapping name: r2
+  Field type: /Btn
+  Value: /2
+  Value as string: 
+  Default value: /2
+  Default value as string: 
+  Default appearance: 0.18039 0.20392 0.21176 rg /ZaDi 0 Tf
+  Quadding: 1
+  Annotation: 34 0 R
+iterating over annotations per page
+Page: 11 0 R
+  Annotation: 83 0 R
+    Field: 4 0 R
+    Subtype: /Widget
+    Rect: [123.4, 692.1, 260.9, 706.7]
+    Appearance stream (/N): 14 0 R
+    Appearance stream (/N, /3): null
+  Annotation: 16 0 R
+    Field: 16 0 R
+    Subtype: /Widget
+    Rect: [149.3, 648.5, 161.6, 660.4]
+    Appearance state: /1
+    Appearance stream (/N): 44 0 R
+    Appearance stream (/N, /3): null
+  Annotation: 17 0 R
+    Field: 17 0 R
+    Subtype: /Widget
+    Rect: [152.7, 627.3, 165, 639.2]
+    Appearance state: /Off
+    Appearance stream (/N): 50 0 R
+    Appearance stream (/N, /3): null
+  Annotation: 18 0 R
+    Field: 18 0 R
+    Subtype: /Widget
+    Rect: [151.3, 601.7, 163.6, 613.6]
+    Appearance state: /Off
+    Appearance stream (/N): 54 0 R
+    Appearance stream (/N, /3): 52 0 R
+  Annotation: 6 0 R
+    Field: 6 0 R
+    Subtype: /Widget
+    Rect: [121.9, 559.1, 134.2, 571]
+    Appearance state: /Off
+    Appearance stream (/N): 19 0 R
+    Appearance stream (/N, /3): null
+  Annotation: 7 0 R
+    Field: 7 0 R
+    Subtype: /Widget
+    Rect: [118.6, 527.7, 130.9, 539.6]
+    Appearance state: /Yes
+    Appearance stream (/N): 26 0 R
+    Appearance stream (/N, /3): null
+  Annotation: 8 0 R
+    Field: 8 0 R
+    Subtype: /Widget
+    Rect: [118.6, 500.5, 130.9, 512.4]
+    Appearance state: /Off
+    Appearance stream (/N): 28 0 R
+    Appearance stream (/N, /3): null
+Page: 40 0 R
+Page: 35 0 R
+  Annotation: 32 0 R
+    Field: 32 0 R
+    Subtype: /Widget
+    Rect: [118.6, 555.7, 130.9, 567.6]
+    Appearance state: /Off
+    Appearance stream (/N): 58 0 R
+    Appearance stream (/N, /3): null
+  Annotation: 33 0 R
+    Field: 33 0 R
+    Subtype: /Widget
+    Rect: [119.3, 514.8, 131.6, 526.7]
+    Appearance state: /2
+    Appearance stream (/N): 60 0 R
+    Appearance stream (/N, /3): null
+  Annotation: 34 0 R
+    Field: 34 0 R
+    Subtype: /Widget
+    Rect: [121.3, 472.5, 133.6, 484.4]
+    Appearance state: /Off
+    Appearance stream (/N): 66 0 R
+    Appearance stream (/N, /3): 64 0 R
+  Annotation: 10 0 R
+    Field: 10 0 R
+    Subtype: /Widget
+    Rect: [113.6, 378.5, 351.3, 396.3]
+    Appearance stream (/N): 36 0 R
+    Appearance stream (/N, /3): null
+test 43 done