fix: Do not return 400 error for encrypted but readable pdfs (#249)

Fixes #236 

The `pdf.is_encrypted` check is true for files with edit protections, so
we were returning 400 "This file is encrypted" for files that were
perfectly readable. To verify, run this branch with `make run-web-app`
and submit the pdf in the linked issue.

Other changes:
* Remove filename from some error messages. This can get way too
verbose.

Author: awalker4

CHANGELOG.md

@@ -1,7 +1,9 @@
-## 0.0.47-dev1
+## 0.0.47
 
 * **Adds `chunking_strategy` kwarg and associated params** These params allow users to "chunk" elements into larger or smaller `CompositeElement`s
 * **Remove `parent_id` from the element metadata**. New metadata fields are causing errors with existing installs. We'll readd this once a fix is widely available.
+* **Fix some pdfs incorrectly returning a file is encrypted error**. The `pypdf.is_encrypted` check caused us to return this error even if the file is readable.
+>>>>>>> main
 
 ## 0.0.46
 

prepline_general/api/general.py

@@ -273,14 +273,17 @@ def pipeline_api(
     if file_content_type == "application/pdf":
         try:
             pdf = PdfReader(file)
+
+            # This will raise if the file is encrypted
+            pdf.metadata
         except pypdf.errors.EmptyFileError:
             raise HTTPException(
-                status_code=400, detail=f"{filename} does not appear to be a valid PDF"
+                status_code=400, detail=f"File does not appear to be a valid PDF"
             )
-        if pdf.is_encrypted:
+        except pypdf.errors.FileNotDecryptedError:
             raise HTTPException(
                 status_code=400,
-                detail=f"File: {filename} is encrypted. Please decrypt it with password.",
+                detail=f"File is encrypted. Please decrypt it with password.",
             )
 
     strategy = (m_strategy[0] if len(m_strategy) else "auto").lower()
@@ -475,7 +478,6 @@ def get_validated_mimetype(file):
             raise HTTPException(
                 status_code=400,
                 detail=(
-                    f"Unable to process {file.filename}: "
                     f"File type {content_type} is not supported."
                 ),
             )

sample-docs/layout-parser-paper-password-protected.pdf

@@ -6,6 +6,7 @@
 import pandas as pd
 from fastapi.testclient import TestClient
 from fastapi import HTTPException
+from pypdf import PdfWriter, PdfReader
 from unittest.mock import Mock, ANY
 
 from prepline_general.api.app import app
@@ -355,9 +356,7 @@ def test_general_api_returns_400_unsupported_file(example_filename):
     response = client.post(
         MAIN_API_ROUTE, files=[("files", (str(test_file), open(test_file, "rb"), filetype))]
     )
-    assert response.json() == {
-        "detail": f"Unable to process {str(test_file)}: " f"File type {filetype} is not supported."
-    }
+    assert response.json() == {"detail": f"File type {filetype} is not supported."}
     assert response.status_code == 400
 
 
@@ -371,7 +370,7 @@ def test_general_api_returns_400_bad_pdf():
     response = client.post(
         MAIN_API_ROUTE, files=[("files", (str(tmp.name), open(tmp.name, "rb"), "application/pdf"))]
     )
-    assert response.json() == {"detail": f"{tmp.name} does not appear to be a valid PDF"}
+    assert response.json() == {"detail": "File does not appear to be a valid PDF"}
     assert response.status_code == 400
     tmp.close()
 
@@ -576,25 +575,6 @@ def test_partition_file_via_api_not_retryable_error_code(monkeypatch, mocker):
     assert remote_partition.call_count < 4
 
 
-def test_password_protected_pdf():
-    """
-    Verify we get a 400 error if the PDF is password protected
-    """
-    client = TestClient(app)
-    # a password protected pdf file, password is "password"
-    test_file = Path("sample-docs") / "layout-parser-paper-password-protected.pdf"
-
-    response = client.post(
-        MAIN_API_ROUTE,
-        files=[("files", (str(test_file), open(test_file, "rb")))],
-        data={"strategy": "fast"},
-    )
-    assert response.status_code == 400
-    assert response.json() == {
-        "detail": f"File: {str(test_file)} is encrypted. Please decrypt it with password."
-    }
-
-
 def test_chunking_strategy_param():
     """
     Verify that responses do not chunk elements unless requested
@@ -666,3 +646,41 @@ def test_chunking_strategy_additional_params():
         response_multipage_true_combine_chars_5000.json()
         != response_from_multipage_false_combine_chars_0.json()
     )
+
+
+def test_encrypted_pdf():
+    """
+    Test that we throw an error if a pdf is password protected.
+    A pdf can be encrypted but still readable - don't throw an error here.
+    """
+    client = TestClient(app)
+    test_file = Path("sample-docs") / "layout-parser-paper-fast.pdf"
+    original_pdf = PdfReader(test_file)
+
+    with tempfile.NamedTemporaryFile() as temp_file:
+        # This file is user encrypted and cannot be read
+        writer = PdfWriter()
+        writer.append_pages_from_reader(original_pdf)
+        writer.encrypt(user_password="password123")
+        writer.write(temp_file.name)
+
+        # Response should be 400
+        response = client.post(
+            MAIN_API_ROUTE,
+            files=[("files", (str(temp_file.name), open(temp_file.name, "rb"), "application/pdf"))],
+        )
+        assert response.json() == {"detail": "File is encrypted. Please decrypt it with password."}
+        assert response.status_code == 400
+
+        # This file is owner encrypted, i.e. readable with edit restrictions
+        writer = PdfWriter()
+        writer.append_pages_from_reader(original_pdf)
+        writer.encrypt(user_password="", owner_password="password123", permissions_flag=0b1100)
+        writer.write(temp_file.name)
+
+        # Response should be 200
+        response = client.post(
+            MAIN_API_ROUTE,
+            files=[("files", (str(temp_file.name), open(temp_file.name, "rb"), "application/pdf"))],
+        )
+        assert response.status_code == 200