A malformed config entry is skipped and counted instead of failing the import (#875)

* A malformed config entry is skipped and counted instead of failing the import

An entry that is not a mapping (or whose key is not text, or a list that is not a list) made the whole file fail to
import in playlists.yaml, repos.yaml, download_manager.yaml and tags.yaml (tag_files/tag_zims), and a non-text name
did in domains.yaml.  Each is now skipped and counted in import_skipped, so the config is not saved over it, and the
rest of the file imports.  A Channel whose `downloads` is not a list keeps its Downloads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* An unreadable entry keeps no row and does not make a section present

The key of an entry too malformed to name a row is now `Unreadable(position)`.  It is still counted in
import_skipped, but `present` ignores it, so a section whose entries are all unreadable removes nothing (as an empty
section), instead of deleting every row.  A playlist whose `items` is not a list keeps its items, and a Channel
Download whose url is not text is counted (its Channel keeps its Downloads) instead of failing the import.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

modules/archive/lib.py

@@ -28,7 +28,7 @@ from wrolpi.common import get_media_directory, get_relative_to_media_directory,
     escape_file_name, aiohttp_post, format_html_string, split_lines_by_length, get_html_soup, get_title_from_html, \
     get_wrolpi_config, html_screenshot, html_file_screenshot, ConfigFile, trim_file_name, chunks
 from wrolpi.collections.models import validate_collection_directory
-from wrolpi.config_import import ConfigSection, media_relative
+from wrolpi.config_import import ConfigSection, Unreadable, media_relative
 from wrolpi.dates import now, Seconds
 from wrolpi.db import get_db_session, get_db_curs, session_curs
 from wrolpi.errors import UnknownArchive, InvalidOrderBy, InvalidDatetime
@@ -65,10 +65,10 @@ class DomainsSection(ConfigSection):
         rows, invalid = dict(), set()
         for position, data in enumerate(config.get('collections') or []):
             name = data.get('name') if isinstance(data, dict) else None
-            if not name:
+            if not name or not isinstance(name, str):
                 logger.error(f'Domain collection at index {position} has no name, skipping: {data!r}')
-                # Counted as skipped; never a domain's name.
+                # Counted as skipped (see `Unreadable`).
-                invalid.add(f'\0{position}')
+                invalid.add(Unreadable(position))
                 continue
             try:
                 if not Collection.is_valid_domain_name(name):

modules/repos/lib.py

@@ -25,7 +25,7 @@ from wrolpi.cmd import GIT_BIN, run_command
 from wrolpi.collections.models import Collection
 from wrolpi.common import ConfigFile, get_media_directory, get_relative_to_media_directory, logger, \
     escape_file_name, background_task, get_wrolpi_config
-from wrolpi.config_import import ConfigSection, media_relative
+from wrolpi.config_import import ConfigSection, Unreadable, media_relative
 from wrolpi.config_version import register_ownership
 from wrolpi.db import get_db_session
 from wrolpi.downloader import Download, DownloadFrequency, download_manager, save_downloads_config
@@ -1397,11 +1397,11 @@ class ReposSection(ConfigSection):
         media_directory = get_media_directory()
         rows, invalid = dict(), set()
         for position, data in enumerate(config.get('repos') or []):
-            url = data.get('url')
+            url = data.get('url') if isinstance(data, dict) else None
-            if not url:
+            if not url or not isinstance(url, str):
-                logger.error(f'Repo config entry has no url, skipping: {data}')
+                logger.error(f'Repo config entry has no url, skipping: {data!r}')
-                # Counted as skipped; never a Repo's key.
+                # Counted as skipped (see `Unreadable`).
-                invalid.add(f'\0{position}')
+                invalid.add(Unreadable(position))
                 continue
             try:
                 parsed = parse_repo_url(url)

modules/repos/test/test_config_import.py

@@ -102,6 +102,34 @@ async def test_invalid_entry_is_kept(test_session, saved_repos):
     assert (found[TWO].mode, found[ONE].collection.description) == ('snapshot', 'Changed')
 
 
+@pytest.mark.asyncio
+async def test_malformed_entry_is_skipped(test_session, saved_repos):
+    """An entry that is not a mapping is skipped and counted; the rest of the file still imports."""
+    edit_repo(saved_repos, ONE, description='Changed')
+    edit_config(saved_repos, lambda data: data['repos'].insert(0, 'not a mapping'))
+
+    config = lib.get_repos_config()
+    config.import_config()
+
+    assert config.successful_import is True
+    assert config.import_skipped == 1
+    found = repos(test_session)
+    assert set(found) == {ONE, TWO}
+    assert found[ONE].collection.description == 'Changed'
+
+
+@pytest.mark.asyncio
+async def test_only_malformed_entries_delete_nothing(test_session, saved_repos):
+    """A file whose entries are all unreadable deletes no Repo (like an empty list)."""
+    edit_config(saved_repos, lambda data: data.update(repos=['not a mapping', dict(url=['a', 'list'])]))
+
+    config = lib.get_repos_config()
+    config.import_config()
+
+    assert config.import_skipped == 2
+    assert set(repos(test_session)) == {ONE, TWO}
+
+
 @pytest.mark.asyncio
 async def test_failed_import_creates_nothing(test_session, saved_repos, monkeypatch):
     """A new Repo is created in the import's one write: a failure later in the write leaves nothing (each new Repo

modules/videos/lib.py

@@ -20,7 +20,7 @@ from wrolpi.common import ConfigFile, extract_domain, logger, \
     escape_file_name, get_media_directory, background_task, Base, get_wrolpi_config, trim_file_name
 from wrolpi.collections import Collection
 from wrolpi.collections.models import validate_collection_directory
-from wrolpi.config_import import ConfigSection, chunks, media_relative
+from wrolpi.config_import import ConfigSection, Unreadable, chunks, media_relative
 from wrolpi.config_version import register_ownership
 from wrolpi.dates import Seconds, from_timestamp
 from wrolpi.db import get_db_curs, get_db_session, session_curs
@@ -295,8 +295,8 @@ class ChannelsSection(ConfigSection):
                 key = _channel_key(data)
             except Exception as e:
                 channel_import_logger.error(f'Skipping invalid Channel in config: {data!r}', exc_info=e)
-                # Counted as skipped; never a directory.  (The entry's name keeps its Channel, see `prepare`.)
+                # Counted as skipped (see `Unreadable`).  The entry's name keeps its Channel (see `prepare`).
-                invalid.add(f'\0{position}')
+                invalid.add(Unreadable(position))
                 continue
             if key in rows or key in invalid:
                 raise RuntimeError('Refusing to import channels config because it contains duplicate directories!')
@@ -394,8 +394,14 @@ class ChannelDownloadsSection(ConfigSection):
                 key = _channel_key(data)
             except Exception:
                 continue
-            for position, download in enumerate(data.get('downloads') or []):
+            downloads = data.get('downloads') or []
-                if isinstance(download, dict) and download.get('url'):
+            if not isinstance(downloads, list):
+                channel_import_logger.error(f'Skipping the Downloads of Channel {key!r}, they are not a list')
+                # Counted as skipped; the Channel keeps its Downloads (see `ChannelsConfig.prepare`).
+                invalid.add((key, '\0downloads'))
+                continue
+            for position, download in enumerate(downloads):
+                if isinstance(download, dict) and download.get('url') and isinstance(download['url'], str):
                     rows[(key, download['url'])] = dict(frequency=download.get('frequency'))
                 else:
                     channel_import_logger.error(f'Skipping invalid Download of Channel {key!r}: {download!r}')

wrolpi/collections/config.py

@@ -7,7 +7,7 @@ from sqlalchemy import insert
 from sqlalchemy.orm import Session
 
 from wrolpi.common import ConfigFile, get_media_directory, get_relative_to_media_directory, logger
-from wrolpi.config_import import ConfigSection, SectionDiff, chunks, media_relative
+from wrolpi.config_import import ConfigSection, SectionDiff, Unreadable, chunks, media_relative
 from wrolpi.config_version import count_config_change
 from wrolpi.db import get_db_session
 from wrolpi.switches import ActivateSwitchMethod, register_switch_handler
@@ -23,16 +23,28 @@ def _playlist_tag_names(rows) -> Set[str]:
     return {i['tag_name'] for i in rows if i['tag_name']}
 
 
+# The position in an invalid item key that stands for all of a playlist's items (its `items` is not a list).
+ITEMS_UNREADABLE = 0
+
+
+def _playlist_name(data) -> Optional[str]:
+    """The name of a playlist entry, or None when the entry is malformed."""
+    name = data.get('name') if isinstance(data, dict) else None
+    return name if isinstance(name, str) and name else None
+
+
 class PlaylistsSection(ConfigSection):
     """playlists.yaml's playlists (Collections with kind 'playlist'), keyed by name."""
     name = 'playlists'
 
     def file_rows(self, config: dict) -> Tuple[Dict[str, dict], Set[str]]:
         rows, invalid = dict(), set()
-        for data in config.get('playlists') or []:
+        for position, data in enumerate(config.get('playlists') or []):
-            name = data.get('name')
+            name = _playlist_name(data)
             if not name:
-                logger.error(f'Playlist config entry has no name, skipping: {data}')
+                logger.error(f'Playlist config entry has no name, skipping: {data!r}')
+                # Counted as skipped (see `Unreadable`).
+                invalid.add(Unreadable(position))
                 continue
             try:
                 rows[name] = dict(
@@ -88,23 +100,32 @@ class PlaylistItemsSection(ConfigSection):
 
     @staticmethod
     def item_key(name: str, data: dict) -> Optional[tuple]:
+        if not isinstance(data, dict):
+            raise ValueError('An item must be a mapping')
         if 'file' in data:
             return name, 'file', media_relative(data['file']), None
         elif 'zim' in data:
-            if not data.get('entry'):
+            if not data.get('entry') or not isinstance(data['entry'], str):
                 raise ValueError('A zim item requires an entry')
             return name, 'zim', media_relative(data['zim']), data['entry']
         elif 'url' in data:
+            if not isinstance(data['url'], str):
+                raise ValueError('An item\'s url must be text')
             return name, 'url', data['url'], None
         raise ValueError('An item requires a file, zim, or url')
 
     def file_rows(self, config: dict) -> Tuple[Dict[tuple, dict], Set[tuple]]:
         rows, invalid = dict(), set()
         for data in config.get('playlists') or []:
-            if not (name := data.get('name')):
+            if not (name := _playlist_name(data)):
+                continue
+            items = data.get('items') or []
+            if not isinstance(items, list):
+                logger.error(f'Skipping the items of playlist {name!r}, they are not a list: {items!r}')
+                invalid.add((name, None, ITEMS_UNREADABLE, None))
                 continue
             # Positions follow the file's order; an item skipped now keeps its place for a later import.
-            for position, item in enumerate(data.get('items') or [], start=1):
+            for position, item in enumerate(items, start=1):
                 try:
                     key = self.item_key(name, item)
                 except Exception as e:
@@ -121,7 +142,7 @@ class PlaylistItemsSection(ConfigSection):
 
     def present(self, config: dict, file_rows: dict, invalid_keys: set) -> bool:
         # A listed playlist without items has none: only an empty or missing playlists list removes nothing.
-        return any(i.get('name') for i in config.get('playlists') or [])
+        return any(map(_playlist_name, config.get('playlists') or []))
 
     @staticmethod
     def _query(session: Session, *columns):
@@ -250,6 +271,7 @@ class PlaylistsConfig(ConfigFile):
     def prepare(self, diffs: dict):
         self._drop_unknown_tags(diffs['playlists'])
         self._drop_missing_items(diffs['items'])
+        self._keep_unreadable_items(diffs['items'])
         if not self.import_deletes:
             self._merge_items(diffs['items'])
 
@@ -268,6 +290,12 @@ class PlaylistsConfig(ConfigFile):
                 row['tag_name'] = None
                 self._config_outdated = True
 
+    def _keep_unreadable_items(self, diff: SectionDiff):
+        """A playlist whose items cannot be read keeps the items it has."""
+        _, invalid = PlaylistItemsSection().file_rows(self._config)
+        if names := {i[0] for i in invalid if i[2] == ITEMS_UNREADABLE}:
+            diff.removed = {i for i in diff.removed if i[0] not in names}
+
     def _drop_missing_items(self, diff: SectionDiff):
         """An added item whose file or Zim is not in the database (not indexed yet, after a rebuild) is skipped
         before the write; a later import (after the refresh) adds it."""

wrolpi/collections/test/test_playlists_config.py

@@ -414,3 +414,60 @@ async def test_import_without_deletes_keeps_database_order(test_session, test_di
     test_session.expire_all()
     collection = test_session.query(Collection).filter_by(name='Mix', kind='playlist').one()
     assert [(i.url, i.position) for i in collection.items] == [('/a', 1), ('/x', 2), ('/b', 3)]
+
+
+@pytest.mark.asyncio
+async def test_import_skips_malformed_entries(test_session, test_directory, playlists_config):
+    """A playlist entry or item that is not a mapping is skipped and counted; the rest of the file still imports."""
+    _make_playlist(test_session)
+
+    file = playlists_config.get_file()
+    file.parent.mkdir(parents=True, exist_ok=True)
+    file.write_text('version: 1\nplaylists:\n- not a mapping\n- name: Survives\n  items:\n  - also not a mapping\n'
+                    '  - url: /map?lat=1&lon=2&z=3\n    title: spot\n- name: New\n')
+
+    config_was_imported(test_session, 'playlists.yaml')
+    playlists_config.import_config()
+
+    assert playlists_config.successful_import is True
+    assert playlists_config.import_skipped == 2
+    test_session.expire_all()
+    playlists = {i.name: i for i in test_session.query(Collection).filter_by(kind='playlist')}
+    assert set(playlists) == {'Survives', 'New'}
+    assert len(playlists['Survives'].items) == 1
+
+
+@pytest.mark.asyncio
+async def test_import_only_malformed_entries_preserves_db(test_session, test_directory, playlists_config):
+    """A file whose playlist entries are all unreadable deletes no playlist (like an empty list)."""
+    _make_playlist(test_session)
+
+    file = playlists_config.get_file()
+    file.parent.mkdir(parents=True, exist_ok=True)
+    file.write_text('version: 1\nplaylists:\n- not a mapping\n- name: [a, list]\n')
+
+    config_was_imported(test_session, 'playlists.yaml')
+    playlists_config.import_config()
+
+    assert playlists_config.import_skipped == 2
+    test_session.expire_all()
+    collection = test_session.query(Collection).filter_by(kind='playlist').one()
+    assert (collection.name, len(collection.items)) == ('Survives', 1)
+
+
+@pytest.mark.asyncio
+async def test_import_unreadable_items_keeps_items(test_session, test_directory, playlists_config):
+    """A playlist whose items are not a list keeps the items it has."""
+    _make_playlist(test_session)
+
+    file = playlists_config.get_file()
+    file.parent.mkdir(parents=True, exist_ok=True)
+    file.write_text('version: 1\nplaylists:\n- name: Survives\n  items: 5\n')
+
+    config_was_imported(test_session, 'playlists.yaml')
+    playlists_config.import_config()
+
+    assert playlists_config.import_skipped == 1
+    test_session.expire_all()
+    collection = test_session.query(Collection).filter_by(kind='playlist').one()
+    assert len(collection.items) == 1

wrolpi/config_import.py

@@ -27,6 +27,18 @@ class SectionDiff:
         return bool(self.added or self.changed or self.removed)
 
 
+@dataclass(frozen=True)
+class Unreadable:
+    """The invalid key of an entry too malformed to name a row (not a mapping, no name).  It is counted as skipped, so
+    the file is not saved over it, but it keeps no row, and on its own it does not make a section present."""
+    position: int
+
+
+def listed(file_rows: dict, invalid_keys: set) -> bool:
+    """Whether a section lists anything: a row, or an invalid row it names."""
+    return bool(file_rows) or any(not isinstance(i, Unreadable) for i in invalid_keys)
+
+
 class ConfigSection:
     """One kind of row a database config saves.
 
@@ -52,7 +64,7 @@ class ConfigSection:
     def present(self, config: dict, file_rows: dict, invalid_keys: set) -> bool:
         """Whether the file lists this section at all; an empty section cannot be told apart from a lost or blanked
         file, so it removes nothing."""
-        return bool(file_rows or invalid_keys)
+        return listed(file_rows, invalid_keys)
 
     def same(self, file_row: dict, db_row: dict) -> bool:
         """Whether a file row is already in the database."""
@@ -77,7 +89,7 @@ def diff_section(file_rows: Dict[Hashable, dict], invalid_keys: Set[Hashable], d
         elif not same(row, db_rows[key]):
             diff.changed[key] = row
     if present is None:
-        present = bool(file_rows or invalid_keys)
+        present = listed(file_rows, invalid_keys)
     if deletes and present:
         diff.removed = db_rows.keys() - file_rows.keys() - invalid_keys
     return diff

wrolpi/downloader.py

@@ -34,7 +34,7 @@ from wrolpi.cmd import which, run_command, CommandResult
 from wrolpi.common import loaded, Base, ModelHelper, logger, wrol_mode_check, zig_zag, ConfigFile, \
     wrol_mode_enabled, background_task, get_absolute_media_path, timer, aiohttp_get, \
     get_download_info, trim_file_name, get_wrolpi_config, TRACE_LEVEL, normalize_domain
-from wrolpi.config_import import ConfigSection, chunks
+from wrolpi.config_import import ConfigSection, Unreadable, chunks
 from wrolpi.config_version import collection_kinds, count_config_change, register_ownership
 from wrolpi.dates import TZDateTime, now, Seconds
 from wrolpi.db import get_db_session, session_curs, serializer
@@ -2109,8 +2109,13 @@ class DownloadsSection(ConfigSection):
     def file_rows(self, config: dict) -> Tuple[Dict[str, dict], Set[str]]:
         defaults = inheritable_video_defaults()
         rows, invalid = dict(), set()
-        for data in config.get('downloads') or []:
+        for position, data in enumerate(config.get('downloads') or []):
-            url = data.get('url')
+            url = data.get('url') if isinstance(data, dict) else None
+            if not url or not isinstance(url, str):
+                logger.error(f'Download config entry has no url, skipping: {data!r}')
+                # Counted as skipped (see `Unreadable`).
+                invalid.add(Unreadable(position))
+                continue
             try:
                 settings = dict(data.get('settings') or {})
                 # An old config kept `destination` and `tag_names` in the settings.

wrolpi/tags.py

@@ -14,7 +14,7 @@ from sqlalchemy.orm import Mapped, relationship, Session, load_only
 from wrolpi import dates, flags
 from wrolpi.common import loaded, ModelHelper, Base, logger, ConfigFile, get_media_directory, background_task, \
     get_relative_to_media_directory, is_valid_hex_color, walk, INVALID_FILE_CHARS, get_wrolpi_config
-from wrolpi.config_import import ConfigSection, SectionDiff, chunks, media_relative
+from wrolpi.config_import import ConfigSection, SectionDiff, Unreadable, chunks, media_relative
 from wrolpi.config_version import count_config_change, register_ownership
 from wrolpi.dates import TZDateTime
 from wrolpi.db import get_db_session, named_placeholders, session_curs, serializer
@@ -400,12 +400,16 @@ class TagFilesSection(ConfigSection):
     def file_rows(self, config: dict) -> Tuple[Dict[tuple, dict], Set[tuple]]:
         tag_names = set(config.get('tags') or dict())
         rows, invalid = dict(), set()
-        for item in config.get('tag_files') or []:
+        for position, item in enumerate(config.get('tag_files') or []):
             try:
-                tag_name, path, *rest = item
+                tag_name, path, *rest = item if isinstance(item, list) else ()
+                if not isinstance(tag_name, str):
+                    raise ValueError('A Tag name must be text')
                 key = (tag_name, media_relative(path))
             except Exception as e:
-                logger.error(f'Skipping invalid tagged file in config: {item}', exc_info=e)
+                logger.error(f'Skipping invalid tagged file in config: {item!r}', exc_info=e)
+                # Counted as skipped (see `Unreadable`).
+                invalid.add(Unreadable(position))
                 continue
             if tag_names and tag_name not in tag_names:
                 logger.warning(f'Cannot find Tag for {repr(key[1])}')
@@ -499,12 +503,16 @@ class TagZimsSection(ConfigSection):
     def file_rows(self, config: dict) -> Tuple[Dict[tuple, dict], Set[tuple]]:
         tag_names = set(config.get('tags') or dict())
         rows, invalid = dict(), set()
-        for item in config.get('tag_zims') or []:
+        for position, item in enumerate(config.get('tag_zims') or []):
             try:
-                tag_name, zim_path, zim_entry, *rest = item
+                tag_name, zim_path, zim_entry, *rest = item if isinstance(item, list) else ()
+                if not isinstance(tag_name, str) or not isinstance(zim_entry, str):
+                    raise ValueError('A Tag name and a Zim entry must be text')
                 key = (tag_name, media_relative(zim_path), zim_entry)
             except Exception as e:
-                logger.error(f'Skipping invalid tagged Zim entry in config: {item}', exc_info=e)
+                logger.error(f'Skipping invalid tagged Zim entry in config: {item!r}', exc_info=e)
+                # Counted as skipped (see `Unreadable`).
+                invalid.add(Unreadable(position))
                 continue
             if tag_names and tag_name not in tag_names:
                 logger.warning(f'Cannot find Tag for {repr(key[1])}')

wrolpi/test/test_config_malformed_entries.py

@@ -0,0 +1,100 @@
+"""A malformed entry in a config (not a mapping, or a key that is not a string) is skipped and counted, so the config
+is never saved over it; the rest of the file still imports."""
+import pytest
+
+from wrolpi.config_import import Unreadable, diff_section
+
+from modules.archive.lib import DomainsSection
+from modules.repos.lib import ReposSection
+from modules.videos.lib import ChannelDownloadsSection, ChannelsSection
+from wrolpi.collections.config import PlaylistItemsSection, PlaylistsSection
+from wrolpi.downloader import DownloadsSection
+from wrolpi.tags import TagFilesSection, TagsSection, TagZimsSection
+
+MALFORMED = ['a string', 5, ['a', 'list'], None]
+
+
+def assert_skipped(section, config, valid_key, malformed_count):
+    rows, invalid = section.file_rows(config)
+    assert valid_key in rows
+    assert len(invalid) >= malformed_count, 'every malformed entry must be counted, so the config is not saved over it'
+
+
+@pytest.mark.parametrize('section,config,valid_key,malformed_count', [
+    (PlaylistsSection(), dict(playlists=[*MALFORMED, dict(name=['a', 'list']), dict(name='good')]), 'good', 5),
+    (ReposSection(), dict(repos=[*MALFORMED, dict(url=['a', 'list']), dict(url='https://example.com/a/b.git')]),
+     'example.com/a/b', 5),
+    (DomainsSection(), dict(collections=[*MALFORMED, dict(name=['a', 'list']), dict(name='example.com')]),
+     'example.com', 5),
+    (ChannelsSection(), dict(channels=[*MALFORMED, dict(name='c', directory=['a', 'list']),
+                                       dict(name='good', directory='videos/good')]), 'videos/good', 5),
+    (DownloadsSection(), dict(downloads=[*MALFORMED, dict(url=['a', 'list'], downloader='archive'),
+                                         dict(url='https://example.com/good', downloader='archive')]),
+     'https://example.com/good', 5),
+    (TagFilesSection(), dict(tags=dict(one=dict()), tag_files=[*MALFORMED, [['a', 'list'], 'foo.txt'],
+                                                               ['one', 'good.txt']]), ('one', 'good.txt'), 5),
+    (TagZimsSection(), dict(tags=dict(one=dict()), tag_zims=[*MALFORMED, [['a', 'list'], 'a.zim', 'A'],
+                                                             ['one', 'good.zim', 'A']]), ('one', 'good.zim', 'A'), 5),
+])
+def test_malformed_entries(test_directory, section, config, valid_key, malformed_count):
+    assert_skipped(section, config, valid_key, malformed_count)
+
+
+def test_malformed_tags(test_directory):
+    """A Tag whose attributes are not a mapping is skipped and counted."""
+    assert_skipped(TagsSection(), dict(tags={'one': dict(color='#fff'), 'two': 'a string', 'three': 5}), 'one', 2)
+
+
+def test_malformed_playlist_items(test_directory):
+    """A playlist's malformed items are skipped and counted; so is an `items` that is not a list."""
+    config = dict(playlists=[
+        dict(name='good', items=[*MALFORMED, dict(url=['a', 'list']), dict(url='https://example.com/good')]),
+        dict(name='bad', items=5),
+    ])
+    rows, invalid = PlaylistItemsSection().file_rows(config)
+    assert list(rows) == [('good', 'url', 'https://example.com/good', None)]
+    assert len(invalid) == 6
+
+
+def test_malformed_channel_downloads(test_directory):
+    """A Channel whose `downloads` is not a list keeps its Downloads (counted)."""
+    config = dict(channels=[dict(name='good', directory='videos/good', downloads=5)])
+    rows, invalid = ChannelDownloadsSection().file_rows(config)
+    assert rows == dict()
+    assert invalid
+
+
+@pytest.mark.parametrize('section,config', [
+    (PlaylistsSection(), dict(playlists=MALFORMED)),
+    (PlaylistItemsSection(), dict(playlists=MALFORMED)),
+    (ReposSection(), dict(repos=MALFORMED)),
+    (DomainsSection(), dict(collections=[*MALFORMED, dict(name=['a', 'list'])])),
+    (ChannelsSection(), dict(channels=MALFORMED)),
+    (DownloadsSection(), dict(downloads=MALFORMED)),
+    (TagsSection(), dict(tags=dict())),
+    # A list of two texts is a tagged file's line (of an unknown Tag), not an unreadable one.
+    (TagFilesSection(), dict(tags=dict(one=dict()), tag_files=['a string', 5, None, [['a'], 'b.txt']])),
+    (TagZimsSection(), dict(tags=dict(one=dict()), tag_zims=MALFORMED)),
+])
+def test_only_malformed_entries_remove_nothing(test_directory, section, config):
+    """A section whose entries are all unreadable is not present: like an empty section, it removes nothing (it
+    cannot be told apart from a damaged file)."""
+    rows, invalid = section.file_rows(config)
+    assert not rows
+    assert not section.present(config, rows, invalid)
+
+
+def test_diff_unreadable():
+    """An unreadable entry keeps no row and does not make the section present; a named invalid row does both."""
+    db_rows = dict(one=dict(), two=dict())
+    assert diff_section(dict(), {Unreadable(0)}, db_rows, deletes=True).removed == set()
+    assert diff_section(dict(), {Unreadable(0), 'one'}, db_rows, deletes=True).removed == {'two'}
+
+
+def test_malformed_channel_download_url(test_directory):
+    """A Channel's Download whose url is not text is counted, so the Channel keeps its Downloads."""
+    config = dict(channels=[dict(name='good', directory='videos/good', downloads=[dict(url=['a', 'list']), dict(url=5)])])
+    rows, invalid = ChannelDownloadsSection().file_rows(config)
+    assert rows == dict()
+    assert {i[0] for i in invalid} == {'videos/good'}
+    assert len(invalid) == 2