From 024386d1892bacf3e3de5526077b4a072cea4a4b Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sat, 8 Apr 2017 16:20:55 -0400 Subject: [PATCH] Fail on history file parsing errors. Instead of skipping bad history lines during the import to sql, fail hard. We don't want to delete the user's old history file if we couldn't parse all of the lines. --- qutebrowser/browser/history.py | 8 +++++-- tests/unit/browser/webkit/test_history.py | 26 ++++++++++++++++------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/qutebrowser/browser/history.py b/qutebrowser/browser/history.py index 6cc98270c..52d32c114 100644 --- a/qutebrowser/browser/history.py +++ b/qutebrowser/browser/history.py @@ -183,12 +183,16 @@ class WebHistory(sql.SqlTable): """Import a text file into the sql database.""" with open(path, 'r', encoding='utf-8') as f: rows = [] - for line in f: + for (i, line) in enumerate(f): + line = line.strip() + if not line: + continue try: row = self._parse_entry(line.strip()) rows.append(row) except ValueError: - log.init.warning('Skipping history line {}'.format(line)) + raise Exception('Failed to parse line #{} of {}: "{}"' + .format(i, path, line)) self.insert_batch(rows) diff --git a/tests/unit/browser/webkit/test_history.py b/tests/unit/browser/webkit/test_history.py index 22db1490a..b2642f7c8 100644 --- a/tests/unit/browser/webkit/test_history.py +++ b/tests/unit/browser/webkit/test_history.py @@ -196,20 +196,16 @@ def test_init(backend, qapp, tmpdir, monkeypatch, cleanup_init): assert default_interface is None -def test_read(hist, tmpdir, caplog): +def test_read(hist, tmpdir): histfile = tmpdir / 'history' + # empty line is deliberate, to test skipping empty lines histfile.write('''12345 http://example.com/ title 12346 http://qutebrowser.org/ 67890 http://example.com/path - xyz http://example.com/bad-timestamp - 12345 - http://example.com/no-timestamp - 68891-r http://example.com/path/other - 68891-r-r http://example.com/double-flag''') + 68891-r http://example.com/path/other ''') - with caplog.at_level(logging.WARNING): - hist.read(str(histfile)) + hist.read(str(histfile)) assert list(hist) == [ ('http://example.com/', 'title', 12345, False), @@ -217,3 +213,17 @@ def test_read(hist, tmpdir, caplog): ('http://example.com/path', '', 67890, False), ('http://example.com/path/other', '', 68891, True) ] + + +@pytest.mark.parametrize('line', [ + 'xyz http://example.com/bad-timestamp', + '12345', + 'http://example.com/no-timestamp', + '68891-r-r http://example.com/double-flag', +]) +def test_read_invalid(hist, tmpdir, line): + histfile = tmpdir / 'history' + histfile.write(line) + + with pytest.raises(Exception): + hist.read(str(histfile))