Differences in protocol parsing between Git and Dulwich

[andrewshadura]

This code in dulwich/client.py around line 1018 doesn't match the corresponding code in Git.

        try:
            self.dumb = (not content_type.startswith("application/x-git-"))
            if not self.dumb:
                proto = Protocol(resp.read, None)
                # The first line should mention the service
                pkts = list(proto.read_pkt_seq())
                if pkts != [b'# service=' + service + b'\n']:
                    raise GitProtocolError(
                        "unexpected first line %r from smart server" % pkts)
                return read_pkt_refs(proto)
            else:
                return read_info_refs(resp), set()
        finally:
            resp.close()

The original implementation:

        /*
         * smart HTTP response; validate that the service
         * pkt-line matches our request.
         */
        line = packet_read_line_buf(&last->buf, &last->len, NULL);

        strbuf_reset(&exp);
        strbuf_addf(&exp, "# service=%s", service);
        if (strcmp(line, exp.buf))
            die("invalid server response; got '%s'", line);
        strbuf_release(&exp);

As you can see, Git doesn't strictly require EOL, which currently makes (hg-git fail with Kallithea)[https://bitbucket.org/conservancy/kallithea/issues/230/cannot-perform-push-with-hg-git-mercurial], as Kallithea doesn't sent it. Arguably, it's an incorrect implementation of the protocol on our side, but in any case I think it's better that both Git and Dulwich do the same thing.

[jelmer]

Agreed that Dulwich should behave the same way as C git here.

On 14 July 2016 11:55:51 GMT+02:00, Andrew Shadura notifications@github.com wrote:

This code in dulwich/client.py around line
1018 doesn't
match the corresponding code in Git.

       try:
       self.dumb = (not content_type.startswith("application/x-git-"))
           if not self.dumb:
               proto = Protocol(resp.read, None)
               # The first line should mention the service
               pkts = list(proto.read_pkt_seq())
               if pkts != [b'# service=' + service + b'\n']:
                   raise GitProtocolError(
                  "unexpected first line %r from smart server" % pkts)
               return read_pkt_refs(proto)
           else:
               return read_info_refs(resp), set()
       finally:
           resp.close()

The original implementation:

       /*
        * smart HTTP response; validate that the service
        * pkt-line matches our request.
        */
       line = packet_read_line_buf(&last->buf, &last->len, NULL);

       strbuf_reset(&exp);
       strbuf_addf(&exp, "# service=%s", service);
       if (strcmp(line, exp.buf))
           die("invalid server response; got '%s'", line);
       strbuf_release(&exp);

As you can see, Git doesn't strictly require EOL, which currently makes
(hg-git fail with
Kallithea)[https://bitbucket.org/conservancy/kallithea/issues/230/cannot-perform-push-with-hg-git-mercurial],
as Kallithea doesn't sent it. Arguably, it's an incorrect
implementation of the protocol on our side, but in any case I think
it's better that both Git and Dulwich do the same thing.

---

You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#442

[andrewshadura]

Actually, is also says that in the docco:

# Clients MUST verify the first pkt-line is `# service=$servicename`.
# Servers MUST set $servicename to be the request parameter value.
# Servers SHOULD include an LF at the end of this line.
# Clients MUST ignore an LF at the end of the line.

:)

[jelmer]

Patches to do the comparison after stripping LF characters welcome.

Alternatively, I'm traveling for a couple of weeks but can look into this when I come back.

On 14 July 2016 11:55:51 GMT+02:00, Andrew Shadura notifications@github.com wrote:

This code in dulwich/client.py around line
1018 doesn't
match the corresponding code in Git.

       try:
       self.dumb = (not content_type.startswith("application/x-git-"))
           if not self.dumb:
               proto = Protocol(resp.read, None)
               # The first line should mention the service
               pkts = list(proto.read_pkt_seq())
               if pkts != [b'# service=' + service + b'\n']:
                   raise GitProtocolError(
                  "unexpected first line %r from smart server" % pkts)
               return read_pkt_refs(proto)
           else:
               return read_info_refs(resp), set()
       finally:
           resp.close()

The original implementation:

       /*
        * smart HTTP response; validate that the service
        * pkt-line matches our request.
        */
       line = packet_read_line_buf(&last->buf, &last->len, NULL);

       strbuf_reset(&exp);
       strbuf_addf(&exp, "# service=%s", service);
       if (strcmp(line, exp.buf))
           die("invalid server response; got '%s'", line);
       strbuf_release(&exp);

As you can see, Git doesn't strictly require EOL, which currently makes
(hg-git fail with
Kallithea)[https://bitbucket.org/conservancy/kallithea/issues/230/cannot-perform-push-with-hg-git-mercurial],
as Kallithea doesn't sent it. Arguably, it's an incorrect
implementation of the protocol on our side, but in any case I think
it's better that both Git and Dulwich do the same thing.

---

You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#442