From dcad81a78f321027d1dcbea90c4d47cd253807c9 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 26 Apr 2016 23:14:55 +0200 Subject: [PATCH 1/5] cache: fix crash when cache_dir is None Issue #1412 When passing --cachedir="" on the command line, standarddir.cache() returns None, which stands for "deactivate cache" and has to be properly handled in DiskCache.__init__() (i.e. don't pass it to os.path.join) --- qutebrowser/browser/cache.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/qutebrowser/browser/cache.py b/qutebrowser/browser/cache.py index b34fa232b..5ef53837a 100644 --- a/qutebrowser/browser/cache.py +++ b/qutebrowser/browser/cache.py @@ -32,16 +32,23 @@ class DiskCache(QNetworkDiskCache): """Disk cache which sets correct cache dir and size. + If the cache is deactivated via the command line argument --cachedir="", + both attributes _cache_dir and _http_cache_dir are set to None. + Attributes: _activated: Whether the cache should be used. - _cache_dir: The base directory for cache files (standarddir.cache()) - _http_cache_dir: the HTTP subfolder in _cache_dir. + _cache_dir: The base directory for cache files (standarddir.cache()) or + None. + _http_cache_dir: the HTTP subfolder in _cache_dir or None. """ def __init__(self, cache_dir, parent=None): super().__init__(parent) self._cache_dir = cache_dir - self._http_cache_dir = os.path.join(cache_dir, 'http') + if cache_dir is None: + self._http_cache_dir = None + else: + self._http_cache_dir = os.path.join(cache_dir, 'http') self._maybe_activate() objreg.get('config').changed.connect(self.on_config_changed) From c0b372591ab7bba68856729419e3abaedf2a36c6 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 26 Apr 2016 23:26:35 +0200 Subject: [PATCH 2/5] tests: add tests for --cachedir="" --- tests/integration/test_invocations.py | 31 +++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/integration/test_invocations.py b/tests/integration/test_invocations.py index a13c6ac25..14b6fb506 100644 --- a/tests/integration/test_invocations.py +++ b/tests/integration/test_invocations.py @@ -53,6 +53,37 @@ def test_no_config(tmpdir, quteproc_new): quteproc_new.wait_for_quit() +@pytest.mark.linux +def test_no_cache(tmpdir, quteproc_new): + """Test starting with --cachedir="". + + We can't run --basedir or --temp-basedir to reproduce this, so we mess with + XDG_*_DIR to get things relocated. + """ + data_dir = tmpdir / 'data' + config_dir = tmpdir / 'config' + runtime_dir = tmpdir / 'runtime' + cache_dir = tmpdir / 'cache' + + runtime_dir.ensure(dir=True) + runtime_dir.chmod(0o700) + + (data_dir / 'qutebrowser' / 'state').write_text( + '[general]\nquickstart-done = 1', encoding='utf-8', ensure=True) + + env = { + 'XDG_DATA_HOME': str(data_dir), + 'XDG_CONFIG_HOME': str(config_dir), + 'XDG_RUNTIME_DIR': str(runtime_dir), + 'XDG_CACHE_HOME': str(cache_dir), + } + + args = ['--debug', '--no-err-windows', '--cachedir=""', 'about:blank'] + quteproc_new.start(args, env=env) + quteproc_new.send_cmd(':quit') + quteproc_new.wait_for_quit() + + @pytest.mark.linux def test_ascii_locale(httpbin, tmpdir, quteproc_new): """Test downloads with LC_ALL=C set. From 758fb9441482d769eca18bdeda0604f41cb98314 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 26 Apr 2016 23:43:55 +0200 Subject: [PATCH 3/5] tests: add test for DiskCache with cache_dir=None --- tests/unit/browser/test_cache.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/unit/browser/test_cache.py b/tests/unit/browser/test_cache.py index ae59604a3..cb570972b 100644 --- a/tests/unit/browser/test_cache.py +++ b/tests/unit/browser/test_cache.py @@ -113,6 +113,16 @@ def test_cache_size_deactivated(config_stub, tmpdir): assert disk_cache.cacheSize() == 0 +def test_cache_no_cache_dir(config_stub): + """Confirm that the cache is deactivated when cache_dir is None.""" + config_stub.data = { + 'storage': {'cache-size': 1024}, + 'general': {'private-browsing': False}, + } + disk_cache = cache.DiskCache(None) + assert disk_cache.cacheSize() == 0 + + def test_cache_existing_metadata_file(config_stub, tmpdir): """Test querying existing meta data file from activated cache.""" config_stub.data = { From 951cab237f45a04c166e789baee726a4f4cadfc2 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 26 Apr 2016 23:51:48 +0200 Subject: [PATCH 4/5] tests: factor out common code in test_invocations --- tests/integration/test_invocations.py | 43 ++++++++------------------- 1 file changed, 13 insertions(+), 30 deletions(-) diff --git a/tests/integration/test_invocations.py b/tests/integration/test_invocations.py index 14b6fb506..72fe55b90 100644 --- a/tests/integration/test_invocations.py +++ b/tests/integration/test_invocations.py @@ -22,11 +22,11 @@ import pytest -@pytest.mark.linux -def test_no_config(tmpdir, quteproc_new): - """Test starting with -c "". +@pytest.fixture +def temp_basedir_env(tmpdir): + """Return a dict of environment variables that fakes --temp-basedir. - We can't run --basedir or --temp-basedir to reproduce this, so we mess with + We can't run --basedir or --temp-basedir for some tests, so we mess with XDG_*_DIR to get things relocated. """ data_dir = tmpdir / 'data' @@ -46,40 +46,23 @@ def test_no_config(tmpdir, quteproc_new): 'XDG_RUNTIME_DIR': str(runtime_dir), 'XDG_CACHE_HOME': str(cache_dir), } + return env + +@pytest.mark.linux +def test_no_config(temp_basedir_env, quteproc_new): + """Test starting with -c "".""" args = ['--debug', '--no-err-windows', '-c', '', 'about:blank'] - quteproc_new.start(args, env=env) + quteproc_new.start(args, env=temp_basedir_env) quteproc_new.send_cmd(':quit') quteproc_new.wait_for_quit() @pytest.mark.linux -def test_no_cache(tmpdir, quteproc_new): - """Test starting with --cachedir="". - - We can't run --basedir or --temp-basedir to reproduce this, so we mess with - XDG_*_DIR to get things relocated. - """ - data_dir = tmpdir / 'data' - config_dir = tmpdir / 'config' - runtime_dir = tmpdir / 'runtime' - cache_dir = tmpdir / 'cache' - - runtime_dir.ensure(dir=True) - runtime_dir.chmod(0o700) - - (data_dir / 'qutebrowser' / 'state').write_text( - '[general]\nquickstart-done = 1', encoding='utf-8', ensure=True) - - env = { - 'XDG_DATA_HOME': str(data_dir), - 'XDG_CONFIG_HOME': str(config_dir), - 'XDG_RUNTIME_DIR': str(runtime_dir), - 'XDG_CACHE_HOME': str(cache_dir), - } - +def test_no_cache(temp_basedir_env, quteproc_new): + """Test starting with --cachedir="".""" args = ['--debug', '--no-err-windows', '--cachedir=""', 'about:blank'] - quteproc_new.start(args, env=env) + quteproc_new.start(args, env=temp_basedir_env) quteproc_new.send_cmd(':quit') quteproc_new.wait_for_quit() From e8aa242d10e200164ee23887e1bffc74455d9dfb Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Wed, 27 Apr 2016 13:02:18 +0200 Subject: [PATCH 5/5] tests: fix invocation test for --cachedir --cachedir="" doesn't work because the quotes are not processed (as they would be by the shell) and the cachedir is set to ./"" (that is a directory with two double quotes as name). The correct start parameter is thus --cachedir=, which correctly fails when the fix is reverted. --- tests/integration/test_invocations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_invocations.py b/tests/integration/test_invocations.py index 72fe55b90..b987d9416 100644 --- a/tests/integration/test_invocations.py +++ b/tests/integration/test_invocations.py @@ -61,7 +61,7 @@ def test_no_config(temp_basedir_env, quteproc_new): @pytest.mark.linux def test_no_cache(temp_basedir_env, quteproc_new): """Test starting with --cachedir="".""" - args = ['--debug', '--no-err-windows', '--cachedir=""', 'about:blank'] + args = ['--debug', '--no-err-windows', '--cachedir=', 'about:blank'] quteproc_new.start(args, env=temp_basedir_env) quteproc_new.send_cmd(':quit') quteproc_new.wait_for_quit()