__str__ method should return byte string, not unicode string. But as we don't have a check in every KS_OBJECT.__str__ method, let's at least add try-except block to prevent UnicodeDecodeError tracebacks.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- pykickstart/base.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/pykickstart/base.py b/pykickstart/base.py index 4ef442c..9079906 100644 --- a/pykickstart/base.py +++ b/pykickstart/base.py @@ -281,12 +281,20 @@ class BaseHandler(KickstartObject):
for prio in lst: for obj in self._writeOrder[prio]: - retval += obj.__str__() + obj_str = str(obj) + try: + retval += obj_str + except UnicodeDecodeError: + retval += obj_str.encode("utf-8")
for script in self.scripts: - retval += script.__str__() + script_str = str(script) + try: + retval += script_str + except UnicodeDecodeError: + retval += script_str.encode("utf-8")
- retval += self.packages.__str__() + retval += str(self.packages)
return retval
On Fri, Nov 30, 2012 at 12:13:16PM +0100, Vratislav Podzimek wrote:
__str__ method should return byte string, not unicode string. But as we don't have a check in every KS_OBJECT.__str__ method, let's at least add try-except block to prevent UnicodeDecodeError tracebacks.
Looks good to me.
Samantha
__str__ method should return byte string, not unicode string. But as we don't have a check in every KS_OBJECT.__str__ method, let's at least add try-except block to prevent UnicodeDecodeError tracebacks.
I think this is okay, but this is the kind of change I would very much like to see some test cases for.
- Chris
On Tue, 2012-12-04 at 10:47 -0500, Chris Lumens wrote:
__str__ method should return byte string, not unicode string. But as we don't have a check in every KS_OBJECT.__str__ method, let's at least add try-except block to prevent UnicodeDecodeError tracebacks.
I think this is okay, but this is the kind of change I would very much like to see some test cases for.
Good idea, I'm sending another patch for that. I guess these two could be squashed before pushing.
On Wed, 2012-12-05 at 12:46 +0100, Vratislav Podzimek wrote:
On Tue, 2012-12-04 at 10:47 -0500, Chris Lumens wrote:
__str__ method should return byte string, not unicode string. But as we don't have a check in every KS_OBJECT.__str__ method, let's at least add try-except block to prevent UnicodeDecodeError tracebacks.
I think this is okay, but this is the kind of change I would very much like to see some test cases for.
Good idea, I'm sending another patch for that. I guess these two could be squashed before pushing.
I've improved the test a little bit and it showed me, that the original patch doesn't cover all common cases. I'll send a new version of the patch (test included) once I find out how to resolve the issue.
I've improved the test a little bit and it showed me, that the original patch doesn't cover all common cases. I'll send a new version of the patch (test included) once I find out how to resolve the issue.
If you want to post the new issue, perhaps I can take a look at it as well.
- Chris
__str__ method should return byte string, not unicode string. But as we don't have a ensure this in every KS_OBJECT.__str__ method, let's add encode("utf-8") call to the result if it is a unicode string.
Also add test for these issues.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- pykickstart/base.py | 10 +++++++-- tests/parser/handle_unicode.py | 49 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 tests/parser/handle_unicode.py
diff --git a/pykickstart/base.py b/pykickstart/base.py index 4ef442c..e12b86c 100644 --- a/pykickstart/base.py +++ b/pykickstart/base.py @@ -281,10 +281,16 @@ class BaseHandler(KickstartObject):
for prio in lst: for obj in self._writeOrder[prio]: - retval += obj.__str__() + obj_str = obj.__str__() + if type(obj_str) == types.UnicodeType: + obj_str = obj_str.encode("utf-8") + retval += obj_str
for script in self.scripts: - retval += script.__str__() + script_str = script.__str__() + if type(script_str) == types.UnicodeType: + script_str = script_str.encode("utf-8") + retval += script_str
retval += self.packages.__str__()
diff --git a/tests/parser/handle_unicode.py b/tests/parser/handle_unicode.py new file mode 100644 index 0000000..22ebd10 --- /dev/null +++ b/tests/parser/handle_unicode.py @@ -0,0 +1,49 @@ +# -*- coding: utf-8 -*- +import unittest +from tests.baseclass import * + +from pykickstart import constants +from pykickstart.errors import KickstartParseError +from pykickstart import version + +class HandleUnicode_TestCase(ParserTest): + ks = """ +rootpw ááááááááá + +%post +echo áááááá +%end +""" + + def runTest(self): + unicode_str1 = u"ááááááááá" + unicode_str2 = u"áááááá" + encoded_str1 = unicode_str1.encode("utf-8") + encoded_str2 = unicode_str2.encode("utf-8") + + # parser should parse string including non-ascii characters + self.parser.readKickstartFromString(self.ks) + + # str(handler) should not cause traceback and should contain the + # original non-ascii strings as utf-8 encoded byte strings and + # str(self.handler) should not fail -- i.e. self.handler.__str__() + # should return byte string not unicode string + self.assertIn(encoded_str1, str(self.handler)) + self.assertIn(encoded_str2, str(self.handler)) + + # set root password to unicode string + self.handler.rootpw.password = unicode_str1 + + # str(handler) should not cause traceback and should contain the + # original unicode string as utf-8 encoded byte string + self.assertIn(encoded_str1, str(self.handler)) + + # set root password to encoded string + self.handler.rootpw.password = encoded_str1 + + # str(handler) should not cause traceback and should contain the + # original unicode string as utf-8 encoded byte string + self.assertIn(encoded_str1, str(self.handler)) + +if __name__ == "__main__": + unittest.main()
__str__ method should return byte string, not unicode string. But as we don't have a ensure this in every KS_OBJECT.__str__ method, let's add encode("utf-8") call to the result if it is a unicode string.
Also add test for these issues.
Thanks for adding the test case. I think this should be fine.
This is for master, and not for F18 correct?
- Chris
On Mon, 2012-12-10 at 13:57 -0500, Chris Lumens wrote:
__str__ method should return byte string, not unicode string. But as we don't have a ensure this in every KS_OBJECT.__str__ method, let's add encode("utf-8") call to the result if it is a unicode string.
Also add test for these issues.
Thanks for adding the test case. I think this should be fine.
This is for master, and not for F18 correct?
Well, I'm not sure. It is neither a blocker nor NTH, but F18 installations may hit the bug and end up with a traceback (though leaving a completely installed system, I guess).
This is for master, and not for F18 correct?
Well, I'm not sure. It is neither a blocker nor NTH, but F18 installations may hit the bug and end up with a traceback (though leaving a completely installed system, I guess).
I'd prefer to stick with just master unless we get some indications there's a serious problem with F18.
- Chris
anaconda-patches@lists.fedorahosted.org
-
Chris Lumens -
Samantha N. Bueno -
Vratislav Podzimek