Merge lp:~milner/landscape-client/prevent-client-ui-unicode into lp:~landscape/landscape-client/trunk

Proposed by Mike Milner
Status: Merged
Approved by: Chad Smith
Approved revision: 520
Merged at revision: 522
Proposed branch: lp:~milner/landscape-client/prevent-client-ui-unicode
Merge into: lp:~landscape/landscape-client/trunk
Diff against target: 43 lines (+22/-0)
2 files modified
landscape/ui/model/configuration/mechanism.py (+2/-0)
landscape/ui/model/configuration/tests/test_mechanism.py (+20/-0)
To merge this branch: bzr merge lp:~milner/landscape-client/prevent-client-ui-unicode
Reviewer Review Type Date Requested Status
Chad Smith Approve
Alberto Donato (community) Approve
Review via email: mp+97704@code.launchpad.net

Description of the change

landscape-client's config system does not handle non-ascii characters. This small fix prevents non-ascii characters from the ui getting into the config system.

This branch simply replaces non-ascii characters with "?".

To post a comment you must log in.
Revision history for this message
Alberto Donato (ack) wrote :

+1!

review: Approve
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

It's ugly, but works in the sense that it doesn't crash ;)

But of course the user won't be able to register the machine like this. Should we perhaps file a new bug about this to fix it properly? With a dialog box perhaps saying that the <field> contained invalid characters.

Revision history for this message
Mike Milner (milner) wrote :

> It's ugly, but works in the sense that it doesn't crash ;)
>
> But of course the user won't be able to register the machine like this. Should
> we perhaps file a new bug about this to fix it properly? With a dialog box
> perhaps saying that the <field> contained invalid characters.

I do have another branch I started first that adds proper unicode support to the config system but it touches too many core parts just before a release.

I've opened Bug #956612 to remind us that we need a better story here.

Revision history for this message
Chad Smith (chad.smith) wrote :

+1 on assumption that we'll handle this more safely in Bug #956612

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'landscape/ui/model/configuration/mechanism.py'
--- landscape/ui/model/configuration/mechanism.py 2012-03-09 17:06:06 +0000
+++ landscape/ui/model/configuration/mechanism.py 2012-03-15 17:13:35 +0000
@@ -97,6 +97,8 @@
97 L{LandscapeSetupConfiguration}.97 L{LandscapeSetupConfiguration}.
98 """98 """
99 if self._is_allowed_by_policy(sender, conn, POLICY_NAME):99 if self._is_allowed_by_policy(sender, conn, POLICY_NAME):
100 # Underlying _config does not support unicode so convert to ascii
101 value = unicode(value).encode("ascii", errors="replace")
100 setattr(self._config, name, value)102 setattr(self._config, name, value)
101103
102 @dbus.service.method(INTERFACE_NAME,104 @dbus.service.method(INTERFACE_NAME,
103105
=== modified file 'landscape/ui/model/configuration/tests/test_mechanism.py'
--- landscape/ui/model/configuration/tests/test_mechanism.py 2012-03-07 09:39:35 +0000
+++ landscape/ui/model/configuration/tests/test_mechanism.py 2012-03-15 17:13:35 +0000
@@ -65,6 +65,26 @@
65 self.mechanism.set("account_name", "bar")65 self.mechanism.set("account_name", "bar")
66 self.assertEqual("bar", self.mechanism.get("account_name"))66 self.assertEqual("bar", self.mechanism.get("account_name"))
6767
68 def test_set_account_name_unicode(self):
69 """
70 Non-ascii characters are replaced before passing to underlying config.
71 """
72 self.mechanism.set("account_name", u"unicode\u00a3unicode")
73 self.assertEqual("unicode?unicode", self.mechanism.get("account_name"))
74
75 def test_no_unicode_to_underlying_config(self):
76 """
77 Non-ascii characters are replaced before passing to underlying config.
78 """
79 class FakeConfig(object):
80 def __init__(self):
81 self.account_name = None
82
83 fake_config = FakeConfig()
84 self.mechanism._config = fake_config
85 self.mechanism.set("account_name", u"unicode\u00a3unicode")
86 self.assertEqual("unicode?unicode", fake_config.account_name)
87
68 def test_get_data_path(self):88 def test_get_data_path(self):
69 """89 """
70 Test we can get the data path from the mechanism.90 Test we can get the data path from the mechanism.

Subscribers

People subscribed via source and target branches

to all changes: