Merge ~pappacena/turnip:http-v2-protocol into turnip:master

[Thiago F. Pappacena]

Accepting git-protocol header for protocol version 2

[Thiago F. Pappacena (pappacena)]

I'll keep this MP as "Work in progress" until I write some functional tests using protocol v2, to make sure it will be compatible with both protocol versions (even if no relevant capability is implemented for v2 right now).

6e8f1a4... by Thiago F. Pappacena on 2020-07-31

Refactoring the way we implement extensions

5132bc1... by Thiago F. Pappacena on 2020-08-04

delegating commands to extension

60583ba... by Thiago F. Pappacena on 2020-08-05

Merge branch 'master' into http-v2-protocol

e126bd5... by Thiago F. Pappacena on 2020-08-05

Decoding v2 protocol extra parameters

5ba7fdf... by Thiago F. Pappacena on 2020-08-05

Merge branch 'decode-v2-protocol-commands' into http-v2-protocol

1b9bf8a... by Thiago F. Pappacena on 2020-08-06

Making the v2 protocol negotiation work, and cleaning up things

797a1ac... by Thiago F. Pappacena on 2020-08-06

ls-refs kind of working

7a76ade... by Thiago F. Pappacena on 2020-08-10

Checkpoint

caefc9d... by Thiago F. Pappacena on 2020-08-10

HTTP v2 working ok-ish

Unmerged commits

caefc9d... by Thiago F. Pappacena on 2020-08-10

HTTP v2 working ok-ish

7a76ade... by Thiago F. Pappacena on 2020-08-10

Checkpoint

797a1ac... by Thiago F. Pappacena on 2020-08-06

ls-refs kind of working

1b9bf8a... by Thiago F. Pappacena on 2020-08-06

Making the v2 protocol negotiation work, and cleaning up things

5ba7fdf... by Thiago F. Pappacena on 2020-08-05

Merge branch 'decode-v2-protocol-commands' into http-v2-protocol

e126bd5... by Thiago F. Pappacena on 2020-08-05

Decoding v2 protocol extra parameters

60583ba... by Thiago F. Pappacena on 2020-08-05

Merge branch 'master' into http-v2-protocol

5132bc1... by Thiago F. Pappacena on 2020-08-04

delegating commands to extension

6e8f1a4... by Thiago F. Pappacena on 2020-07-31

Refactoring the way we implement extensions

b37f9e2... by Thiago F. Pappacena on 2020-07-28

Adding git v2 protocol header for HTTP frontend

diff --git a/config.yaml b/config.yaml
index 99b0e7b..3a75467 100644
--- a/config.yaml
+++ b/config.yaml
@@ -21,3 +21,8 @@ openid_provider_root: https://testopenid.test/
 site_name: git.launchpad.test
 main_site_root: https://launchpad.test/
 celery_broker: pyamqp://guest@localhost//
+
+# Enabled pack extension classes, relative to turnip.pack.extensions. This
+# is only available for v2 protocol. If no extension is enabled, v2
+# compatibility will not be advertised.
+pack_extensions: .agent.Agent .lsrefs.LSRefs .fetch.Fetch
diff --git a/requirements.txt b/requirements.txt
index 89895ea..4c5f050 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -39,7 +39,7 @@ mock==3.0.5; python_version < "3"
 pathlib2==2.3.5
 Paste==2.0.2
 PasteDeploy==2.1.0
-pbr==5.4.4
+pbr==5.4.5
 pep8==1.5.7
 psutil==5.7.0
 pyasn1==0.4.8
@@ -60,7 +60,8 @@ scandir==1.10.0
 setuptools-scm==1.17.0
 simplejson==3.6.5
 six==1.14.0
-testtools==2.3.0
+testscenarios-0.5.0
+testtools==2.4.0
 traceback2==1.4.0
 translationstring==1.3
 Twisted[conch]==20.3.0
diff --git a/setup.py b/setup.py
index 76bf6c2..44507da 100755
--- a/setup.py
+++ b/setup.py
@@ -37,6 +37,7 @@ test_requires = [
     'fixtures',
     'flake8',
     'mock; python_version < "3"',
+    'testscenarios',
     'testtools',
     'webtest',
     ]
diff --git a/turnip/pack/extensions/__init__.py b/turnip/pack/extensions/__init__.py
new file mode 100644
index 0000000..08b685e
--- /dev/null
+++ b/turnip/pack/extensions/__init__.py
@@ -0,0 +1,50 @@
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+from importlib import import_module
+
+import six
+
+from turnip.config import config
+from turnip.pack.helpers import encode_packet, FLUSH_PKT
+from turnip.version_info import version_info
+
+
+def get_available_extensions(version='2'):
+    """Reeturns the list of available PacketExtension classes."""
+    # For now, we only have 1 and 2, but this might change in the future
+    # (2.1? 2.5?), so delay this "int" conversion as much as possible.
+    version = int(version)
+    if version == 1:
+        return []
+    available = []
+    for class_path in config.get('pack_extensions').split():
+        module_path, class_name = class_path.rsplit('.', 1)
+        module = import_module(module_path, 'turnip.pack.extensions')
+        extension_cls = getattr(module, class_name)
+        if extension_cls.min_version > version:
+            continue
+        available.append(extension_cls)
+    return available
+
+
+def get_capabilities_description(version='1'):
+    """Returns the capability advertisement binary string for the given
+    protocol version."""
+    if version != '2':
+        return b""
+    turnip_version = six.ensure_binary(version_info.get("revision_id", '-1'))
+    return (
+        encode_packet(b"version 2\n") +
+        encode_packet(b"agent=turnip/%s\n" % turnip_version) +
+        encode_packet(b"ls-refs\n") +
+        encode_packet(b"fetch=shallow\n") +
+        encode_packet(b"server-option\n") +
+        FLUSH_PKT
+    )
diff --git a/turnip/pack/extensions/agent.py b/turnip/pack/extensions/agent.py
new file mode 100644
index 0000000..3183c01
--- /dev/null
+++ b/turnip/pack/extensions/agent.py
@@ -0,0 +1,26 @@
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+import six
+
+from turnip.pack.extensions.extension import PackExtension
+from turnip.pack.helpers import (
+    encode_packet,
+    )
+from turnip.version_info import version_info
+
+
+class Agent(PackExtension):
+    min_version = 2
+
+    @classmethod
+    def getCapabilityAdvertisement(cls):
+        version = version_info.get('revision_id', '-1')
+        agent = b"agent=turnip/%s\n" % six.ensure_binary(version)
+        return encode_packet(agent)
diff --git a/turnip/pack/extensions/extension.py b/turnip/pack/extensions/extension.py
new file mode 100644
index 0000000..58843c8
--- /dev/null
+++ b/turnip/pack/extensions/extension.py
@@ -0,0 +1,61 @@
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+
+import os
+
+from turnip.api.store import open_repo
+
+
+class PackExtension:
+    # Minimum protocol version where this extension is available.
+    min_version = 2
+
+    # Which command this extension represents (for example, 'ls-refs',
+    # 'fetch', ...)
+    command = None
+
+    # If False, the connection with the client will be closed immediately
+    # after execute() is called.
+    is_async = False
+
+    def __init__(self, backend_protocol=None, path=None, args=None):
+        """Initialize the extension.
+
+        :param backend_protocol: The PackBackendProtocol instance requesting
+            this extension to do its job.
+        """
+        self.backend_protocol = backend_protocol
+        self.path = path
+        self.args = args
+        self.data_buffer = None
+        self.done = False
+
+    def open_repo(self):
+        repo_store = os.path.dirname(self.path)
+        repo_name = os.path.basename(self.path)
+        return open_repo(repo_store, repo_name)
+
+    @property
+    def log(self):
+        return self.backend_protocol.log
+
+    def sendRawData(self, data):
+        self.backend_protocol.sendRawData(data)
+
+    def sendPacket(self, data):
+        self.backend_protocol.sendPacket(data)
+
+    @classmethod
+    def getCapabilityAdvertisement(self):
+        raise NotImplementedError(
+            "This must be implemented by subclasses, returning its binary "
+            "representation")
+
diff --git a/turnip/pack/extensions/fetch.py b/turnip/pack/extensions/fetch.py
new file mode 100644
index 0000000..8a64ae8
--- /dev/null
+++ b/turnip/pack/extensions/fetch.py
@@ -0,0 +1,22 @@
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+from turnip.pack.extensions.extension import PackExtension
+from turnip.pack.helpers import encode_packet, FLUSH_PKT
+
+
+class Fetch(PackExtension):
+    min_version = 2
+    command = b'fetch'
+    is_write_operation = False
+    is_async = False
+
+    @classmethod
+    def getCapabilityAdvertisement(cls):
+        return encode_packet(b"fetch=shallow\n")
diff --git a/turnip/pack/extensions/lsrefs.py b/turnip/pack/extensions/lsrefs.py
new file mode 100644
index 0000000..9a23f03
--- /dev/null
+++ b/turnip/pack/extensions/lsrefs.py
@@ -0,0 +1,22 @@
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+from turnip.pack.extensions.extension import PackExtension
+from turnip.pack.helpers import encode_packet, FLUSH_PKT
+
+
+class LSRefs(PackExtension):
+    min_version = 2
+    command = b'ls-refs'
+    is_write_operation = False
+    is_async = False
+
+    @classmethod
+    def getCapabilityAdvertisement(cls):
+        return encode_packet(b"ls-refs\n")
diff --git a/turnip/pack/extensions/tests/__init__.py b/turnip/pack/extensions/tests/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/turnip/pack/extensions/tests/__init__.py
diff --git a/turnip/pack/extensions/tests/test_extensions.py b/turnip/pack/extensions/tests/test_extensions.py
new file mode 100644
index 0000000..299e786
--- /dev/null
+++ b/turnip/pack/extensions/tests/test_extensions.py
@@ -0,0 +1,42 @@
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+from fixtures import EnvironmentVariable
+from testtools import TestCase
+
+from turnip.pack.extensions import (
+    get_capabilities_description,
+    get_available_extensions,
+    )
+from turnip.pack.extensions.agent import Agent
+from turnip.pack.extensions.lsrefs import LSRefs
+from turnip.version_info import version_info
+
+
+class TestCapabilities(TestCase):
+    def test_get_extensions_from_config(self):
+        self.useFixture(EnvironmentVariable(
+            "PACK_EXTENSIONS", ".agent.Agent .lsrefs.LSRefs"))
+        available = get_available_extensions('2')
+        self.assertEqual(2, len(available))
+        self.assertEqual(available[0], Agent)
+        self.assertEqual(available[1], LSRefs)
+
+    def test_get_capabilities_description_v1_is_empty(self):
+        self.assertEqual(b"", get_capabilities_description(1))
+
+    def test_get_capabilities_description_v2_with_agent_only(self):
+        self.useFixture(EnvironmentVariable(
+            "PACK_EXTENSIONS", ".agent.Agent .lsrefs.LSRefs"))
+        self.assertEqual(
+            b"000eversion 2\n"
+            b"003aagent=turnip/%s\n"
+            b"000cls-refs\n"
+            b"0000" % version_info['revision_id'],
+            get_capabilities_description('2'))
diff --git a/turnip/pack/git.py b/turnip/pack/git.py
index 763d5d6..43cbb32 100644
--- a/turnip/pack/git.py
+++ b/turnip/pack/git.py
@@ -29,6 +29,7 @@ from turnip.api import store
 from turnip.api.store import AlreadyExistsError
 from turnip.config import config
 from turnip.helpers import compose_path
+from turnip.pack.extensions import get_available_extensions
 from turnip.pack.helpers import (
     decode_packet,
     decode_request,
@@ -37,8 +38,8 @@ from turnip.pack.helpers import (
     ensure_config,
     ensure_hooks,
     INCOMPLETE_PKT,
-    translate_xmlrpc_fault,
-    )
+    translate_xmlrpc_fault, FLUSH_PKT, DELIM_PKT,
+)
 
 
 ERROR_PREFIX = b'ERR '
@@ -77,13 +78,17 @@ class UnstoppableProducerWrapper(object):
         pass
 
 
-class PackProtocol(protocol.Protocol):
+class PackProtocol(protocol.Protocol, object):
 
     paused = False
     raw = False
+    extension = None
 
     __buffer = b''
 
+    def getProtocolVersion(self):
+        raise NotImplementedError()
+
     def packetReceived(self, payload):
         raise NotImplementedError()
 
@@ -187,11 +192,11 @@ class PackServerProtocol(PackProxyProtocol):
                 command, pathname, params = decode_request(data)
             except ValueError as e:
                 self.die(str(e).encode('utf-8'))
-                return
+                return None
             self.pauseProducing()
             self.got_request = True
             self.requestReceived(command, pathname, params)
-            return
+            return None
         if data is None:
             self.raw = True
         self.peer.sendPacket(data)
@@ -239,15 +244,20 @@ class GitProcessProtocol(protocol.ProcessProtocol):
 
     _err_buffer = b''
 
-    def __init__(self, peer):
+    def __init__(self, peer, cmd_input=None):
         self.peer = peer
+        self.cmd_input = cmd_input
         self.out_started = False
 
     def connectionMade(self):
         self.peer.setPeer(self)
         self.peer.transport.registerProducer(self, True)
-        self.transport.registerProducer(
-            UnstoppableProducerWrapper(self.peer.transport), True)
+        if not self.cmd_input:
+            self.transport.registerProducer(
+                UnstoppableProducerWrapper(self.peer.transport), True)
+        else:
+            self.transport.write(self.cmd_input)
+            self.loseWriteConnection()
         self.peer.resumeProducing()
 
     def outReceived(self, data):
@@ -415,7 +425,7 @@ class PackProxyServerProtocol(PackServerProtocol):
     def resumeProducing(self):
         # Send our translated request and then open the gate to the client.
         self.sendNextCommand()
-        PackServerProtocol.resumeProducing(self)
+        super(PackProxyServerProtocol, self).resumeProducing()
 
     def readConnectionLost(self):
         # Forward the closed stdin down the stack.
@@ -431,12 +441,14 @@ class PackBackendProtocol(PackServerProtocol):
 
     hookrpc_key = None
     expect_set_symbolic_ref = False
+    extension = None
 
     @defer.inlineCallbacks
     def requestReceived(self, command, raw_pathname, params):
         self.extractRequestMeta(command, raw_pathname, params)
         self.command = command
         self.raw_pathname = raw_pathname
+        self.params = params
         self.path = compose_path(self.factory.root, self.raw_pathname)
         auth_params = self.createAuthParams(params)
 
@@ -457,14 +469,42 @@ class PackBackendProtocol(PackServerProtocol):
             return
 
         write_operation = False
-        if command == b'git-upload-pack':
-            subcmd = b'upload-pack'
-        elif command == b'git-receive-pack':
-            subcmd = b'receive-pack'
-            write_operation = True
+        cmd_input = None
+        send_path_as_option = False
+        cmd_env = {}
+        if params.get('version') != '2':
+            if command == b'git-upload-pack':
+                subcmd = b'upload-pack'
+            elif command == b'git-receive-pack':
+                subcmd = b'receive-pack'
+                write_operation = True
+            else:
+                self.die(b"Invalid command: %s" % command)
         else:
-            self.die(b'Unsupported command in request')
-            return
+            # v2 protocol
+            if command == b'git-upload-pack':
+                self.expectNextCommand()
+                self.transport.loseConnection()
+                return
+            else:
+                subcmd = b'upload-pack'
+                cmd_env["GIT_PROTOCOL"] = 'version=2'
+                send_path_as_option = True
+                params.pop(b'turnip-advertise-refs', None)
+                cmd_input = encode_packet(b"command=%s\n" % command)
+                for capability in params["capabilities"].split(b"\n"):
+                    cmd_input += encode_packet(b"%s\n" % capability)
+                cmd_input += DELIM_PKT
+                ignore_keys = ('capabilities', 'version', 'extra_flags')
+                for k, v in params.items():
+                    k = six.ensure_binary(k)
+                    if k.startswith(b"turnip-") or k in ignore_keys:
+                        continue
+                    for param_value in v.split(b'\n'):
+                        value = (b"" if not param_value
+                                 else b" %s" % param_value)
+                        cmd_input += encode_packet(b"%s%s\n" % (k, value))
+                cmd_input += FLUSH_PKT
 
         args = []
         if params.pop(b'turnip-stateless-rpc', None):
@@ -475,10 +515,14 @@ class PackBackendProtocol(PackServerProtocol):
         self.spawnGit(subcmd,
                       args,
                       write_operation=write_operation,
-                      auth_params=auth_params)
+                      auth_params=auth_params,
+                      send_path_as_option=send_path_as_option,
+                      cmd_env=cmd_env,
+                      cmd_input=cmd_input)
 
     def spawnGit(self, subcmd, extra_args, write_operation=False,
-                 send_path_as_option=False, auth_params=None):
+                 send_path_as_option=False, auth_params=None,
+                 cmd_env=None, cmd_input=None):
         cmd = b'git'
         args = [b'git']
         if send_path_as_option:
@@ -487,6 +531,7 @@ class PackBackendProtocol(PackServerProtocol):
         args.extend(extra_args)
 
         env = {}
+        env.update((cmd_env or {}))
         if write_operation and self.factory.hookrpc_handler:
             # This is a write operation, so prepare config, hooks, the hook
             # RPC server, and the environment variables that link them up.
@@ -499,7 +544,7 @@ class PackBackendProtocol(PackServerProtocol):
             env[b'TURNIP_HOOK_RPC_KEY'] = self.hookrpc_key
 
         self.log.info('Spawning {args}', args=args)
-        self.peer = GitProcessProtocol(self)
+        self.peer = GitProcessProtocol(self, cmd_input)
         self.spawnProcess(cmd, args, env=env)
 
     def spawnProcess(self, cmd, args, env=None):
@@ -628,7 +673,7 @@ class PackVirtServerProtocol(PackProxyServerProtocol):
     @defer.inlineCallbacks
     def requestReceived(self, command, pathname, params):
         self.extractRequestMeta(command, pathname, params)
-        permission = 'read' if command == b'git-upload-pack' else 'write'
+        permission = 'read' if command != b'git-receive-pack' else 'write'
         proxy = xmlrpc.Proxy(self.factory.virtinfo_endpoint, allowNone=True)
         try:
             auth_params = self.createAuthParams(params)
diff --git a/turnip/pack/helpers.py b/turnip/pack/helpers.py
index 9cf411d..52649a9 100644
--- a/turnip/pack/helpers.py
+++ b/turnip/pack/helpers.py
@@ -7,6 +7,8 @@ from __future__ import (
     unicode_literals,
     )
 
+from collections import OrderedDict
+
 import enum
 import hashlib
 import os.path
@@ -25,6 +27,8 @@ import yaml
 import turnip.pack.hooks
 
 
+FLUSH_PKT = b'0000'
+DELIM_PKT = b'0001'
 PKT_LEN_SIZE = 4
 PKT_PAYLOAD_MAX = 65520
 INCOMPLETE_PKT = object()
@@ -33,7 +37,7 @@ INCOMPLETE_PKT = object()
 def encode_packet(payload):
     if payload is None:
         # flush-pkt.
-        return b'0000'
+        return FLUSH_PKT
     else:
         # data-pkt
         if len(payload) > PKT_PAYLOAD_MAX:
@@ -47,21 +51,37 @@ def decode_packet(input):
     """Consume a packet, returning the payload and any unconsumed tail."""
     if len(input) < PKT_LEN_SIZE:
         return (INCOMPLETE_PKT, input)
-    if input.startswith(b'0000'):
+    if input.startswith(FLUSH_PKT):
         # flush-pkt
         return (None, input[PKT_LEN_SIZE:])
-    else:
-        # data-pkt
-        try:
-            pkt_len = int(input[:PKT_LEN_SIZE], 16)
-        except ValueError:
-            pkt_len = 0
-        if not (PKT_LEN_SIZE <= pkt_len <= (PKT_LEN_SIZE + PKT_PAYLOAD_MAX)):
-            raise ValueError("Invalid pkt-len")
-        if len(input) < pkt_len:
-            # Some of the packet is yet to be received.
-            return (INCOMPLETE_PKT, input)
-        return (input[PKT_LEN_SIZE:pkt_len], input[pkt_len:])
+    # data-pkt
+    try:
+        pkt_len = int(input[:PKT_LEN_SIZE], 16)
+    except ValueError:
+        pkt_len = 0
+    if not (PKT_LEN_SIZE <= pkt_len <= (PKT_LEN_SIZE + PKT_PAYLOAD_MAX)):
+        raise ValueError("Invalid pkt-len")
+    if len(input) < pkt_len:
+        # Some of the packet is yet to be received.
+        return (INCOMPLETE_PKT, input)
+
+    # v2 protocol "hides" extra parameters after the end of the packet.
+    if len(input) > pkt_len and b'version=2\x00' in input:
+        if FLUSH_PKT not in input:
+            return INCOMPLETE_PKT, input
+        end = input.index(FLUSH_PKT)
+        return input[PKT_LEN_SIZE:end], input[end + len(FLUSH_PKT):]
+
+    return (input[PKT_LEN_SIZE:pkt_len], input[pkt_len:])
+
+
+def decode_packet_list(data):
+    remaining = data
+    retval = []
+    while remaining:
+        pkt, remaining = decode_packet(remaining)
+        retval.append(pkt)
+    return retval
 
 
 def decode_request(data):
@@ -77,10 +97,11 @@ def decode_request(data):
     bits = rest.split(b'\0')
     # Following the command is a pathname, then any number of named
     # parameters. Each of these is NUL-terminated.
-    if len(bits) < 2 or bits[-1] != b'':
+    # After that, v1 should end (v2 might have extra commands).
+    if len(bits) < 2 or (b'version=2' not in bits and bits[-1] != b''):
         raise ValueError('Invalid git-proto-request')
     pathname = bits[0]
-    params = {}
+    params = OrderedDict()
     for index, param in enumerate(bits[1:-1]):
         if param == b'':
             if (index < len(bits) - 1):
@@ -94,7 +115,42 @@ def decode_request(data):
         if name in params:
             raise ValueError('Parameters must not be repeated')
         params[name] = value
-    return (command, pathname, params)
+
+    if not bits[-1]:
+        return command, pathname, params
+    else:
+        # Parse v2 extra commands.
+        cmd, remaining = decode_packet(bits[-1])
+        cmd = cmd.split(b'=', 1)[-1].strip()
+        capabilities, args = remaining.split(DELIM_PKT)
+        cmd_params = OrderedDict({
+            b"capabilities": decode_packet_list(capabilities)})
+        cmd_params.update(params)
+        for arg in decode_packet_list(args):
+            if arg is None:
+                continue
+            arg = arg.strip('\n')
+            if b' ' in arg:
+                k, v = arg.split(b' ', 1)
+                if k not in cmd_params:
+                    cmd_params[k] = []
+                cmd_params[k].append(v)
+            else:
+                cmd_params[arg] = b""
+        return (cmd, pathname, cmd_params)
+
+
+def get_encoded_value(value):
+    """Encode a value for serialization on encode_request"""
+    if value is None:
+        return b''
+    if isinstance(value, list):
+        if any(b'\n' in i for i in value):
+            raise ValueError('Metacharacter in list argument')
+        return b"\n".join(get_encoded_value(i) for i in value)
+    if isinstance(value, bool):
+        return b'1' if value else b'0'
+    return six.ensure_binary(value)
 
 
 def encode_request(command, pathname, params):
@@ -105,9 +161,9 @@ def encode_request(command, pathname, params):
     if b' ' in command or b'\0' in pathname:
         raise ValueError('Metacharacter in arguments')
     bits = [pathname]
-    for name in sorted(params):
+    for name in params:
         value = params[name]
-        value = six.ensure_binary(value) if value is not None else b''
+        value = get_encoded_value(value)
         name = six.ensure_binary(name)
         if b'=' in name or b'\0' in name + value:
             raise ValueError('Metacharacter in arguments')
diff --git a/turnip/pack/http.py b/turnip/pack/http.py
index c0becf0..7ae8765 100644
--- a/turnip/pack/http.py
+++ b/turnip/pack/http.py
@@ -12,6 +12,7 @@ import json
 import os.path
 import tempfile
 import textwrap
+
 try:
     from urllib.parse import urlencode
 except ImportError:
@@ -29,6 +30,7 @@ from paste.auth.cookie import (
     decode as decode_cookie,
     encode as encode_cookie,
     )
+import six
 from twisted.internet import (
     defer,
     error,
@@ -49,6 +51,7 @@ from twisted.web import (
     )
 
 from turnip.helpers import compose_path
+from turnip.pack.extensions import get_capabilities_description
 from turnip.pack.git import (
     ERROR_PREFIX,
     PackProtocol,
@@ -80,6 +83,16 @@ def fail_request(request, message, code=http.INTERNAL_SERVER_ERROR):
     return b''
 
 
+def get_protocol_version_from_request(request):
+    version_header = request.requestHeaders.getRawHeaders(
+        'git-protocol', ['version=1'])[0]
+    try:
+        return version_header.split('version=', 1)[1]
+    except Exception:
+        pass
+    return 1
+
+
 class HTTPPackClientProtocol(PackProtocol):
     """Abstract bridge between a Git pack connection and a smart HTTP request.
 
@@ -99,7 +112,11 @@ class HTTPPackClientProtocol(PackProtocol):
 
     def startGoodResponse(self):
         """Prepare the HTTP response for forwarding from the backend."""
-        raise NotImplementedError()
+        self.factory.http_request.write(
+            get_capabilities_description(self.getProtocolVersion()))
+
+    def getProtocolVersion(self):
+        return get_protocol_version_from_request(self.factory.http_request)
 
     def backendConnected(self):
         """Called when the backend is connected and has sent a good packet."""
@@ -209,12 +226,14 @@ class HTTPPackClientRefsProtocol(HTTPPackClientProtocol):
         self.factory.http_request.setHeader(
             b'Content-Type',
             b'application/x-%s-advertisement' % self.factory.command)
+        super(HTTPPackClientRefsProtocol, self).startGoodResponse()
 
     def backendConnected(self):
         HTTPPackClientProtocol.backendConnected(self)
-        self.rawDataReceived(
-            encode_packet(b'# service=%s\n' % self.factory.command))
-        self.rawDataReceived(encode_packet(None))
+        if self.getProtocolVersion() != b'2':
+            self.rawDataReceived(
+                encode_packet(b'# service=%s\n' % self.factory.command))
+            self.rawDataReceived(encode_packet(None))
 
 
 class HTTPPackClientCommandProtocol(HTTPPackClientProtocol):
@@ -225,6 +244,7 @@ class HTTPPackClientCommandProtocol(HTTPPackClientProtocol):
         self.factory.http_request.setHeader(
             b'Content-Type',
             b'application/x-%s-result' % self.factory.command)
+        # super(HTTPPackClientCommandProtocol, self).startGoodResponse()
 
 
 class HTTPPackClientFactory(protocol.ClientFactory):
@@ -279,9 +299,24 @@ class BaseSmartHTTPResource(resource.Resource):
         The turnip-authenticated-* parameters are set to the values returned
         by the virt service, if any.
         """
+        # where = content.tell()
+        # content.seek(0)
+        # data = content.read()
+        # content.seek(where)
+        # print(b"req#%s will connect to backend" % (id(request), ))
+        # print("--- body ---\n", data, "\n--- endbody ---")
+        # # import ipdb; ipdb.set_trace()
+        # orig = request.write
+        # def x(data):
+        #     print(b"\n\nReq#%s %s %s -> user\n---%s----" % (
+        #         id(request), request.method, request.path, data))
+        #     orig(data)
+        # request.write = x
         params = {
             b'turnip-can-authenticate': b'yes',
             b'turnip-request-id': str(uuid.uuid4()),
+            b'version': six.ensure_binary(
+                get_protocol_version_from_request(request))
             }
         authenticated_params = yield self.authenticateUser(request)
         for key, value in authenticated_params.items():
diff --git a/turnip/pack/tests/test_functional.py b/turnip/pack/tests/test_functional.py
index d6aaa7f..2809593 100644
--- a/turnip/pack/tests/test_functional.py
+++ b/turnip/pack/tests/test_functional.py
@@ -35,6 +35,7 @@ from fixtures import (
     TempDir,
     )
 from pygit2 import GIT_OID_HEX_ZERO
+from testscenarios.testcase import WithScenarios
 from testtools import TestCase
 from testtools.content import text_content
 from testtools.deferredruntest import AsynchronousDeferredRunTest
@@ -77,10 +78,14 @@ from turnip.pack.tests.fake_servers import (
 from turnip.version_info import version_info
 
 
-class FunctionalTestMixin(object):
+class FunctionalTestMixin(WithScenarios):
 
     run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=30)
 
+    scenarios = [
+        ('v1 protocol', {"protocol_version": "1"}),
+        ('v2 protocol', {"protocol_version": "2"})]
+
     def startVirtInfo(self):
         # Set up a fake virt information XML-RPC server which just
         # maps paths to their SHA-256 hash.
@@ -120,8 +125,13 @@ class FunctionalTestMixin(object):
 
     @defer.inlineCallbacks
     def assertCommandSuccess(self, command, path='.'):
+        if command[0] == b'git' and self.protocol_version == '2':
+            args = list(command[1:])
+            command = [b'git', b'-c', b'protocol.version=2'] + args
+        env = {"GIT_TRACE_PACKET": '1'}
+        env.update(os.environ)
         out, err, code = yield utils.getProcessOutputAndValue(
-            command[0], command[1:], env=os.environ, path=path)
+            command[0], command[1:], env=env, path=path)
         if code != 0:
             self.addDetail('stdout', text_content(out))
             self.addDetail('stderr', text_content(err))
@@ -130,6 +140,9 @@ class FunctionalTestMixin(object):
 
     @defer.inlineCallbacks
     def assertCommandFailure(self, command, path='.'):
+        if self.protocol_version == '2':
+            args = list(command[1:])
+            command = [b'git', b'-c', b'protocol.version=2'] + args
         out, err, code = yield utils.getProcessOutputAndValue(
             command[0], command[1:], env=os.environ, path=path)
         if code == 0:
diff --git a/turnip/pack/tests/test_helpers.py b/turnip/pack/tests/test_helpers.py
index a7a3870..8d09c98 100644
--- a/turnip/pack/tests/test_helpers.py
+++ b/turnip/pack/tests/test_helpers.py
@@ -23,7 +23,7 @@ from testtools import TestCase
 
 from turnip.pack import helpers
 import turnip.pack.hooks
-
+from turnip.pack.helpers import FLUSH_PKT
 
 TEST_DATA = b'0123456789abcdef'
 TEST_PKT = b'00140123456789abcdef'
@@ -91,8 +91,8 @@ class TestDecodeRequest(TestCase):
         # We parse extra params behind 2 NUL bytes
         req = b'git-upload-pack /test_repo\0host=git.launchpad.test\0\0ver=2\0'
         self.assertEqual(
-            (b'git-upload-pack', b'/test_repo',
-                {b'host': b'git.launchpad.test', b'ver': b'2'}),
+            [(b'git-upload-pack', b'/test_repo',
+                {b'host': b'git.launchpad.test', b'ver': b'2'})],
             helpers.decode_request(req))
 
     def test_parse_multiple_extra_params_after_2_null_bytes(self):
@@ -102,9 +102,9 @@ class TestDecodeRequest(TestCase):
                b'ver=2\0\0param2=value2\0\0param3=value3\0'
                )
         self.assertEqual(
-            (b'git-upload-pack', b'/test_repo',
+            [(b'git-upload-pack', b'/test_repo',
                 {b'host': b'git.launchpad.test', b'ver': b'2',
-                 b'param2': b'value2', b'param3': b'value3'}),
+                 b'param2': b'value2', b'param3': b'value3'})],
             helpers.decode_request(req))
 
     def test_rejects_extra_param_without_end_null_bytes(self):
@@ -124,25 +124,25 @@ class TestDecodeRequest(TestCase):
     def test_allow_2_end_nul_bytes(self):
         req = b'git-upload-pack /test_repo\0host=git.launchpad.test\0\0'
         self.assertEqual(
-            (b'git-upload-pack', b'/test_repo',
-                {b'host': b'git.launchpad.test'}),
+            [(b'git-upload-pack', b'/test_repo',
+                {b'host': b'git.launchpad.test'})],
             helpers.decode_request(req))
 
     def test_without_parameters(self):
         self.assertEqual(
-            (b'git-do-stuff', b'/some/path', {}),
+            [(b'git-do-stuff', b'/some/path', {})],
             helpers.decode_request(b'git-do-stuff /some/path\0'))
 
     def test_with_host_parameter(self):
         self.assertEqual(
-            (b'git-do-stuff', b'/some/path', {b'host': b'example.com'}),
+            [(b'git-do-stuff', b'/some/path', {b'host': b'example.com'})],
             helpers.decode_request(
                 b'git-do-stuff /some/path\0host=example.com\0'))
 
     def test_with_host_and_user_parameters(self):
         self.assertEqual(
-            (b'git-do-stuff', b'/some/path',
-             {b'host': b'example.com', b'user': b'foo=bar'}),
+            [(b'git-do-stuff', b'/some/path',
+             {b'host': b'example.com', b'user': b'foo=bar'})],
             helpers.decode_request(
                 b'git-do-stuff /some/path\0host=example.com\0user=foo=bar\0'))
 
@@ -173,6 +173,33 @@ class TestDecodeRequest(TestCase):
             b'git-do-stuff /foo\0host=foo\0host=bar\0',
             b'Parameters must not be repeated')
 
+    def test_v2_extra_commands(self):
+        data = (
+            b"git-upload-pack b306d" +
+            b"\x00turnip-x=yes\x00turnip-request-id=123\x00" +
+            b"version=2\x000014command=ls-refs\n0014agent=git/2.25.1" +
+            b'00010009peel\n000csymrefs\n0014ref-prefix HEAD\n' +
+            b'001bref-prefix refs/heads/\n'
+            b'001aref-prefix refs/tags/\n0000' +
+            FLUSH_PKT)
+
+        decoded = helpers.decode_request(data)
+        self.assertEqual(2, len(decoded))
+        first, second = decoded
+        self.assertEqual((b'ls-refs', b'b306d', {
+            b'turnip-x': b'yes',
+            b'turnip-request-id': b'123',
+            b'version': b'2',
+            b'capabilities': [b'agent=git/2.25.1'],
+            b'symrefs': True,
+            b'peel': True,
+            b'ref-prefix': [b'HEAD', b'refs/heads/', b'refs/tags/']
+            }), first)
+        self.assertEqual((b'git-upload-pack', b'b306d', {
+            b'turnip-x': b'yes',
+            b'turnip-request-id': b'123',
+            b'version': b'2'}), second)
+
 
 class TestEncodeRequest(TestCase):
     """Test git-proto-request encoding."""
diff --git a/turnip/pack/tests/test_http.py b/turnip/pack/tests/test_http.py
index 26e2754..0ce06f5 100644
--- a/turnip/pack/tests/test_http.py
+++ b/turnip/pack/tests/test_http.py
@@ -9,6 +9,7 @@ from __future__ import (
 
 from io import BytesIO
 
+from fixtures import EnvironmentVariable
 from testtools import TestCase
 from testtools.deferredruntest import AsynchronousDeferredRunTest
 from twisted.internet import (
@@ -24,7 +25,10 @@ from turnip.pack import (
     helpers,
     http,
     )
+from turnip.pack.helpers import encode_packet
 from turnip.pack.tests.fake_servers import FakeVirtInfoService
+from turnip.tests.compat import mock
+from turnip.version_info import version_info
 
 
 class LessDummyRequest(requesthelper.DummyRequest):
@@ -74,12 +78,14 @@ class FakeRoot(object):
 
     def __init__(self):
         self.backend_transport = None
+        self.client_factory = None
         self.backend_connected = defer.Deferred()
 
     def authenticateWithPassword(self, user, password):
         return {}
 
     def connectToBackend(self, client_factory):
+        self.client_factory = client_factory
         self.backend_transport = testing.StringTransportWithDisconnection()
         p = client_factory.buildProtocol(None)
         self.backend_transport.protocol = p
@@ -186,6 +192,33 @@ class TestSmartHTTPRefsResource(ErrorTestMixin, TestCase):
             'And I am raw, since we got a good packet to start with.',
             self.request.value)
 
+    @defer.inlineCallbacks
+    def test_good_v2_included_version_and_capabilities(self):
+        self.useFixture(EnvironmentVariable("PACK_EXTENSIONS", ".agent.Agent"))
+        self.request.requestHeaders.addRawHeader("Git-Protocol", "version=2")
+        yield self.performRequest(
+            helpers.encode_packet(b'I am git protocol data.') +
+            b'And I am raw, since we got a good packet to start with.')
+        self.assertEqual(200, self.request.responseCode)
+        self.assertEqual(self.root.client_factory.params, {
+            'version': '2',
+            'turnip-advertise-refs': 'yes',
+            'turnip-can-authenticate': 'yes',
+            'turnip-request-id': mock.ANY,
+            'turnip-stateless-rpc': 'yes'})
+
+        capabilities = (
+            '000eversion 2\n' +
+            encode_packet(
+                'agent=turnip/%s\n' % version_info["revision_id"]) +
+            '0000\n')
+        self.assertEqual(
+            capabilities +
+            '001e# service=git-upload-pack\n'
+            '0000001bI am git protocol data.'
+            'And I am raw, since we got a good packet to start with.',
+            self.request.value)
+
 
 class TestSmartHTTPCommandResource(ErrorTestMixin, TestCase):