Rev 5989: (vila) Clearer exceptions for bad-content config files (Vincent Ladeuil) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Mon Jun 20 16:01:51 UTC 2011
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 5989 [merge]
revision-id: pqm at pqm.ubuntu.com-20110620160147-19cp6oo7bt8a92jo
parent: pqm at pqm.ubuntu.com-20110620104625-ih5t1e8w288pc4l2
parent: v.ladeuil+lp at free.fr-20110620140340-mtij9ioyc58ot3is
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Mon 2011-06-20 16:01:47 +0000
message:
(vila) Clearer exceptions for bad-content config files (Vincent Ladeuil)
modified:
bzrlib/config.py config.py-20051011043216-070c74f4e9e338e8
bzrlib/errors.py errors.py-20050309040759-20512168c4e14fbd
bzrlib/tests/test_config.py testconfig.py-20051011041908-742d0c15d8d8c8eb
doc/en/release-notes/bzr-2.4.txt bzr2.4.txt-20110114053217-k7ym9jfz243fddjm-1
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py 2011-06-20 09:31:17 +0000
+++ b/bzrlib/config.py 2011-06-20 14:03:40 +0000
@@ -692,6 +692,8 @@
self._parser = ConfigObj(co_input, encoding='utf-8')
except configobj.ConfigObjError, e:
raise errors.ParseConfigError(e.errors, e.config.filename)
+ except UnicodeDecodeError:
+ raise errors.ConfigContentError(self.file_name)
# Make sure self.reload() will use the right file name
self._parser.filename = self.file_name
for hook in OldConfigHooks['load']:
@@ -1697,6 +1699,8 @@
self._config = ConfigObj(self._input, encoding='utf-8')
except configobj.ConfigObjError, e:
raise errors.ParseConfigError(e.errors, e.config.filename)
+ except UnicodeError:
+ raise errors.ConfigContentError(self._filename)
return self._config
def _save(self):
@@ -2178,12 +2182,21 @@
except errors.NoSuchFile:
return StringIO()
+ def _external_url(self):
+ return urlutils.join(self._transport.external_url(), self._filename)
+
def _get_configobj(self):
f = self._get_config_file()
try:
- return ConfigObj(f, encoding='utf-8')
+ try:
+ conf = ConfigObj(f, encoding='utf-8')
+ except configobj.ConfigObjError, e:
+ raise errors.ParseConfigError(e.errors, self._external_url())
+ except UnicodeDecodeError:
+ raise errors.ConfigContentError(self._external_url())
finally:
f.close()
+ return conf
def _set_configobj(self, configobj):
out_file = StringIO()
@@ -2285,14 +2298,10 @@
"""Loads the Store from persistent storage."""
raise NotImplementedError(self.load)
- def _load_from_string(self, str_or_unicode):
+ def _load_from_string(self, bytes):
"""Create a store from a string in configobj syntax.
- :param str_or_unicode: A string representing the file content. This will
- be encoded to suit store needs internally.
-
- This is for tests and should not be used in production unless a
- convincing use case can be demonstrated :)
+ :param bytes: A string representing the file content.
"""
raise NotImplementedError(self._load_from_string)
@@ -2370,21 +2379,22 @@
for hook in ConfigHooks['load']:
hook(self)
- def _load_from_string(self, str_or_unicode):
+ def _load_from_string(self, bytes):
"""Create a config store from a string.
- :param str_or_unicode: A string representing the file content. This will
- be utf-8 encoded internally.
+ :param bytes: A string representing the file content.
"""
if self.is_loaded():
raise AssertionError('Already loaded: %r' % (self._config_obj,))
- co_input = StringIO(str_or_unicode)
+ co_input = StringIO(bytes)
try:
# The config files are always stored utf8-encoded
self._config_obj = ConfigObj(co_input, encoding='utf-8')
except configobj.ConfigObjError, e:
self._config_obj = None
raise errors.ParseConfigError(e.errors, self.external_url())
+ except UnicodeDecodeError:
+ raise errors.ConfigContentError(self.external_url())
def save(self):
if not self.is_loaded():
=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py 2011-06-02 16:57:09 +0000
+++ b/bzrlib/errors.py 2011-06-20 13:58:14 +0000
@@ -1770,6 +1770,15 @@
_fmt = "Working tree has conflicts."
+class ConfigContentError(BzrError):
+
+ _fmt = "Config file %(filename)s is not UTF-8 encoded\n"
+
+ def __init__(self, filename):
+ BzrError.__init__(self)
+ self.filename = filename
+
+
class ParseConfigError(BzrError):
_fmt = "Error(s) parsing config file %(filename)s:\n%(errors)s"
=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py 2011-06-20 09:31:17 +0000
+++ b/bzrlib/tests/test_config.py 2011-06-20 13:58:54 +0000
@@ -1871,9 +1871,34 @@
class TestTransportConfig(tests.TestCaseWithTransport):
+ def test_load_utf8(self):
+ """Ensure we can load an utf8-encoded file."""
+ t = self.get_transport()
+ unicode_user = u'b\N{Euro Sign}ar'
+ unicode_content = u'user=%s' % (unicode_user,)
+ utf8_content = unicode_content.encode('utf8')
+ # Store the raw content in the config file
+ t.put_bytes('foo.conf', utf8_content)
+ conf = config.TransportConfig(t, 'foo.conf')
+ self.assertEquals(unicode_user, conf.get_option('user'))
+
+ def test_load_non_ascii(self):
+ """Ensure we display a proper error on non-ascii, non utf-8 content."""
+ t = self.get_transport()
+ t.put_bytes('foo.conf', 'user=foo\n#\xff\n')
+ conf = config.TransportConfig(t, 'foo.conf')
+ self.assertRaises(errors.ConfigContentError, conf._get_configobj)
+
+ def test_load_erroneous_content(self):
+ """Ensure we display a proper error on content that can't be parsed."""
+ t = self.get_transport()
+ t.put_bytes('foo.conf', '[open_section\n')
+ conf = config.TransportConfig(t, 'foo.conf')
+ self.assertRaises(errors.ParseConfigError, conf._get_configobj)
+
def test_get_value(self):
"""Test that retreiving a value from a section is possible"""
- bzrdir_config = config.TransportConfig(transport.get_transport('.'),
+ bzrdir_config = config.TransportConfig(self.get_transport('.'),
'control.conf')
bzrdir_config.set_option('value', 'key', 'SECTION')
bzrdir_config.set_option('value2', 'key2')
@@ -2328,12 +2353,22 @@
self.assertRaises(AssertionError, store._load_from_string, 'bar=baz')
-class TestBug799212(TestStore):
-
- def test_load_utf8(self):
+class TestIniFileStoreContent(tests.TestCaseWithTransport):
+ """Simulate loading a config store without content of various encodings.
+
+ All files produced by bzr are in utf8 content.
+
+ Users may modify them manually and end up with a file that can't be
+ loaded. We need to issue proper error messages in this case.
+ """
+
+ invalid_utf8_char = '\xff'
+
+ def test_load_utf8(self):
+ """Ensure we can load an utf8-encoded file."""
t = self.get_transport()
# From http://pad.lv/799212
- unicode_user = u'Piotr O\u017carowski'
+ unicode_user = u'b\N{Euro Sign}ar'
unicode_content = u'user=%s' % (unicode_user,)
utf8_content = unicode_content.encode('utf8')
# Store the raw content in the config file
@@ -2343,6 +2378,58 @@
stack = config.Stack([store.get_sections], store)
self.assertEquals(unicode_user, stack.get('user'))
+ def test_load_non_ascii(self):
+ """Ensure we display a proper error on non-ascii, non utf-8 content."""
+ t = self.get_transport()
+ t.put_bytes('foo.conf', 'user=foo\n#%s\n' % (self.invalid_utf8_char,))
+ store = config.IniFileStore(t, 'foo.conf')
+ self.assertRaises(errors.ConfigContentError, store.load)
+
+ def test_load_erroneous_content(self):
+ """Ensure we display a proper error on content that can't be parsed."""
+ t = self.get_transport()
+ t.put_bytes('foo.conf', '[open_section\n')
+ store = config.IniFileStore(t, 'foo.conf')
+ self.assertRaises(errors.ParseConfigError, store.load)
+
+
+class TestIniConfigContent(tests.TestCaseWithTransport):
+ """Simulate loading a IniBasedConfig without content of various encodings.
+
+ All files produced by bzr are in utf8 content.
+
+ Users may modify them manually and end up with a file that can't be
+ loaded. We need to issue proper error messages in this case.
+ """
+
+ invalid_utf8_char = '\xff'
+
+ def test_load_utf8(self):
+ """Ensure we can load an utf8-encoded file."""
+ # From http://pad.lv/799212
+ unicode_user = u'b\N{Euro Sign}ar'
+ unicode_content = u'user=%s' % (unicode_user,)
+ utf8_content = unicode_content.encode('utf8')
+ # Store the raw content in the config file
+ with open('foo.conf', 'wb') as f:
+ f.write(utf8_content)
+ conf = config.IniBasedConfig(file_name='foo.conf')
+ self.assertEquals(unicode_user, conf.get_user_option('user'))
+
+ def test_load_badly_encoded_content(self):
+ """Ensure we display a proper error on non-ascii, non utf-8 content."""
+ with open('foo.conf', 'wb') as f:
+ f.write('user=foo\n#%s\n' % (self.invalid_utf8_char,))
+ conf = config.IniBasedConfig(file_name='foo.conf')
+ self.assertRaises(errors.ConfigContentError, conf._get_parser)
+
+ def test_load_erroneous_content(self):
+ """Ensure we display a proper error on content that can't be parsed."""
+ with open('foo.conf', 'wb') as f:
+ f.write('[open_section\n')
+ conf = config.IniBasedConfig(file_name='foo.conf')
+ self.assertRaises(errors.ParseConfigError, conf._get_parser)
+
class TestMutableStore(TestStore):
@@ -3052,6 +3139,11 @@
self.assertEquals({}, conf._get_config())
self._got_user_passwd(None, None, conf, 'http', 'foo.net')
+ def test_non_utf8_config(self):
+ conf = config.AuthenticationConfig(_file=StringIO(
+ 'foo = bar\xff'))
+ self.assertRaises(errors.ConfigContentError, conf._get_config)
+
def test_missing_auth_section_header(self):
conf = config.AuthenticationConfig(_file=StringIO('foo = bar'))
self.assertRaises(ValueError, conf.get_credentials, 'ftp', 'foo.net')
=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- a/doc/en/release-notes/bzr-2.4.txt 2011-06-20 09:31:17 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt 2011-06-20 13:58:54 +0000
@@ -29,6 +29,10 @@
Bug Fixes
*********
+* Display a proper error message when a config file content cannot be
+ decoded as UTF-8 or when it cannot be parsed.
+ (Vincent Ladeuil, #502060, #688677, #792246)
+
* Properly load utf8-encoded config files. (Vincent Ladeuil, #799212)
.. Fixes for situations where bzr would previously crash or give incorrect
More information about the bazaar-commits
mailing list