Repository: py-pdf/pypdf
Pull request: ROB : update_page_form_field_values not failing if no fields
fixes #1343

Problem statement:
KeyError: '/Annots' in update_page_form_field_values
I'm filling a PDF file with the update_page_form_field_values method but if the PDF file contains at least one page without a field a KeyError was thrown.

## Environment

Which environment were you using when you encountered the problem?

```bash
$ python -m platform
Linux-5.10.104-linuxkit-x86_64-with-glibc2.31

$ python -c "import PyPDF2;print(PyPDF2.__version__)"
2.10.6
```

## Code + PDF

This is a minimal, complete example that shows the issue:

```python
for idx, page in enumerate(reader.pages):
            writer.add_page(page)
            writer.update_page_form_field_values(
                writer.get_page(idx), self.data, FieldFlag.READ_ONLY
            )
```

## Traceback

```
File "/app/.venv/lib/python3.10/site-packages/PyPDF2/_writer.py", line 625, in update_page_form_field_values
for j in range(len(page[PG.ANNOTS])):  # type: ignore
File "/app/.venv/lib/python3.10/site-packages/PyPDF2/generic/_data_structures.py", line 149, in __getitem__
return dict.__getitem__(self, key).get_object()
KeyError: '/Annots'
```

Hints from issue discussion:

Commit message: ROB : update_page_form_field_values not failing if no fields

fixes #1343

Patch to review:
diff --git a/PyPDF2/_writer.py b/PyPDF2/_writer.py
index 942604f4e..6e2d1b8d6 100644
--- a/PyPDF2/_writer.py
+++ b/PyPDF2/_writer.py
@@ -622,6 +622,8 @@ def update_page_form_field_values(
         """
         self.set_need_appearances_writer()
         # Iterate through pages, update field values
+        if PG.ANNOTS not in page:
+            return
         for j in range(len(page[PG.ANNOTS])):  # type: ignore
             writer_annot = page[PG.ANNOTS][j].get_object()  # type: ignore
             # retrieve parent field values, if present
diff --git a/tests/test_writer.py b/tests/test_writer.py
index 9c8f0dae3..7b5c6f303 100644
--- a/tests/test_writer.py
+++ b/tests/test_writer.py
@@ -370,11 +370,17 @@ def test_fill_form():
     page = reader.pages[0]
 
     writer.add_page(page)
+    writer.add_page(PdfReader(RESOURCE_ROOT / "crazyones.pdf").pages[0])
 
     writer.update_page_form_field_values(
         writer.pages[0], {"foo": "some filled in text"}, flags=1
     )
 
+    # check if no fields to fill in the page
+    writer.update_page_form_field_values(
+        writer.pages[1], {"foo": "some filled in text"}, flags=1
+    )
+
     writer.update_page_form_field_values(
         writer.pages[0], {"foo": "some filled in text"}
     )


Code review comments:
diff_hunk: @@ -622,6 +622,8 @@ def update_page_form_field_values(
         """
         self.set_need_appearances_writer()
         # Iterate through pages, update field values
+        if PG.ANNOTS not in page:
+            return
path: PyPDF2/_writer.py
text: @user1:
Shouldn't we add the attribute if it doesn't exist, e.g.:

```suggestion
        if PG.ANNOTS not in page:
            page[PG.ANNOTS] = DictionaryObject()
```

@author:
if the form has no fields to field, adding the PG.ANNOTS field may be of no use and quite confusing after (looking at the page structure you may think after there is so fields to fill up whereas there isn't)

@user1:
> if the form has no fields to field

I guess there was a typo? I don't understand this.

At the very least we should emit a warning ... if we are in strict mode maybe even an exception?

@author:
if the form has no fields to _be filled_, adding the PG.ANNOTS field may be of no use and quite confusing after (looking at the page structure you may think after there is so fields to fill up whereas there isn't) ; more consistent with PDF philosophy : if you don't need it, it is not in the dictionary (no fields means logically no PG.ANNOTS entry)

@user1:
I understand why you don't want to add it. It makes sense to me now.

However, looking at it from the users perspective: The user wants to fill a form field that doesn't exist. PyPDF2 executes sucessfully without an exception, without a warning, without a log message. No field is filled. That feels wrong to me.

A warning log message - something like "Filling fields was skipped as the page does not have any annotations" would give the user a chance to fix the code.