Always set page positions

documentcloud/documents/models/document.py

@@ -486,42 +486,34 @@ def set_page_text(self, page_text_infos):
 
     def _set_page_positions(self, pages, file_names, file_contents):
         """Handle grafting page positions back into the document"""
-
         current_pdf = pymupdf.open(stream=storage.open(self.doc_path, "rb").read())
         start_page = pages[0]["page_number"]
         stop_page = pages[-1]["page_number"]
 
+        # always write the position JSON - this is cheap and does not cause the
+        # memory issues that gating below guards against
+        self._write_position_json(pages, file_names, file_contents)
+
         visible_text = self._check_visible_text(current_pdf, start_page, stop_page)
         logger.info(
             "[SET PAGE TEXT] %d - visible text detected: %s", self.pk, visible_text
         )
         if visible_text:
             # merging when we need to flatten visible text causes excessive memory usage
+            current_pdf.close()
             return None
-
         grafted_pdf, base_pdf_stream = self._init_graft_pdf(
             current_pdf,
             start_page,
             stop_page,
             visible_text,
         )
-
         for page in pages:
             page_number = page["page_number"]
             if page.get("positions"):
-                logger.info(
-                    "[SET PAGE TEXT] %d - positions page %d", self.pk, page_number
-                )
-                file_names.append(
-                    path.page_text_position_path(self.pk, self.slug, page_number)
-                )
-                positions = [{**p.pop("metadata", {}), **p} for p in page["positions"]]
-                file_contents.append(json.dumps(positions).encode("utf-8"))
-
                 logger.info("[SET PAGE TEXT] %d - graft page %d", self.pk, page_number)
                 # create the overlay file
                 graft_page(page["positions"], grafted_pdf[page_number - start_page])
-
         # merge the overlay pages back onto the original document
         if visible_text:
             contents = self._merge_overlay_visible(
@@ -539,9 +531,26 @@ def _set_page_positions(self, pages, file_names, file_contents):
             )
         current_pdf.close()
         grafted_pdf.close()
-
         return contents
 
+    def _write_position_json(self, pages, file_names, file_contents):
+        """Stage the per-page position JSON files for upload.
+
+        Always safe to run - writing the position files is cheap and does not
+        trigger the memory issues associated with grafting into the PDF.
+        """
+        for page in pages:
+            page_number = page["page_number"]
+            if page.get("positions"):
+                logger.info(
+                    "[SET PAGE TEXT] %d - positions page %d", self.pk, page_number
+                )
+                file_names.append(
+                    path.page_text_position_path(self.pk, self.slug, page_number)
+                )
+                positions = [{**p.pop("metadata", {}), **p} for p in page["positions"]]
+                file_contents.append(json.dumps(positions).encode("utf-8"))
+
     def _merge_overlay(self, base_pdf_stream, grafted_pdf, start_page, stop_page):
         """Merge the text only overlay pages back in to the base PDF"""
         base_pdf = Pdf.open(base_pdf_stream)

documentcloud/oembed/views.py

@@ -112,7 +112,16 @@ def get_int(key):
             except (ValueError, KeyError):
                 return None
 
-        furl_url = furl(request.GET["url"])
+        try:
+            furl_url = furl(request.GET["url"])
+        except ValueError:
+            # furl raises ValueError on port errors,
+            # malformed IPs, and invalid hosts,
+            # so we must handle this
+            return Response(
+                {"error": "Invalid or missing url parameter"},
+                status=status.HTTP_400_BAD_REQUEST,
+            )
         # remove _escaped_fragment_ if it exists
         furl_url.query.params.pop("_escaped_fragment_", None)
         # always add embed=1