launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-ref-url-encode-non-ascii into lp:launchpad.

Commit message:
Generate properly-encoded URLs for non-ASCII git reference names.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-ref-url-encode-non-ascii/+merge/353633

Ref names can be practically anything up to and including emoji, just as long as certain ASCII control and punctuation characters aren't used.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-ref-url-encode-non-ascii into lp:launchpad.

=== modified file 'lib/lp/code/browser/tests/test_gitlisting.py'
--- lib/lp/code/browser/tests/test_gitlisting.py	2018-08-22 14:36:31 +0000
+++ lib/lp/code/browser/tests/test_gitlisting.py	2018-08-23 10:18:20 +0000
@@ -41,7 +41,7 @@
             main_repo,
             paths=[
                 "refs/heads/master", "refs/heads/1.0", "refs/heads/with#hash",
-                "refs/tags/1.1",
+                "refs/heads/\N{SNOWMAN}", "refs/tags/1.1",
                 ])
 
         other_repo = self.factory.makeGitRepository(
@@ -77,7 +77,7 @@
         table = soup.find(
             'div', id='default-repository-branches').find('table')
         self.assertContentEqual(
-            ['1.0', 'master', 'with#hash'],
+            ['1.0', 'master', 'with#hash', '\N{SNOWMAN}'],
             [link.find(text=True) for link in table.findAll('a')])
         self.assertEndsWith(
             table.find(text="1.0").parent['href'],
@@ -85,6 +85,9 @@
         self.assertEndsWith(
             table.find(text="with#hash").parent['href'],
             "/~foowner/%s/+git/foo/+ref/with%%23hash" % self.target_path)
+        self.assertEndsWith(
+            table.find(text="\N{SNOWMAN}").parent['href'],
+            "/~foowner/%s/+git/foo/+ref/%%E2%%98%%83" % self.target_path)
 
         # Other repos are listed.
         table = soup.find(

=== modified file 'lib/lp/code/browser/tests/test_gitref.py'
--- lib/lp/code/browser/tests/test_gitref.py	2018-08-20 23:33:01 +0000
+++ lib/lp/code/browser/tests/test_gitref.py	2018-08-23 10:18:20 +0000
@@ -68,6 +68,12 @@
             "%s/+ref/with%%23hash" % canonical_url(ref.repository),
             canonical_url(ref))
 
+    def test_canonical_url_non_ascii(self):
+        [ref] = self.factory.makeGitRefs(paths=["refs/heads/\N{SNOWMAN}"])
+        self.assertEqual(
+            "%s/+ref/%%E2%%98%%83" % canonical_url(ref.repository),
+            canonical_url(ref))
+
     def test_canonical_url_tag(self):
         [ref] = self.factory.makeGitRefs(paths=["refs/tags/1.0"])
         self.assertEqual(

=== modified file 'lib/lp/code/browser/tests/test_gitrepository.py'
--- lib/lp/code/browser/tests/test_gitrepository.py	2018-08-20 23:33:01 +0000
+++ lib/lp/code/browser/tests/test_gitrepository.py	2018-08-23 10:18:20 +0000
@@ -81,6 +81,11 @@
         url = "%s/+ref/with%%23hash" % canonical_url(ref.repository)
         self.assertEqual(ref, test_traverse(url)[0])
 
+    def test_traverse_non_ascii(self):
+        [ref] = self.factory.makeGitRefs(paths=["refs/heads/\N{SNOWMAN}"])
+        url = "%s/+ref/%%E2%%98%%83" % canonical_url(ref.repository)
+        self.assertEqual(ref, test_traverse(url)[0])
+
 
 class TestGitRepositoryView(BrowserTestCase):
 

=== modified file 'lib/lp/code/model/gitref.py'
--- lib/lp/code/model/gitref.py	2018-08-20 23:33:01 +0000
+++ lib/lp/code/model/gitref.py	2018-08-23 10:18:20 +0000
@@ -110,7 +110,7 @@
     @property
     def url_quoted_name(self):
         """See `IGitRef`."""
-        return quote(self.name)
+        return quote(self.name.encode("UTF-8"))
 
     @property
     def identity(self):

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2018-07-02 14:08:22 +0000
+++ lib/lp/testing/factory.py	2018-08-23 10:18:20 +0000
@@ -20,7 +20,6 @@
     ]
 
 import base64
-from cryptography.utils import int_to_bytes
 from datetime import (
     datetime,
     timedelta,
@@ -49,6 +48,7 @@
 
 from bzrlib.plugins.builder.recipe import BaseRecipeBranch
 from bzrlib.revision import Revision as BzrRevision
+from cryptography.utils import int_to_bytes
 from lazr.jobrunner.jobrunner import SuspendJobException
 import pytz
 from pytz import UTC
@@ -1809,7 +1809,8 @@
             paths = [self.getUniqueString('refs/heads/path').decode('utf-8')]
         refs_info = {
             path: {
-                u"sha1": unicode(hashlib.sha1(path).hexdigest()),
+                u"sha1": unicode(
+                    hashlib.sha1(path.encode('utf-8')).hexdigest()),
                 u"type": GitObjectType.COMMIT,
                 }
             for path in paths}