Make make_reader(feed_root=...) default to None.

For #183.

CHANGES.rst

@@ -54,6 +54,10 @@ Unreleased
   :meth:`~Reader.update_feeds()` and :meth:`~Reader.update_feeds_iter()`
   are now keyword-only. (:issue:`183`)
 
+* The ``feed_root`` argument of :func:`make_reader`
+  now defaults to ``None`` (don't open local feeds)
+  instead of ``''`` (full filesystem access).
+
 
 Version 1.19
 ------------

docs/guide.rst

@@ -60,10 +60,12 @@ File-system access
 
 *reader* supports *http(s)://* and local (*file:*) feeds.
 
-For security reasons, you might want to restrict file-system access
-to a single directory or prevent it entirely;
-you can do so by using the ``feed_root`` :func:`make_reader` argument::
+For security reasons, local feeds are disabled by default.
+You can allow full file-system access or restrict it to a single directory
+by using the ``feed_root`` :func:`make_reader` argument::
 
+    >>> # all local feed paths allowed
+    >>> reader = make_reader("db.sqlite", feed_root='')
     >>> # local feed paths are relative to /feeds
     >>> reader = make_reader("db.sqlite", feed_root='/feeds')
     >>> # ok, resolves to /feeds/feed.xml
@@ -72,8 +74,6 @@ you can do so by using the ``feed_root`` :func:`make_reader` argument::
     >>> reader.add_feed("file:also/feed.xml")
     >>> # error on update, resolves to /feed.xml, which is above /feeds
     >>> reader.add_feed("file:../feed.xml")
-    >>> # all local paths will fail to update
-    >>> reader = make_reader("db.sqlite", feed_root=None)
 
 Note that it is still possible to `add <Adding feeds_>`_ local feeds
 regardless of ``feed_root``;

src/reader/_cli.py

@@ -201,14 +201,24 @@ def wrapper(*args, **kwargs):
     default=get_default_config_path(),
     show_default=True,
 )
+@click.option(
+    '--feed-root',
+    type=click.Path(file_okay=False),
+    show_default=True,
+    help=(
+        "Directory local feeds are relative to. "
+        "'' (empty string) means full filesystem access. "
+        "If not provided, don't open local feeds."
+    ),
+)
 @click.option(
     '--debug-storage/--no-debug-storage',
     hidden=True,
     help="NOT TESTED. With -vv, log storage database calls.",
 )
 @click.version_option(reader.__version__, message='%(prog)s %(version)s')
 @click.pass_obj
-def cli(config, db, plugin, debug_storage):
+def cli(config, db, plugin, feed_root, debug_storage):
     # TODO: mention in docs that --db/--plugin/envvars ALWAYS override the config
     # (same for wsgi envvars)
     # NOTE: we can never use click defaults for --db/--plugin, because they would override the config always
@@ -227,6 +237,9 @@ def cli(config, db, plugin, debug_storage):
     if plugin:
         config.all['reader']['plugins'] = dict.fromkeys(plugin)
 
+    if feed_root is not None:
+        config['default']['reader']['feed_root'] = feed_root
+
     # until we make debug_storage a proper make_reader argument,
     # and we get rid of make_reader_with_plugins
     config['default']['reader']['debug_storage'] = debug_storage

src/reader/_parser.py

@@ -75,6 +75,7 @@ def __call__(
         ...
 
 
+# TODO: use Collection instead of Iterable
 FeedAndEntries = Tuple[FeedData, Iterable[EntryData[Optional[datetime]]]]
 
 

src/reader/core.py

@@ -78,7 +78,7 @@
 def make_reader(
     url: str,
     *,
-    feed_root: Optional[str] = '',
+    feed_root: Optional[str] = None,
     plugins: Iterable[Union[str, ReaderPluginType]] = _DEFAULT_PLUGINS,
     session_timeout: TimeoutType = SESSION_TIMEOUT,
     reserved_name_scheme: Mapping[str, str] = DEFAULT_RESERVED_NAME_SCHEME,
@@ -137,9 +137,10 @@ def make_reader(
 
         feed_root (str or None):
             Directory where to look for local feeds.
-            One of ``None`` (don't open local feeds),
-            ``''`` (full filesystem access; default), or
-            ``'/path/to/feed/root'`` (an absolute path that feed paths are relative to).
+            One of ``None`` (don't open local feeds; default),
+            ``''`` (full filesystem access), or
+            ``'/path/to/feed/root'``
+            (an absolute path that feed paths are relative to).
 
         plugins (iterable(str or callable(Reader)) or None):
             An iterable of built-in plugin names or
@@ -172,11 +173,6 @@ def make_reader(
     .. versionadded:: 1.6
         The ``feed_root`` keyword argument.
 
-    .. versionchanged:: 2.0
-        The default ``feed_root`` behavior will change from
-        *full filesystem access* (``''``) to
-        *don't open local feeds* (``None``).
-
     .. versionadded:: 1.14
         The ``session_timeout`` keyword argument,
         with a default of (3.05, 60) seconds;
@@ -189,6 +185,10 @@ def make_reader(
     .. versionadded:: 1.17
         The ``reserved_name_scheme`` argument.
 
+    .. versionchanged:: 2.0
+        ``feed_root`` now defaults to ``None`` (don't open local feeds)
+        instead of ``''`` (full filesystem access).
+
     """
 
     # If we ever need to change the signature of make_reader(),

tests/conftest.py

@@ -60,7 +60,7 @@ def make_reader(*args, **kwargs):
 
 @pytest.fixture
 def reader():
-    with closing(original_make_reader(':memory:')) as reader:
+    with closing(original_make_reader(':memory:', feed_root='')) as reader:
         yield reader
 
 

tests/test_cli.py

@@ -51,86 +51,89 @@ def test_cli(db_path, data_dir):
 
     runner = CliRunner()
 
-    result = runner.invoke(cli, ['--db', db_path, 'list', 'feeds'])
+    def invoke(*args):
+        return runner.invoke(cli, ('--db', db_path, '--feed-root', '') + args)
+
+    result = invoke('list', 'feeds')
     assert result.exit_code == 0
     assert result.output == ''
 
-    result = runner.invoke(cli, ['--db', db_path, 'add', feed_path])
+    result = invoke('add', feed_path)
     assert result.exit_code == 0
     assert result.output == ''
 
-    result = runner.invoke(cli, ['--db', db_path, 'list', 'feeds'])
+    result = invoke('list', 'feeds')
     assert result.exit_code == 0
     assert result.output.splitlines() == [feed_path]
 
-    result = runner.invoke(cli, ['--db', db_path, 'list', 'entries'])
+    result = invoke('list', 'entries')
     assert result.exit_code == 0
     assert result.output == ''
 
-    result = runner.invoke(cli, ['--db', db_path, 'update'])
+    result = invoke('update')
     assert result.exit_code == 0
     assert "1 ok, 0 error, 0 not modified; entries: 2 new, 0 modified" in result.output
 
-    result = runner.invoke(cli, ['--db', db_path, 'list', 'feeds'])
+    result = invoke('list', 'feeds')
     assert result.exit_code == 0
     assert result.output.splitlines() == [feed_path]
 
-    result = runner.invoke(cli, ['--db', db_path, 'list', 'entries'])
+    result = invoke('list', 'entries')
     assert result.exit_code == 0
     assert [l.split() for l in result.output.splitlines()] == [
         [feed_path, e.link or e.id]
         for e in sorted(expected['entries'], key=lambda e: e.updated, reverse=True)
     ]
 
-    result = runner.invoke(cli, ['--db', db_path, 'search', 'status'])
+    result = invoke('search', 'status')
     assert result.exit_code == 0
     assert 'search: disabled' in result.output
 
-    result = runner.invoke(cli, ['--db', db_path, 'search', 'update'])
+    result = invoke('search', 'update')
     assert result.exit_code != 0
 
-    result = runner.invoke(cli, ['--db', db_path, 'search', 'entries', 'amok'])
+    result = invoke('search', 'entries', 'amok')
     assert result.exit_code != 0
 
-    result = runner.invoke(cli, ['--db', db_path, 'search', 'enable'])
+    result = invoke('search', 'enable')
     assert result.exit_code == 0
     assert result.output == ''
 
-    result = runner.invoke(cli, ['--db', db_path, 'search', 'status'])
+    result = invoke('search', 'status')
     assert result.exit_code == 0
     assert 'search: enabled' in result.output
 
-    result = runner.invoke(cli, ['--db', db_path, 'search', 'entries', 'amok'])
+    result = invoke('search', 'entries', 'amok')
     assert result.exit_code == 0
     assert result.output == ''
 
-    result = runner.invoke(cli, ['--db', db_path, 'search', 'update'])
+    result = invoke('search', 'update')
     assert result.exit_code == 0
     assert result.output == ''
 
-    result = runner.invoke(cli, ['--db', db_path, 'search', 'entries', 'amok'])
+    result = invoke('search', 'entries', 'amok')
     assert result.exit_code == 0
     assert {tuple(l.split()) for l in result.output.splitlines()} == {
         (feed_path, e.link or e.id) for e in expected['entries']
     }
 
-    result = runner.invoke(cli, ['--db', db_path, 'search', 'entries', 'again'])
+    result = invoke('search', 'entries', 'again')
     assert result.exit_code == 0
     assert {tuple(l.split()) for l in result.output.splitlines()} == {
         (feed_path, e.link or e.id)
         for e in expected['entries']
         if 'again' in e.title.lower()
     }
 
-    result = runner.invoke(cli, ['--db', db_path, 'search', 'entries', 'nope'])
+    result = invoke('search', 'entries', 'nope')
     assert result.exit_code == 0
     assert {tuple(l.split()) for l in result.output.splitlines()} == set()
 
-    result = runner.invoke(cli, ['--db', db_path, 'search', 'disable'])
+    result = invoke('search', 'disable')
     assert result.exit_code == 0
     assert result.output == ''
 
-    result = runner.invoke(cli, ['--db', db_path, 'search', 'status'])
+    result = invoke('search', 'status')
     assert result.exit_code == 0
     assert 'search: disabled' in result.output
 

tests/test_reader.py

@@ -1888,7 +1888,7 @@ def test_entries_filtering_error(reader, pre_stuff, call_method, kwargs):
 @pytest.mark.parametrize(
     'kwargs, feed_root',
     [
-        (dict(), ''),
+        (dict(), None),
         (dict(feed_root=None), None),
         (dict(feed_root=''), ''),
         (dict(feed_root='/path'), '/path'),