Archives: an unpaired-surrogate title must not kill the modeler.

Scraped pages can carry half an emoji in their titles; sqlite3 refuses
to store lone surrogates, so one poisoned title failed archive_modeler's
batch commit and ended the whole modeling run on 10.0.0.9.  Every
archive after it stayed unmodeled, and cleanup then deleted their
'empty' domain collections (1,670 -> 578).

Strip surrogates at every point Archive ingests scraped strings (titles,
author, description, url, article text), and make the modeler skip a
batch whose commit fails instead of dying.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

modules/archive/__init__.py

@@ -13,7 +13,7 @@ from sqlalchemy.orm import Session
 from wrolpi.cmd import SINGLE_FILE_BIN, CHROMIUM, FIREFOX, DENO_BIN, SINGLE_FILE_DENO_SCRIPT
 from wrolpi.collections import Collection
 from wrolpi.common import logger, register_modeler, register_refresh_cleanup, limit_concurrent, split_lines_by_length, \
-    slow_logger, get_title_from_html, TRACE_LEVEL
+    slow_logger, get_title_from_html, strip_surrogates, TRACE_LEVEL
 from wrolpi.db import get_db_session
 from wrolpi.downloader import Downloader, Download, DownloadContext, DownloadResult
 from wrolpi.errors import UnrecoverableDownloadError
@@ -306,10 +306,13 @@ def model_archive(session: Session, file_group: FileGroup) -> Archive:
             title = get_title(readability_json_path)
         if not title:
             title = get_title_from_html(lib.read_singlefile_html(singlefile_path))
+        # Scraped titles can contain unpaired surrogates which sqlite3 refuses to store; a poisoned
+        # title fails the modeler's batch commit (it killed archive modeling on 10.0.0.9).
+        title = strip_surrogates(title)
 
         contents = None
         if readability_txt_path:
-            contents = get_article(readability_txt_path)
+            contents = strip_surrogates(get_article(readability_txt_path))
 
         # Check if an Archive already exists for this FileGroup
         archive = session.query(Archive).filter_by(file_group_id=file_group_id).one_or_none()
@@ -405,7 +408,17 @@ async def archive_modeler(progress_callback: Callable[[int], None] = None):
                             if PYTEST:
                                 raise
 
-            session.commit()
+            try:
+                session.commit()
+            except Exception as e:
+                # One poisoned row must not kill the modeler: skip this batch for the rest of this
+                # run and keep modeling.  The batch stays unindexed, so the next refresh retries it.
+                session.rollback()
+                logger.error(f'archive_modeler failed to commit batch, skipping {len(results_list)} FileGroups',
+                             exc_info=e)
+                invalid_archives.update(fg.id for fg, _ in results_list)
+                if PYTEST:
+                    raise
 
             # Report batch progress
             batch_count = len(results_list)

modules/archive/models.py

@@ -9,7 +9,8 @@ from sqlalchemy.orm import relationship, Session
 
 from wrolpi import dates
 from wrolpi.collections import Collection
-from wrolpi.common import ModelHelper, Base, logger, get_title_from_html, get_wrolpi_config, get_media_directory
+from wrolpi.common import ModelHelper, Base, logger, get_title_from_html, get_wrolpi_config, get_media_directory, \
+    strip_surrogates
 from wrolpi.dates import now
 from wrolpi.errors import UnknownArchive
 from wrolpi.files.models import FileGroup
@@ -283,8 +284,9 @@ class Archive(Base, ModelHelper):
         try:
             with readability_json_path.open() as fh:
                 json_contents = json.load(fh)
-                url = json_contents.get('url')
-                title = json_contents.get('title')
+                # Scraped strings can contain unpaired surrogates which sqlite3 refuses to store.
+                url = strip_surrogates(json_contents.get('url'))
+                title = strip_surrogates(json_contents.get('title'))
         except Exception as e:
             raise InvalidArchive() from e
 
@@ -346,7 +348,7 @@ class Archive(Base, ModelHelper):
         # Local import to avoid circular import within archive module
         from modules.archive import lib
         if self.singlefile_path and not self.file_group.title:
-            self.file_group.title = get_title_from_html(lib.read_singlefile_html(self.singlefile_path))
+            self.file_group.title = strip_surrogates(get_title_from_html(lib.read_singlefile_html(self.singlefile_path)))
 
     def apply_metadata(self):
         """Read and apply <meta> (and more) data from the Singlefile HTML."""
@@ -356,11 +358,11 @@ class Archive(Base, ModelHelper):
 
         metadata = lib.parse_article_html_metadata(contents)
         if metadata.author:
-            self.file_group.author = metadata.author
+            self.file_group.author = strip_surrogates(metadata.author)
         if metadata.title:
-            self.file_group.title = metadata.title
+            self.file_group.title = strip_surrogates(metadata.title)
         if metadata.description:
-            self.file_group.b_text = metadata.description
+            self.file_group.b_text = strip_surrogates(metadata.description)
         if metadata.published_datetime:
             self.file_group.published_datetime = metadata.published_datetime
         if metadata.modified_datetime:

modules/archive/test/test_archive_modeler_loop.py

@@ -105,3 +105,43 @@ async def test_archive_modeler_processes_more_than_20_files(async_client, test_s
     ).count()
     assert still_unindexed == 0, \
         f"All HTML FileGroups should be indexed, but {still_unindexed} remain unindexed"
+
+
+@pytest.mark.asyncio
+async def test_archive_modeler_surrogate_title(async_client, test_session, test_directory,
+                                               make_files_structure):
+    """A scraped title containing an unpaired UTF-16 surrogate must not kill the modeler.
+
+    Regression test from 10.0.0.9: one archive's readability title contained half of an emoji
+    ('\\ud83c'); sqlite3 refused the batch commit with "UnicodeEncodeError: surrogates not
+    allowed", the whole archive_modeler run died, and every archive after it was left
+    unmodeled (their empty domain collections were then deleted by cleanup)."""
+    domain = 'example.com'
+    archive_dir = test_directory / 'archives' / domain
+    archive_dir.mkdir(parents=True)
+
+    files_structure = {}
+    for i in range(3):
+        timestamp = archive_strftime(datetime.datetime(2000, 1, 1 + i, 0, 0, 0).astimezone(pytz.UTC))
+        url = f'https://{domain}/article-{i}'
+        files_structure[str(archive_dir / f'{timestamp}_Article {i}.html')] = \
+            create_singlefile_content(url, f'Article {i}')
+        if i == 1:
+            # The readability json's title wins over the HTML title; give it a lone surrogate.
+            files_structure[str(archive_dir / f'{timestamp}_Article {i}.readability.json')] = \
+                '{"title": "Broken emoji \\ud83c title", "url": "%s"}' % url
+
+    files = make_files_structure(files_structure)
+    for file_path in files:
+        if pathlib.Path(file_path).suffix == '.html':
+            FileGroup.from_paths(test_session, *[f for f in files
+                                                 if str(f).startswith(str(file_path)[:-5])])
+    test_session.commit()
+
+    await archive_modeler()
+
+    archives = test_session.query(Archive).all()
+    assert len(archives) == 3
+    titles = sorted(a.file_group.title for a in archives)
+    # The surrogate was stripped, the rest of the title survived.
+    assert titles == ['Article 0', 'Article 2', 'Broken emoji  title']

wrolpi/common.py

@@ -2678,6 +2678,15 @@ WHITESPACE_SPLITTER = re.compile(r'\s+')
 TAB = re.compile(r'\t+')
 
 
+def strip_surrogates(text: Optional[str]) -> Optional[str]:
+    """Remove unpaired UTF-16 surrogate characters (e.g. broken emoji halves from scraped HTML).
+
+    sqlite3 refuses to store them: "UnicodeEncodeError: surrogates not allowed"."""
+    if text is None:
+        return None
+    return text.encode('utf-8', errors='ignore').decode('utf-8')
+
+
 def split_lines_by_length(text: str, max_line_length: int = 38) -> str:
     """
     Break up any long lines along word boundaries.

wrolpi/files/lib.py

@@ -28,7 +28,7 @@ from wrolpi.common import get_media_directory, wrol_mode_check, logger, \
     partition, \
     get_files_and_directories, chunks_by_stem, walk, \
     get_wrolpi_config, \
-    unique_by_predicate, get_paths_in_media_directory, TRACE_LEVEL, get_relative_to_media_directory
+    unique_by_predicate, get_paths_in_media_directory, TRACE_LEVEL, get_relative_to_media_directory, strip_surrogates
 from wrolpi.dates import now, from_timestamp, months_selector_to_where, date_range_to_where
 from wrolpi.db import get_db_session, get_db_curs, values_clause
 from wrolpi.downloader import download_manager, Download
@@ -929,7 +929,7 @@ async def apply_indexers(progress_callback: Callable[[int, int], None] = None):
                    ' c_text = :c_text, d_text = :d_text WHERE id = :id')
 
     def _strip_surrogates(value):
-        return value.encode('utf-8', errors='replace').decode('utf-8') if isinstance(value, str) else value
+        return strip_surrogates(value) if isinstance(value, str) else value
 
     while True:
         # Read the next batch as plain columns and let the read transaction end.  Indexers can run