launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-698349-cStringIO-unicode-funsies into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #698349 Exception while uploading build upload log
  https://bugs.launchpad.net/bugs/698349

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-698349-cStringIO-unicode-funsies/+merge/45470

This branch fixes a crash in the upload processor, where PackageBuild.createUploadLog explodes if it receives a unicode instead of a str. The fix is simply to encode any unicode as UTF-8.

The problem is described in detail at https://bugs.launchpad.net/launchpad/+bug/698349/comments/3: there are some differences in Unicode handling between cStringIO and StringIO which result in roughly A Whole Lot of Fun when switching from one to the other.

To QA, cause a build upload to be rejected with a Unicode exception -- for example, a "Version older than that in the archive" error. If the upload processor doesn't crash, this fix is good.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-698349-cStringIO-unicode-funsies/+merge/45470
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-698349-cStringIO-unicode-funsies into lp:launchpad.

=== modified file 'lib/lp/buildmaster/model/packagebuild.py'
--- lib/lp/buildmaster/model/packagebuild.py	2010-12-15 22:30:35 +0000
+++ lib/lp/buildmaster/model/packagebuild.py	2011-01-07 00:33:56 +0000
@@ -222,6 +222,8 @@
         if filename is None:
             filename = 'upload_%s_log.txt' % self.id
         contentType = filenameToContentType(filename)
+        if isinstance(content, unicode):
+            content = content.encode('utf-8')
         file_size = len(content)
         file_content = StringIO(content)
         restricted = self.is_private

=== modified file 'lib/lp/buildmaster/tests/test_packagebuild.py'
--- lib/lp/buildmaster/tests/test_packagebuild.py	2010-11-19 12:22:15 +0000
+++ lib/lp/buildmaster/tests/test_packagebuild.py	2011-01-07 00:33:56 +0000
@@ -141,6 +141,14 @@
         self.package_build.storeUploadLog("Some content")
         self.failUnless(self.package_build.upload_log.restricted)
 
+    def test_storeUploadLog_unicode(self):
+        # Unicode upload logs are uploaded as UTF-8.
+        self.package_build.storeUploadLog(u"Some content \N{SNOWMAN}")
+        self.failIfEqual(None, self.package_build.upload_log)
+        self.failUnlessEqual(
+            hashlib.sha1("Some content \xe2\x98\x83").hexdigest(),
+            self.package_build.upload_log.content.sha1)
+
     def test_upload_log_url(self):
         # The url of the upload log file is determined by the PackageBuild.
         Store.of(self.package_build).flush()