Encoding hard-coded in gettextutils.Message

[John Warren]

The __str__ method in openstack.common.gettextutils.Message is hard-coded to encode to utf-8 which creates problems when other logic goes to decode the string using the default encoding, e.g. when formatting log messages. Message objects with double-byte translations fail to log because of this.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/46553

[Mark McLoughlin (markmc)]

Since Nova calls gettextutils.enable_lazy(), I'm assuming this is potentially a serious issue for Nova ... so targeting to Nova's rc1

[Mark McLoughlin (markmc)]

ceilometer also calls enable_lazy()

[Mark McLoughlin (markmc)]

Ah, cinder calls gettextutils.install('cinder', lazy=True)

[Mark McLoughlin (markmc)]

Heat calls gettextutils.install('heat', lazy=True)

[Mark McLoughlin (markmc)]

Neutron calls gettextutils.install('neutron', lazy=True)

[Mark McLoughlin (markmc)]

See also https://blueprints.launchpad.net/openstack/?searchtext=user-locale-api

[Mark McLoughlin (markmc)]

Meh, that search link doesn't get completed blueprints

For reference

https://blueprints.launchpad.net/nova/+spec/user-locale-api
https://blueprints.launchpad.net/glance/+spec/user-locale-api
https://blueprints.launchpad.net/ceilometer/+spec/user-locale-api
https://blueprints.launchpad.net/heat/+spec/user-locale-api
https://blueprints.launchpad.net/cinder/+spec/user-locale-api
https://blueprints.launchpad.net/neutron/+spec/user-locale-api
https://blueprints.launchpad.net/keystone/+spec/user-locale-api

[Mark McLoughlin (markmc)]

ttx, dhellman and I discussed this a little on irc - conclusions:

1) We're not 100% sure what the user impact of this issue is, but we suspect it has the potential to be a critical issue for some users. Is it the case that a translation containing double-byte characters can basically render a service completely unusable?

2) Our fear is that the fix is complex enough such that there's a reasonable chance it's broken in some way

i.e. this is a critical issue and we don't have a huge amount of confidence that this fix resolves the issue without introducing further issues

That all suggests that we should disable lazy translation in all projects and revisit in Icehouse

This would involve removing the calls to gettextutils.enable_lazy() or gettextutils.install(lazy=True)

But we're eager to hear thoughts from folks more intimately familiar with the work - e.g. mrodden, luisg, bnemec, jswarren

[Luis A. Garcia (luisg-8)]

Sorry we missed you on IRC Mark. We met with dhellman, and bnemec on IRC.

We all agree that the current proposal of basically changing the Message class at a fundamental level so that it is immutable is a no-go for Havana, given the complexity of the fix and risk for regressions.

However given how the issue is limited to logging, there is a different approach that was proposed by dhellman that addresses the specific problem with logging instead of having to drop the whole REST API translations feature.

To recap the problem, inside python logging the Message is converted first to string then to unicode, which breaks if the message contained non-ascii characters. It is converted first to string because that is the default behavior for non-basestring classes which Message is.

dhellman proposes creating a LoggerAdapter, very similar to ContextAdapter, that will unicode() the Message objects so the loggers don't deal with Messages.

We will write, test and send out a patch shortly.

[Doug Hellmann (doug-hellmann)]

We still need to look at jwarren's changeset to see if the Message class has any fundamental issues.

I also still think it is safer to disable the lazy evaluation in the other projects for now, and focus on fixing translation handling more completely during Icehouse.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/49230

[John Warren (jswarren)]

My comments in my changeset were addressed to people who wanted to have Message extend unicode and keep the rest of the class basically in tact. I don't see a problem with keeping Message extending UserString and handling the issue via a logging Adapter as a tactical solution. I would recommend making the changes I suggested to Message class in Icehouse.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/49266

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/49275

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/49278

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/49230
Committed: http://github.com/openstack/oslo-incubator/commit/572cfecb4da5c618a407b8de2a7feae9c3f35ce4
Submitter: Jenkins
Branch: master

commit 572cfecb4da5c618a407b8de2a7feae9c3f35ce4
Author: Matt Odden <email address hidden>
Date: Tue Oct 1 05:11:43 2013 +0000

    Make Messages unicode before hitting logging

    Some of the log handlers in the python standard library
    are hitting issues with Messages since they check for
    basestring types only, and attempt to coerce anything else
    to string. This leads to Messages being coerced to bad encodings,
    resulting in unhandled exceptions.

    This change adds a check that looks for non-basestring
    objects and coerces them to unicode before they hit the problematic
    logging areas.

    bug 1225099

    Change-Id: I0bff6b52205e3c88db38876b8c6de1bd820f460a
    Co-authored-by: Luis A. Garcia <email address hidden>

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/49287

[Mark McLoughlin (markmc)]

Nova change: https://review.openstack.org/#/c/49277/

[Mark McLoughlin (markmc)]

We used this etherpad for coordinating: https://etherpad.openstack.org/disable-lazy-translation

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/49266
Committed: http://github.com/openstack/heat/commit/53b9314799c7d83779032c4535734f1c05326a27
Submitter: Jenkins
Branch: master

commit 53b9314799c7d83779032c4535734f1c05326a27
Author: Ben Nemec <email address hidden>
Date: Tue Oct 1 21:39:07 2013 +0000

    Disable lazy translation

    Late in the Havana cycle bug 1225099 was found in the lazy
    translation code, and to be safe it was decided to disable lazy
    translation for Havana. This change does that.

    Change-Id: Ia934a7df9386baf6ae8eb9ff48c24386c47ecd23
    Partial-bug: 1225099

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/49275
Committed: http://github.com/openstack/cinder/commit/52a98c29755f893154263a7793c0135c763938f6
Submitter: Jenkins
Branch: master

commit 52a98c29755f893154263a7793c0135c763938f6
Author: Doug Hellmann <email address hidden>
Date: Tue Oct 1 18:01:59 2013 -0400

    Disable lazy translation

    Late in the Havana cycle bug 1225099 was found in the lazy
    translation code, and to be safe it was decided to disable lazy
    translation for Havana. This change does that.

    Change-Id: Ia934a7df9386baf6ae8eb9ff48c24386c47ecd23
    Partial-bug: 1225099

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/49278
Committed: http://github.com/openstack/ceilometer/commit/6840df8e5e48117438a8431bcf428343f988ddc4
Submitter: Jenkins
Branch: master

commit 6840df8e5e48117438a8431bcf428343f988ddc4
Author: Ben Nemec <email address hidden>
Date: Tue Oct 1 22:19:51 2013 +0000

    Disable lazy translation

    Late in the Havana cycle bug 1225099 was found in the lazy
    translation code, and to be safe it was decided to disable lazy
    translation for Havana. This change does that.

    Change-Id: Ia934a7df9386baf6ae8eb9ff48c24386c47ecd23
    Partial-bug: 1225099

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/49277
Committed: http://github.com/openstack/nova/commit/e355d476187951181ed644ebc2d0f7e858cb3cf4
Submitter: Jenkins
Branch: master

commit e355d476187951181ed644ebc2d0f7e858cb3cf4
Author: Matt Odden <email address hidden>
Date: Tue Oct 1 09:20:04 2013 +0000

    Disable lazy gettext

    This change disables lazy gettext functionality in Nova,
    due to problems with character encoding and logging
    detailed in bug 1225099.

    It is part of a series of commits across all projects with
    lazy gettext enabled.

    Change-Id: Ia934a7df9386baf6ae8eb9ff48c24386c47ecd23
    Partial-bug: 1225099

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/49287
Committed: http://github.com/openstack/neutron/commit/76e7542f0f54779f469df7125ff7b94480c44963
Submitter: Jenkins
Branch: master

commit 76e7542f0f54779f469df7125ff7b94480c44963
Author: Ben Nemec <email address hidden>
Date: Tue Oct 1 23:15:23 2013 +0000

    Disable lazy translation

    Late in the Havana cycle bug 1225099 was found in the lazy
    translation code, and to be safe it was decided to disable lazy
    translation for Havana. This change does that.

    Change-Id: Ia934a7df9386baf6ae8eb9ff48c24386c47ecd23
    Partial-bug: 1225099

[Matt Riedemann (mriedem)]

Keystone patch is here: https://review.openstack.org/#/c/49285/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/49285
Committed: http://github.com/openstack/keystone/commit/c3b92295b718a41c3136876eb39297081015a97c
Submitter: Jenkins
Branch: master

commit c3b92295b718a41c3136876eb39297081015a97c
Author: Matt Odden <email address hidden>
Date: Tue Oct 1 08:52:15 2013 +0000

    Disable lazy gettext

    This change disables lazy gettext functionality in Keystone,
    due to problems with character encoding and logging
    detailed in bug 1225099.

    It is part of a series of commits across all projects with
    lazy gettext enabled.

    Change-Id: Ia934a7df9386baf6ae8eb9ff48c24386c47ecd23
    Partial-bug: 1225099

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: milestone-proposed
Review: https://review.openstack.org/52120