Change in vdsm[master]: logging: Don't crash on non-ASCII in SimpleLogAdapter

mzamazal at redhat.com mzamazal at redhat.com
Fri Nov 13 16:25:18 UTC 2015


Milan Zamazal has posted comments on this change.

Change subject: logging: Don't crash on non-ASCII in SimpleLogAdapter
......................................................................


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/48542/1/tests/utilsTests.py
File tests/utilsTests.py:

Line 1084:         # various codings.
Line 1085:         invalid = chr(254) + chr(0)
Line 1086:         u_invalid = unichr(253) + unichr(254)
Line 1087:         extra = dict(e1=u'Hello', e2='Здра́вствуйте', e3=u'שלום', e4='你好',
Line 1088:                      e5=invalid, e6=u_invalid)
> VmId is a uuid, cannot contain unicode.
Believe me or not, vm_id as passed to SimpleLogAdapter constructor in Vm.__init__ is a unicode instance (I didn't investigated why). It's not a problem per se, but it makes a unicode from the formatted string in SimpleLogAdapter.perform (thus the returned result is going to be unicode as well!) and when a non-ASCII message is logged, it crashes.

No objections against validating input, it's definitely a right thing to do, but a few additional checks don't harm, especially at a place where we've already been hit.
Line 1089:         log = logUtils.SimpleLogAdapter(self._log, extra)
Line 1090:         log.debug("Dobrý večer")
Line 1091:         log.debug(u"Dobrý večer")
Line 1092:         log.debug(invalid)


Line 1087:         extra = dict(e1=u'Hello', e2='Здра́вствуйте', e3=u'שלום', e4='你好',
Line 1088:                      e5=invalid, e6=u_invalid)
Line 1089:         log = logUtils.SimpleLogAdapter(self._log, extra)
Line 1090:         log.debug("Dobrý večer")
Line 1091:         log.debug(u"Dobrý večer")
> We control whats in the source, for input, we should encode/decode values a
I agree, and the check in SimpleLogAdapter conforms to that rule now.
Line 1092:         log.debug(invalid)


-- 
To view, visit https://gerrit.ovirt.org/48542
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1930817ade1799b218913348ffa099aab2211ef7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal <mzamazal at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list