From fb5fbd09da40bbdf41452cb5d10657204c4b9397 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 19 Mar 2015 12:39:56 +0100 Subject: [PATCH] Handle unencodable file paths in config types. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If an user e.g. has a download-directory of ~/föö, but has LC_ALL=C set, we'll get an UnicodeEncodeError when trying to validate it. This is now handled properly by raising a ValidationError. Fixes #562. --- qutebrowser/config/configtypes.py | 53 +++++++++++++-------- qutebrowser/test/config/test_configtypes.py | 22 +++++++++ qutebrowser/test/helpers.py | 7 +++ 3 files changed, 61 insertions(+), 21 deletions(-) diff --git a/qutebrowser/config/configtypes.py b/qutebrowser/config/configtypes.py index bd4ecdf94..19b3854b0 100644 --- a/qutebrowser/config/configtypes.py +++ b/qutebrowser/config/configtypes.py @@ -833,10 +833,14 @@ class File(BaseType): else: raise configexc.ValidationError(value, "may not be empty!") value = os.path.expanduser(value) - if not os.path.isfile(value): - raise configexc.ValidationError(value, "must be a valid file!") - if not os.path.isabs(value): - raise configexc.ValidationError(value, "must be an absolute path!") + try: + if not os.path.isfile(value): + raise configexc.ValidationError(value, "must be a valid file!") + if not os.path.isabs(value): + raise configexc.ValidationError( + value, "must be an absolute path!") + except UnicodeEncodeError as e: + raise configexc.ValidationError(value, e) def transform(self, value): if not value: @@ -858,11 +862,15 @@ class Directory(BaseType): raise configexc.ValidationError(value, "may not be empty!") value = os.path.expandvars(value) value = os.path.expanduser(value) - if not os.path.isdir(value): - raise configexc.ValidationError(value, "must be a valid " - "directory!") - if not os.path.isabs(value): - raise configexc.ValidationError(value, "must be an absolute path!") + try: + if not os.path.isdir(value): + raise configexc.ValidationError( + value, "must be a valid directory!") + if not os.path.isabs(value): + raise configexc.ValidationError( + value, "must be an absolute path!") + except UnicodeEncodeError as e: + raise configexc.ValidationError(value, e) def transform(self, value): if not value: @@ -1179,18 +1187,21 @@ class UserStyleSheet(File): raise configexc.ValidationError(value, "may not be empty!") value = os.path.expandvars(value) value = os.path.expanduser(value) - if not os.path.isabs(value): - # probably a CSS, so we don't handle it as filename. - # FIXME We just try if it is encodable, maybe we should validate - # CSS? - # https://github.com/The-Compiler/qutebrowser/issues/115 - try: - value.encode('utf-8') - except UnicodeEncodeError as e: - raise configexc.ValidationError(value, str(e)) - return - elif not os.path.isfile(value): - raise configexc.ValidationError(value, "must be a valid file!") + try: + if not os.path.isabs(value): + # probably a CSS, so we don't handle it as filename. + # FIXME We just try if it is encodable, maybe we should + # validate CSS? + # https://github.com/The-Compiler/qutebrowser/issues/115 + try: + value.encode('utf-8') + except UnicodeEncodeError as e: + raise configexc.ValidationError(value, str(e)) + return + elif not os.path.isfile(value): + raise configexc.ValidationError(value, "must be a valid file!") + except UnicodeEncodeError as e: + raise configexc.ValidationError(value, e) def transform(self, value): path = os.path.expandvars(value) diff --git a/qutebrowser/test/config/test_configtypes.py b/qutebrowser/test/config/test_configtypes.py index 633dc0e32..a922e6fde 100644 --- a/qutebrowser/test/config/test_configtypes.py +++ b/qutebrowser/test/config/test_configtypes.py @@ -1374,6 +1374,13 @@ class FileTests(unittest.TestCase): self.t.validate('~/foobar') os_path.expanduser.assert_called_once_with('~/foobar') + def test_validate_invalid_encoding(self, os_path): + """Test validate with an invalid encoding, e.g. LC_ALL=C.""" + os_path.isfile.side_effect = helpers.unicode_encode_err + os_path.isabs.side_effect = helpers.unicode_encode_err + with self.assertRaises(configexc.ValidationError): + self.t.validate('foobar') + def test_transform(self, os_path): """Test transform.""" os_path.expanduser.side_effect = lambda x: x.replace('~', '/home/foo') @@ -1445,6 +1452,13 @@ class DirectoryTests(unittest.TestCase): self.t.validate('$BAR/foobar') os_path.expandvars.assert_called_once_with('$BAR/foobar') + def test_validate_invalid_encoding(self, os_path): + """Test validate with an invalid encoding, e.g. LC_ALL=C.""" + os_path.isdir.side_effect = helpers.unicode_encode_err + os_path.isabs.side_effect = helpers.unicode_encode_err + with self.assertRaises(configexc.ValidationError): + self.t.validate('foobar') + def test_transform(self, os_path): """Test transform.""" os_path.expandvars.side_effect = lambda x: x @@ -1879,6 +1893,14 @@ class UserStyleSheetTests(unittest.TestCase): def setUp(self): self.t = configtypes.UserStyleSheet() + @mock.patch('qutebrowser.config.configtypes.os.path', autospec=True) + def test_validate_invalid_encoding(self, os_path): + """Test validate with an invalid encoding, e.g. LC_ALL=C.""" + os_path.isfile.side_effect = helpers.unicode_encode_err + os_path.isabs.side_effect = helpers.unicode_encode_err + with self.assertRaises(configexc.ValidationError): + self.t.validate('foobar') + def test_transform_empty(self): """Test transform with an empty value.""" self.assertIsNone(self.t.transform('')) diff --git a/qutebrowser/test/helpers.py b/qutebrowser/test/helpers.py index 873312ab0..23ef2b25e 100644 --- a/qutebrowser/test/helpers.py +++ b/qutebrowser/test/helpers.py @@ -29,6 +29,13 @@ from PyQt5.QtWebKitWidgets import QWebPage from PyQt5.QtNetwork import QNetworkAccessManager +unicode_encode_err = UnicodeEncodeError('ascii', # codec + '', # object + 0, # start + 2, # end + 'fake exception') # reason + + @contextlib.contextmanager def environ_set_temp(env): """Set temporary environment variables.