launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/simplejson-3.8.2 into lp:launchpad.

Commit message:
Upgrade to simplejson 3.8.2, fixing decoding of Git diffs involving non-UTF-8 text that decodes to unpaired surrogates when treated as UTF-8.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1589411 in Launchpad itself: "can not generate diff with non-UTF-8 encoding files"
  https://bugs.launchpad.net/launchpad/+bug/1589411

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/simplejson-3.8.2/+merge/296539

Upgrade to simplejson 3.8.2, fixing decoding of Git diffs involving non-UTF-8 text that decodes to unpaired surrogates when treated as UTF-8.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/simplejson-3.8.2 into lp:launchpad.

=== modified file 'lib/lp/code/model/tests/test_githosting.py'
--- lib/lp/code/model/tests/test_githosting.py	2016-05-19 14:54:33 +0000
+++ lib/lp/code/model/tests/test_githosting.py	2016-06-06 11:17:56 +0000
@@ -1,3 +1,7 @@
+# -*- coding: utf-8 -*-
+# NOTE: The first line above must stay first; do not move the copyright
+# notice to the top.  See http://www.python.org/dev/peps/pep-0263/.
+#
 # Copyright 2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
@@ -226,6 +230,17 @@
         self.assertRequest(
             "repo/123/compare-merge/a:b?sha1_prerequisite=c", method="GET")
 
+    def test_getMergeDiff_unpaired_surrogate(self):
+        # pygit2 tries to decode the diff as UTF-8 with errors="replace".
+        # In some cases this can result in unpaired surrogates, which older
+        # versions of json/simplejson don't like.
+        content = json.dumps(
+            {"patch": "卷。".encode("GBK").decode("UTF-8", errors="replace")})
+        with self.mockRequests(content=content):
+            diff = self.client.getMergeDiff("123", "a", "b")
+        self.assertEqual({"patch": "\uFFFD\uD863"}, diff)
+        self.assertRequest("repo/123/compare-merge/a:b", method="GET")
+
     def test_getMergeDiff_failure(self):
         with self.mockRequests(status_code=400, content=b"Bad request"):
             self.assertRaisesWithContent(

=== modified file 'versions.cfg'
--- versions.cfg	2016-05-24 04:45:38 +0000
+++ versions.cfg	2016-06-06 11:17:56 +0000
@@ -125,7 +125,7 @@
 s4 = 0.1.2
 setproctitle = 1.1.7
 setuptools-git = 1.0
-simplejson = 3.1.3
+simplejson = 3.8.2
 SimpleTAL = 4.3
 six = 1.9.0
 soupmatchers = 0.2