launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/turnip:py3-cgit-resource into turnip:master with ~pappacena/turnip:py3-twisted-cgi-compat-backport as a prerequisite.

Commit message:
Making cgit resource compatible with python3

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/turnip/+git/turnip/+merge/395407
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/turnip:py3-cgit-resource into turnip:master.

diff --git a/turnip/pack/http.py b/turnip/pack/http.py
index 8f4ce1c..a9772be 100644
--- a/turnip/pack/http.py
+++ b/turnip/pack/http.py
@@ -416,7 +416,7 @@ class RobotsResource(static.Data):
         """).encode('US-ASCII')
 
     def __init__(self):
-        static.Data.__init__(self, self.robots_txt, b'text/plain')
+        static.Data.__init__(self, self.robots_txt, 'text/plain')
 
 
 class CGitScriptResource(twcgi.CGIScript):
@@ -440,7 +440,9 @@ class CGitScriptResource(twcgi.CGIScript):
         self.cgit_config = tempfile.NamedTemporaryFile(
             mode='w+', prefix='turnip-cgit-')
         os.chmod(self.cgit_config.name, 0o644)
-        fmt = {'repo_url': self.repo_url, 'repo_path': self.repo_path}
+        fmt = {
+            'repo_url': six.ensure_text(self.repo_url),
+            'repo_path': six.ensure_text(self.repo_path)}
         if self.root.site_name is not None:
             prefixes = " ".join(
                 "{}://{}".format(scheme, self.root.site_name)
@@ -470,6 +472,7 @@ class CGitScriptResource(twcgi.CGIScript):
         env["CGIT_CONFIG"] = self.cgit_config.name
         env["PATH_INFO"] = "/%s%s" % (self.repo_url, self.trailing)
         env["SCRIPT_NAME"] = "/"
+        env['QUERY_STRING'] = six.ensure_text(env['QUERY_STRING'])
         twcgi.CGIScript.runProcess(self, env, request, *args, **kwargs)
 
 
@@ -617,7 +620,7 @@ class HTTPAuthRootResource(BaseHTTPAuthResource):
         if 'path' not in translated:
             return fail_request(
                 request, 'translatePath response did not include path')
-        repo_url = request.path.rstrip('/')
+        repo_url = six.ensure_text(request.path.rstrip(b'/'))
         # cgit simply parses configuration values up to the end of a line
         # following the first '=', so almost anything is safe, but
         # double-check that there are no newlines to confuse things.
@@ -625,7 +628,8 @@ class HTTPAuthRootResource(BaseHTTPAuthResource):
             return fail_request(
                 request, 'repository URL may not contain newlines')
         try:
-            repo_path = compose_path(self.root.repo_store, translated['path'])
+            repo_path = compose_path(
+                self.root.repo_store, six.ensure_binary(translated['path']))
         except ValueError as e:
             return fail_request(request, str(e))
         trailing = translated.get('trailing')
@@ -638,9 +642,12 @@ class HTTPAuthRootResource(BaseHTTPAuthResource):
                     'translatePath returned inconsistent response: '
                     '"%s" does not end with "%s"' % (repo_url, trailing))
             repo_url = repo_url[:-len(trailing)]
-        repo_url = repo_url.strip('/')
+        repo_url = six.ensure_text(repo_url.strip('/'))
         cgit_resource = CGitScriptResource(
             self.root, repo_url, repo_path, trailing, translated['private'])
+        # XXX pappacena 2020-12-15: CGIScript.render assumes postpath items
+        # are str, and fail if any item is bytes.
+        request.postpath = [six.ensure_text(i) for i in request.postpath]
         request.render(cgit_resource)
 
     def _translatePathErrback(self, failure, request, session):
@@ -680,7 +687,7 @@ class HTTPAuthRootResource(BaseHTTPAuthResource):
         identity_url = session.get('identity_url', self.anonymous_id)
         proxy = xmlrpc.Proxy(self.root.virtinfo_endpoint, allowNone=True)
         d = proxy.callRemote(
-            'translatePath', request.path, b'read',
+            'translatePath', six.ensure_text(request.path), 'read',
             {'uid': identity_url, 'can-authenticate': True})
         d.addTimeout(self.root.virtinfo_timeout, self.root.reactor)
         d.addCallback(self._translatePathCallback, request)
@@ -714,7 +721,8 @@ class SmartHTTPFrontendResource(resource.Resource):
         resource.Resource.__init__(self)
         self.backend_host = config.get("pack_virt_host")
         self.backend_port = int(config.get("pack_virt_port"))
-        self.virtinfo_endpoint = config.get("virtinfo_endpoint")
+        self.virtinfo_endpoint = six.ensure_binary(
+            config.get("virtinfo_endpoint"))
         self.virtinfo_timeout = int(config.get("virtinfo_timeout"))
         self.reactor = reactor or default_reactor
         # XXX cjwatson 2015-03-30: Knowing about the store path here
@@ -745,9 +753,9 @@ class SmartHTTPFrontendResource(resource.Resource):
             with open(os.path.join(stdir, 'private.css'), 'rb') as f:
                 private_css = css + b'\n' + f.read()
             static_resource.putChild(
-                b'cgit-public.css', static.Data(css, b'text/css'))
+                b'cgit-public.css', static.Data(css, 'text/css'))
             static_resource.putChild(
-                b'cgit-private.css', static.Data(private_css, b'text/css'))
+                b'cgit-private.css', static.Data(private_css, 'text/css'))
             self.putChild(b'static', static_resource)
             favicon = os.path.join(stdir, 'launchpad.png')
             self.putChild(b'favicon.ico', static.File(favicon))
diff --git a/turnip/pack/tests/fake_servers.py b/turnip/pack/tests/fake_servers.py
index 8ed152d..92b77b9 100644
--- a/turnip/pack/tests/fake_servers.py
+++ b/turnip/pack/tests/fake_servers.py
@@ -105,6 +105,9 @@ class FakeVirtInfoService(xmlrpc.XMLRPC):
             else:
                 clone_from = None
             retval["creation_params"] = {"clone_from": clone_from}
+        retval['private'] = False
+        retval['trailing'] = ''
+        retval["writable"] = writable
         return retval
 
     def xmlrpc_authenticateWithPassword(self, username, password):
diff --git a/turnip/pack/tests/test_functional.py b/turnip/pack/tests/test_functional.py
index e74e47f..02bf771 100644
--- a/turnip/pack/tests/test_functional.py
+++ b/turnip/pack/tests/test_functional.py
@@ -101,7 +101,7 @@ class FunctionalTestMixin(WithScenarios):
         self.virtinfo_listener = reactor.listenTCP(
             0, server.Site(self.virtinfo))
         self.virtinfo_port = self.virtinfo_listener.getHost().port
-        self.virtinfo_url = b'http://localhost:%d/' % self.virtinfo_port
+        self.virtinfo_url = 'http://localhost:%d/' % self.virtinfo_port
         self.addCleanup(self.virtinfo_listener.stopListening)
         self.virtinfo.ref_permissions = {
             b'refs/heads/master': ['create', 'push']}
diff --git a/turnip/pack/tests/test_http.py b/turnip/pack/tests/test_http.py
index 0c743b4..dcd4b22 100644
--- a/turnip/pack/tests/test_http.py
+++ b/turnip/pack/tests/test_http.py
@@ -8,7 +8,9 @@ from __future__ import (
     )
 
 from io import BytesIO
+import os
 
+from fixtures import TempDir
 import six
 from testtools import TestCase
 from testtools.deferredruntest import AsynchronousDeferredRunTest
@@ -18,9 +20,12 @@ from twisted.internet import (
     task,
     testing,
     )
+from twisted.internet.address import IPv4Address
 from twisted.web import server
 from twisted.web.test import requesthelper
 
+from turnip.api import store
+from turnip.config import config
 from turnip.pack import (
     helpers,
     http,
@@ -36,6 +41,10 @@ class LessDummyRequest(requesthelper.DummyRequest):
 
     startedWriting = 0
 
+    def __init__(self, *args, **kwargs):
+        super(LessDummyRequest, self).__init__(*args, **kwargs)
+        self.content = BytesIO()
+
     @property
     def value(self):
         return b"".join(self.written)
@@ -56,6 +65,9 @@ class LessDummyRequest(requesthelper.DummyRequest):
     def getPassword(self):
         return None
 
+    def getClientAddress(self):
+        return IPv4Address('TCP', '127.0.0.1', '80')
+
 
 class AuthenticatedLessDummyRequest(LessDummyRequest):
     def getUser(self):
@@ -85,10 +97,13 @@ class FakeRoot(object):
     allowed_services = frozenset((
         b'git-upload-pack', b'git-receive-pack', b'turnip-set-symbolic-ref'))
 
-    def __init__(self):
+    def __init__(self, repo_store=None):
         self.backend_transport = None
         self.client_factory = None
         self.backend_connected = defer.Deferred()
+        self.repo_store = repo_store
+        self.cgit_exec_path = config.get("cgit_exec_path")
+        self.site_name = 'turnip'
 
     def authenticateWithPassword(self, user, password):
         """Pretends to talk to Launchpad XML-RPC service to authenticate the user.
@@ -293,9 +308,10 @@ class TestHTTPAuthRootResource(TestCase):
 
     run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=5)
 
-    def test_translatePath_timeout(self):
-        root = FakeRoot()
-        virtinfo = FakeVirtInfoService(allowNone=True)
+    def setUp(self):
+        super(TestHTTPAuthRootResource, self).setUp()
+        self.root = root = FakeRoot(self.useFixture(TempDir()).path)
+        self.virtinfo = virtinfo = FakeVirtInfoService(allowNone=True)
         virtinfo_listener = default_reactor.listenTCP(0, server.Site(virtinfo))
         virtinfo_port = virtinfo_listener.getHost().port
         virtinfo_url = b'http://localhost:%d/' % virtinfo_port
@@ -304,6 +320,9 @@ class TestHTTPAuthRootResource(TestCase):
         root.virtinfo_timeout = 15
         root.reactor = task.Clock()
         root.cgit_secret = None
+
+    def test_translatePath_timeout(self):
+        root = self.root
         request = LessDummyRequest([''])
         request.method = b'GET'
         request.path = b'/example'
@@ -315,6 +334,19 @@ class TestHTTPAuthRootResource(TestCase):
         self.assertEqual(504, request.responseCode)
         self.assertEqual(b'Path translation timed out.', request.value)
 
+    @defer.inlineCallbacks
+    def test_render_root_repo(self):
+        root = self.root
+        request = LessDummyRequest([''])
+        store.init_repo(os.path.join(
+            root.repo_store, self.virtinfo.getInternalPath('/example')))
+        request.method = b'GET'
+        request.path = b'/example'
+        request.uri = b'http://dummy/example'
+        yield render_resource(http.HTTPAuthRootResource(root), request)
+        response_content = b''.join(request.written)
+        self.assertIn(b"Repository seems to be empty", response_content)
+
 
 class TestProtocolVersion(TestCase):
     def test_get_protocol_version_from_request_default_zero(self):