[PATCH v4 00/11] protocol transition
Changes in v4:
* Added more tests for the new handeling of ssh variants.
* Removed the 'default' case in upload_pack and receive_pack and instead
ensured that all enum values were accounted for. This way when a new
protocol version is introduced the compiler will throw an error if the new
protocol version isn't accounted for in these switch statements.
* Added Jonathan's Documentation patch ontop of the series (with the small
change I pointed out in reply to the patch itself)
* A few other small changes due to reviewer comments.
Brandon Williams (9):
pkt-line: add packet_write function
protocol: introduce protocol extension mechanisms
daemon: recognize hidden request arguments
upload-pack, receive-pack: introduce protocol version 1
connect: teach client to recognize v1 server response
connect: tell server that the client understands v1
http: tell server that the client understands v1
i5700: add interop test for protocol transition
ssh: introduce a 'simple' ssh variant
Jonathan Tan (2):
connect: in ref advertisement, shallows are last
Documentation: document Extra Parameters
Documentation/config.txt | 44 +++-
Documentation/git.txt | 15 +-
Documentation/technical/http-protocol.txt | 8 +
Documentation/technical/pack-protocol.txt | 43 +++-
Makefile | 1 +
builtin/receive-pack.c | 17 ++
cache.h | 10 +
connect.c | 354 ++++++++++++++++++++----------
daemon.c | 71 +++++-
http.c | 18 ++
pkt-line.c | 6 +
pkt-line.h | 1 +
protocol.c | 79 +++++++
protocol.h | 33 +++
t/interop/i5700-protocol-transition.sh | 68 ++++++
t/lib-httpd/apache.conf | 7 +
t/t5601-clone.sh | 26 ++-
t/t5700-protocol-v1.sh | 294 +++++++++++++++++++++++++
upload-pack.c | 20 +-
19 files changed, 967 insertions(+), 148 deletions(-)
create mode 100644 protocol.c
create mode 100644 protocol.h
create mode 100755 t/interop/i5700-protocol-transition.sh
create mode 100755 t/t5700-protocol-v1.sh
--- interdiff with 'origin/bw/protocol-v1'
diff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt
index 1c561bdd9..a0e45f288 100644
--- a/Documentation/technical/http-protocol.txt
+++ b/Documentation/technical/http-protocol.txt
@@ -219,6 +219,10 @@ smart server reply:
S: 003c2cb58b79488a98d2721cea644875a8dd0026b115 refs/tags/v1.0\n
S: 003fa3c2e2402b99163d1d59756e5f207ae21cccba4c refs/tags/v1.0^{}\n
+The client may send Extra Parameters (see
+Documentation/technical/pack-protocol.txt) as a colon-separated string
+in the Git-Protocol HTTP header.
+
Dumb Server Response
^^^^^^^^^^^^^^^^^^^^
Dumb servers MUST respond with the dumb server reply format.
@@ -269,7 +273,11 @@ the C locale ordering. The stream SHOULD include the default ref
named `HEAD` as the first ref. The stream MUST include capability
declarations behind a NUL on the first ref.
+The returned response contains "version 1" if "version=1" was sent as an
+Extra Parameter.
+
smart_reply = PKT-LINE("# service=$servicename" LF)
+ *1("version 1")
ref_list
"0000"
ref_list = empty_list / non_empty_list
diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt
index ed1eae8b8..cd31edc91 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -39,6 +39,19 @@ communicates with that invoked process over the SSH connection.
The file:// transport runs the 'upload-pack' or 'receive-pack'
process locally and communicates with it over a pipe.
+Extra Parameters
+----------------
+
+The protocol provides a mechanism in which clients can send additional
+information in its first message to the server. These are called "Extra
+Parameters", and are supported by the Git, SSH, and HTTP protocols.
+
+Each Extra Parameter takes the form of `<key>=<value>` or `<key>`.
+
+Servers that receive any such Extra Parameters MUST ignore all
+unrecognized keys. Currently, the only Extra Parameter recognized is
+"version=1".
+
Git Transport
-------------
@@ -46,18 +59,25 @@ The Git transport starts off by sending the command and repository
on the wire using the pkt-line format, followed by a NUL byte and a
hostname parameter, terminated by a NUL byte.
- 0032git-upload-pack /project.git\0host=myserver.com\0
+ 0033git-upload-pack /project.git\0host=myserver.com\0
+
+The transport may send Extra Parameters by adding an additional NUL
+byte, and then adding one or more NUL-terminated strings:
+
+ 003egit-upload-pack /project.git\0host=myserver.com\0\0version=1\0
--
- git-proto-request = request-command SP pathname NUL [ host-parameter NUL ]
+ git-proto-request = request-command SP pathname NUL
+ [ host-parameter NUL ] [ NUL extra-parameters ]
request-command = "git-upload-pack" / "git-receive-pack" /
"git-upload-archive" ; case sensitive
pathname = *( %x01-ff ) ; exclude NUL
host-parameter = "host=" hostname [ ":" port ]
+ extra-parameters = 1*extra-parameter
+ extra-parameter = 1*( %x01-ff ) NUL
--
-Only host-parameter is allowed in the git-proto-request. Clients
-MUST NOT attempt to send additional parameters. It is used for the
+host-parameter is used for the
git-daemon name based virtual hosting. See --interpolated-path
option to git daemon, with the %H/%CH format characters.
@@ -117,6 +137,12 @@ we execute it without the leading '/'.
v
ssh user@example.com "git-upload-pack '~alice/project.git'"
+Depending on the value of the `protocol.version` configuration variable,
+Git may attempt to send Extra Parameters as a colon-separated string in
+the GIT_PROTOCOL environment variable. This is done only if
+the `ssh.variant` configuration variable indicates that the ssh command
+supports passing environment variables as an argument.
+
A few things to remember here:
- The "command name" is spelled with dash (e.g. git-upload-pack), but
@@ -137,11 +163,13 @@ Reference Discovery
-------------------
When the client initially connects the server will immediately respond
-with a listing of each reference it has (all branches and tags) along
+with a version number (if "version=1" is sent as an Extra Parameter),
+and a listing of each reference it has (all branches and tags) along
with the object name that each reference currently points to.
- $ echo -e -n "0039git-upload-pack /schacon/gitbook.git\0host=example.com\0" |
+ $ echo -e -n "0044git-upload-pack /schacon/gitbook.git\0host=example.com\0\0version=1\0" |
nc -v example.com 9418
+ 000aversion 1
00887217a7c7e582c46cec22a130adf4b9d7d950fba0 HEAD\0multi_ack thin-pack
side-band side-band-64k ofs-delta shallow no-progress include-tag
00441d3fcd5ced445d1abc402225c0b8a1299641f497 refs/heads/integration
@@ -165,7 +193,8 @@ immediately after the ref itself, if presented. A conforming server
MUST peel the ref if it's an annotated tag.
----
- advertised-refs = (no-refs / list-of-refs)
+ advertised-refs = *1("version 1")
+ (no-refs / list-of-refs)
*shallow
flush-pkt
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 94b7d29ea..839c1462d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1966,15 +1966,17 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
switch (determine_protocol_version_server()) {
case protocol_v1:
- if (advertise_refs || !stateless_rpc)
- packet_write_fmt(1, "version 1\n");
/*
* v1 is just the original protocol with a version string,
* so just fall through after writing the version string.
*/
+ if (advertise_refs || !stateless_rpc)
+ packet_write_fmt(1, "version 1\n");
+
+ /* fallthrough */
case protocol_v0:
break;
- default:
+ case protocol_unknown_version:
BUG("unknown protocol version");
}
diff --git a/connect.c b/connect.c
index 65cee49b6..7fbd396b3 100644
--- a/connect.c
+++ b/connect.c
@@ -836,7 +836,8 @@ static enum ssh_variant determine_ssh_variant(const char *ssh_command,
}
}
- if (!strcasecmp(variant, "ssh"))
+ if (!strcasecmp(variant, "ssh") ||
+ !strcasecmp(variant, "ssh.exe"))
ssh_variant = VARIANT_SSH;
else if (!strcasecmp(variant, "plink") ||
!strcasecmp(variant, "plink.exe"))
diff --git a/daemon.c b/daemon.c
index 36cc794c9..e37e343d0 100644
--- a/daemon.c
+++ b/daemon.c
@@ -582,6 +582,9 @@ static void canonicalize_client(struct strbuf *out, const char *in)
/*
* Read the host as supplied by the client connection.
+ *
+ * Returns a pointer to the character after the NUL byte terminating the host
+ * arguemnt, or 'extra_args' if there is no host arguemnt.
*/
static char *parse_host_arg(struct hostinfo *hi, char *extra_args, int buflen)
{
diff --git a/pkt-line.c b/pkt-line.c
index c025d0332..7006b3587 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -188,7 +188,7 @@ static int packet_write_gently(const int fd_out, const char *buf, size_t size)
return 0;
}
-void packet_write(const int fd_out, const char *buf, size_t size)
+void packet_write(int fd_out, const char *buf, size_t size)
{
if (packet_write_gently(fd_out, buf, size))
die_errno("packet write failed");
diff --git a/pkt-line.h b/pkt-line.h
index d9e9783b1..3dad583e2 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -22,7 +22,7 @@
void packet_flush(int fd);
void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
void packet_buf_flush(struct strbuf *buf);
-void packet_write(const int fd_out, const char *buf, size_t size);
+void packet_write(int fd_out, const char *buf, size_t size);
void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
int packet_flush_gently(int fd);
int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index ee1a24c5b..86811a0c3 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -309,21 +309,18 @@ test_expect_success 'clone checking out a tag' '
setup_ssh_wrapper () {
test_expect_success 'setup ssh wrapper' '
cp "$GIT_BUILD_DIR/t/helper/test-fake-ssh$X" \
- "$TRASH_DIRECTORY/ssh-wrapper$X" &&
- GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper$X" &&
+ "$TRASH_DIRECTORY/ssh$X" &&
+ GIT_SSH="$TRASH_DIRECTORY/ssh$X" &&
export GIT_SSH &&
- GIT_SSH_VARIANT=ssh &&
- export GIT_SSH_VARIANT &&
export TRASH_DIRECTORY &&
>"$TRASH_DIRECTORY"/ssh-output
'
}
copy_ssh_wrapper_as () {
- cp "$TRASH_DIRECTORY/ssh-wrapper$X" "${1%$X}$X" &&
+ cp "$TRASH_DIRECTORY/ssh$X" "${1%$X}$X" &&
GIT_SSH="${1%$X}$X" &&
- export GIT_SSH &&
- unset GIT_SSH_VARIANT
+ export GIT_SSH
}
expect_ssh () {
@@ -365,6 +362,22 @@ test_expect_success 'bracketed hostnames are still ssh' '
expect_ssh "-p 123" myhost src
'
+test_expect_success 'OpenSSH variant passes -4' '
+ git clone -4 "[myhost:123]:src" ssh-ipv4-clone &&
+ expect_ssh "-4 -p 123" myhost src
+'
+
+test_expect_success 'variant can be overriden' '
+ git -c ssh.variant=simple clone -4 "[myhost:123]:src" ssh-simple-clone &&
+ expect_ssh myhost src
+'
+
+test_expect_success 'simple is treated as simple' '
+ copy_ssh_wrapper_as "$TRASH_DIRECTORY/simple" &&
+ git clone -4 "[myhost:123]:src" ssh-bracket-clone-simple &&
+ expect_ssh myhost src
+'
+
test_expect_success 'uplink is treated as simple' '
copy_ssh_wrapper_as "$TRASH_DIRECTORY/uplink" &&
git clone "[myhost:123]:src" ssh-bracket-clone-uplink &&
diff --git a/upload-pack.c b/upload-pack.c
index ef438e9c2..ef99a029c 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -1071,16 +1071,18 @@ int cmd_main(int argc, const char **argv)
switch (determine_protocol_version_server()) {
case protocol_v1:
- if (advertise_refs || !stateless_rpc)
- packet_write_fmt(1, "version 1\n");
/*
* v1 is just the original protocol with a version string,
* so just fall through after writing the version string.
*/
+ if (advertise_refs || !stateless_rpc)
+ packet_write_fmt(1, "version 1\n");
+
+ /* fallthrough */
case protocol_v0:
upload_pack();
break;
- default:
+ case protocol_unknown_version:
BUG("unknown protocol version");
}
--
2.15.0.rc0.271.g36b669edcc-goog