launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/launchpad/unicode-bug-806882 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #806882 in Launchpad itself: "UnicodeDecodeError when processing recipe build uploads"
  https://bugs.launchpad.net/launchpad/+bug/806882

For more details, see:
https://code.launchpad.net/~rvb/launchpad/unicode-bug-806882/+merge/80326

This branch removes the usage of .encode('utf-8') that has been introduced before the all-should-be-unicode era (https://code.launchpad.net/~julian-edwards/launchpad/upload-bug-275509/+merge/8265).

Unfortunately, bigjools and I were unable to recreate the conditions that triggered this problem but we know the name of the team in question as non ascii characters in it. This fix is only a guess but I'm pretty confident it's the right one (as a rule of thumb, we should never use encoded strings inside LP outside of interfaces with other systems (mail, etc)). I think the UnicodeDecodeError comes from an attempt to concatenate Unicode string with encoding string containing non ascii characters.

self.reject(error_message) simply adds error_message to a list that will later be concatenated into give a single error message containing all the errors. The list already may contains Unicode strings (because statements like this "there is a problem with distro %s" %distro.name where distro.name is Unicode return Unicode strings.) so the change in this branch simply preserves the Unicode nature of the error messages instead of encoding it to utf8 prematurely. The fix itself is pretty simple, it removes the encoding of an error string that might be later concatenated with a proper unicode string.

= Tests =

(modified test)
./bin/test -vvc test_ppauploadprocessor testErrorMessagesWithUnicode

= Q/A =

None
-- 
https://code.launchpad.net/~rvb/launchpad/unicode-bug-806882/+merge/80326
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/unicode-bug-806882 into lp:launchpad.

=== modified file 'lib/lp/archiveuploader/nascentupload.py'
--- lib/lp/archiveuploader/nascentupload.py	2011-10-20 12:05:21 +0000
+++ lib/lp/archiveuploader/nascentupload.py	2011-10-25 12:22:27 +0000
@@ -391,9 +391,9 @@
         try:
             callable()
         except UploadError, error:
-            self.reject("".join(error.args).encode("utf8"))
+            self.reject("".join(error.args))
         except UploadWarning, error:
-            self.warn("".join(error.args).encode("utf8"))
+            self.warn("".join(error.args))
 
     def run_and_collect_errors(self, callable):
         """Run 'special' callable that generates a list of errors/warnings.
@@ -413,9 +413,9 @@
         """
         for error in callable():
             if isinstance(error, UploadError):
-                self.reject("".join(error.args).encode("utf8"))
+                self.reject("".join(error.args))
             elif isinstance(error, UploadWarning):
-                self.warn("".join(error.args).encode("utf8"))
+                self.warn("".join(error.args))
             else:
                 raise AssertionError(
                     "Unknown error occurred: %s" % str(error))

=== modified file 'lib/lp/archiveuploader/tests/test_ppauploadprocessor.py'
--- lib/lp/archiveuploader/tests/test_ppauploadprocessor.py	2011-08-25 08:19:28 +0000
+++ lib/lp/archiveuploader/tests/test_ppauploadprocessor.py	2011-10-25 12:22:27 +0000
@@ -1029,16 +1029,16 @@
         # errors.
         self.assertEqual(
             self.uploadprocessor.last_processed_upload.rejection_message,
-            'File bar_1.0.orig.tar.gz already exists in unicode PPA name: '
-            'áří, but uploaded version has different '
-            'contents. See more information about this error in '
-            'https://help.launchpad.net/Packaging/UploadErrors.\n'
-            'File bar_1.0-1.diff.gz already exists in unicode PPA name: '
-            'áří, but uploaded version has different contents. See more '
-            'information about this error in '
-            'https://help.launchpad.net/Packaging/UploadErrors.\n'
-            'Files specified in DSC are broken or missing, skipping package '
-            'unpack verification.')
+            u'File bar_1.0.orig.tar.gz already exists in unicode PPA name: '
+            u'áří, but uploaded version has different '
+            u'contents. See more information about this error in '
+            u'https://help.launchpad.net/Packaging/UploadErrors.\n'
+            u'File bar_1.0-1.diff.gz already exists in unicode PPA name: '
+            u'áří, but uploaded version has different contents. See more '
+            u'information about this error in '
+            u'https://help.launchpad.net/Packaging/UploadErrors.\n'
+            u'Files specified in DSC are broken or missing, skipping package '
+            u'unpack verification.')
 
         # Also, the email generated should be sane.
         from_addr, to_addrs, raw_msg = stub.test_emails.pop()