launchpad-reviewers team mailing list archive

[Ioana Lasc <ioana.lasc@xxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/turnip:bug-1870080 into turnip:master.

Commit message:
Hide extra params after 2 NUL bytes

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1870080 in turnip: ""git clone git://git.launchpad.net/<anything>" fails with git 2.26.0"
  https://bugs.launchpad.net/turnip/+bug/1870080

For more details, see:
https://code.launchpad.net/~ilasc/turnip/+git/turnip/+merge/381583

Added check for params following 2 NUL bytes to def decode_request.
If extra params are present after the 2 NUL bytes they will be hidden instead of raising exception. 
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/turnip:bug-1870080 into turnip:master.

diff --git a/turnip/pack/helpers.py b/turnip/pack/helpers.py
index 8364464..a6a6dcc 100644
--- a/turnip/pack/helpers.py
+++ b/turnip/pack/helpers.py
@@ -80,13 +80,19 @@ def decode_request(data):
         raise ValueError('Invalid git-proto-request')
     pathname = bits[0]
     params = {}
+    index = 0
     for param in bits[1:-1]:
+        if param == '':
+            if (index < len(bits)-1):
+                # hide extra params afer 2 NUL bytes
+                return (command, pathname, params)
         if b'=' not in param:
             raise ValueError('Parameters must have values')
         name, value = param.split(b'=', 1)
         if name in params:
             raise ValueError('Parameters must not be repeated')
         params[name] = value
+        index += 1
     return (command, pathname, params)
 
 
diff --git a/turnip/pack/tests/test_helpers.py b/turnip/pack/tests/test_helpers.py
index 6f90113..ff32c52 100644
--- a/turnip/pack/tests/test_helpers.py
+++ b/turnip/pack/tests/test_helpers.py
@@ -87,6 +87,27 @@ class TestDecodeRequest(TestCase):
         e = self.assertRaises(ValueError, helpers.decode_request, req)
         self.assertEqual(message, str(e).encode('utf-8'))
 
+    def test_hide_extra_params_after_2_null_bytes(self):
+        # We hide 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': 'git.launchpad.test'}),
+            helpers.decode_request(req))
+
+    def test_rejects_extra_params_without_end_null_bytes(self):
+        # Extra params after 2 must be NUL-terminated
+        self.assertInvalid(
+            b'git-upload-pack /some/path\0host=git.launchpad.test\0\0ver=2',
+            b'Invalid git-proto-request')
+
+    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': 'git.launchpad.test'}),
+            helpers.decode_request(req))
+
     def test_without_parameters(self):
         self.assertEqual(
             (b'git-do-stuff', b'/some/path', {}),