launchpad-reviewers team mailing list archive

Thread
Date

[Merge] lp:~gz/python-oops-tools/dbsummaries_utf8_encode_891186 into lp:python-oops-tools

To: mp+85180@xxxxxxxxxxxxxxxxxx
From: Martin Packman <martin.packman@xxxxxxxxxxxxx>
Date: Fri, 09 Dec 2011 19:22:23 -0000
Reply-to: mp+85180@xxxxxxxxxxxxxxxxxx
Sender: bounces@xxxxxxxxxxxxx

Martin Packman has proposed merging lp:~gz/python-oops-tools/dbsummaries_utf8_encode_891186 into lp:python-oops-tools.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #891186 in python-oops-tools: "analyse_oops_reports raised an UnicodeEncodeError loading an oops from the filesystem"
  https://bugs.launchpad.net/python-oops-tools/+bug/891186

For more details, see:
https://code.launchpad.net/~gz/python-oops-tools/dbsummaries_utf8_encode_891186/+merge/85180

There doesn't seem any need to hold off on this bug, so this branch merges Diogo's test and gets it working then applies a version of the fix I hope is acceptable to everyone. As we're not convinced that non-ascii byte strings can't be passed to the _escape function, cope with them by doing some gentle mangling to ascii. This ensures the output is well encoded but should give enough feedback if there are issues.

Is there anything else that's required? It's possible that some more _escape() calls need adding around some of the output, but this should be enough to resolve the bug as filed.
-- 
https://code.launchpad.net/~gz/python-oops-tools/dbsummaries_utf8_encode_891186/+merge/85180
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gz/python-oops-tools/dbsummaries_utf8_encode_891186 into lp:python-oops-tools.

=== modified file 'src/oopstools/oops/dbsummaries.py'
--- src/oopstools/oops/dbsummaries.py	2011-10-24 05:21:01 +0000
+++ src/oopstools/oops/dbsummaries.py	2011-12-09 19:21:31 +0000
@@ -42,6 +42,11 @@
 
 def _escape(value):
     if value is not None:
+        if not isinstance(value, unicode):
+            # If a random byte string gets here, ensure it's ascii
+            value = value.decode("latin1").encode("ascii", "backslashreplace")
+        else:
+            value = value.encode("utf-8")
         value = cgi.escape(value)
     return value
 
@@ -656,6 +661,7 @@
         fp.write('<html>\n'
                  '<head>\n'
                  '<title>Oops Report Summary</title>\n'
+                 '<meta http-equiv="Content-Type" content="text/html;charset=UTF-8" />\n'
                  '<link rel="stylesheet" type="text/css" href="%s/oops/static/oops.css" />\n'
                  '</head>\n'
                  '<body>\n'

=== added file 'src/oopstools/oops/test/test_dbsummaries.py'
--- src/oopstools/oops/test/test_dbsummaries.py	1970-01-01 00:00:00 +0000
+++ src/oopstools/oops/test/test_dbsummaries.py	2011-12-09 19:21:31 +0000
@@ -0,0 +1,51 @@
+# Copyright 2005-2011 Canonical Ltd.  All rights reserved.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+from datetime import (
+    datetime,
+    )
+from cStringIO import StringIO
+
+from pytz import utc
+from testtools import TestCase
+from testtools.matchers import Contains
+
+from oopstools.oops.dbsummaries import WebAppErrorSummary
+from oopstools.oops.models import parsed_oops_to_model_oops
+
+
+class TestWebAppErrorSummary(TestCase):
+
+    def _createOops(self):
+        python_oops = {'id': 'OOPS-1234S101', 'reporter': 'edge',
+            'type': 'Exception', 'value': u'a unicode char (\xa7)',
+            'time': datetime(2008, 1, 13, 23, 14, 23, 00, utc)}
+        ignored = parsed_oops_to_model_oops(
+            python_oops, 'test_unicode_handling')
+
+    def setUp(self):
+        super(TestWebAppErrorSummary, self).setUp()
+        self._createOops()
+        start = end = datetime(2008, 1, 13)
+        prefixes = ['EDGE']
+        self.summary = WebAppErrorSummary(start, end, prefixes)
+
+    def test_renderHTML_with_unicode_data(self):
+        # Using cStringIO here to simulate a file object open without an
+        # encoding specified.
+        fp = StringIO()
+        self.summary.renderHTML(fp)
+        self.assertThat(fp.getvalue(), Contains('a unicode char (\xc2\xa7)'))

=== modified file 'src/oopstools/scripts/analyse_error_reports.py'
--- src/oopstools/scripts/analyse_error_reports.py	2011-10-13 20:18:51 +0000
+++ src/oopstools/scripts/analyse_error_reports.py	2011-12-09 19:21:31 +0000
@@ -26,14 +26,7 @@
     Prefix,
     Report,
     )
-from oopstools.oops.oopsstore import OopsStore
 from oopstools.oops import dbsummaries
-from oopstools.oops.summaries import (
-    WebAppErrorSummary,
-    CheckwatchesErrorSummary,
-    CodeHostingWithRemoteSectionSummary,
-    GenericErrorSummary,
-)
 
 
 def main(argv=None):