Re: [PATCH 2/3] remote-curl: tighten "version 2" check for smart-http

[Josh Steadmon]

On 2018.11.16 03:48, Jeff King wrote:
> In a v2 smart-http conversation, the server should reply to our initial
> request with a pkt-line saying "version 2" (this is the start of the
> "capabilities advertisement"). We check for the string using
> starts_with(), but that's overly permissive (it would match "version
> 20", for example).
> 
> Let's tighten this check to use strcmp(). Note that we don't need to
> worry about a trailing newline here, because the ptk-line code will have
> chomped it for us already.
> 
> Signed-off-by: Jeff King 
> ---
>  remote-curl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/remote-curl.c b/remote-curl.c
> index dd9bc41aa1..3c9c4a07c3 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -375,7 +375,7 @@ static void check_smart_http(struct discovery *d, const 
> char *service,
>   d->len = src_len;
>   d->proto_git = 1;
>  
> - } else if (starts_with(line, "version 2")) {
> + } else if (!strcmp(line, "version 2")) {
>   /*
>* v2 smart http; do not consume version packet, which will
>* be handled elsewhere.
> -- 
> 2.19.1.1636.gc7a073d580
> 

Looks good to me.

Reviewed-by: Josh Steadmon 

Re: [PATCH 1/3] remote-curl: refactor smart-http discovery

[Josh Steadmon]

On 2018.11.16 03:47, Jeff King wrote:
> After making initial contact with an http server, we have to decide if
> the server supports smart-http, and if so, which version. Our rules are
> a bit inconsistent:
> 
>   1. For v0, we require that the content-type indicates a smart-http
>  response. We also require the response to look vaguely like a
>  pkt-line starting with "#". If one of those does not match, we fall
>  back to dumb-http.
> 
>  But according to our http protocol spec[1]:
> 
>Dumb servers MUST NOT return a return type starting with
>`application/x-git-`.
> 
>  If we see the expected content-type, we should consider it
>  smart-http. At that point we can parse the pkt-line for real, and
>  complain if it is not syntactically valid.
> 
>   2. For v2, we do not actually check the content-type. Our v2 protocol
>  spec says[2]:
> 
>When using the http:// or https:// transport a client makes a
>"smart" info/refs request as described in `http-protocol.txt`[...]
> 
>  and the http spec is clear that for a smart-http[3]:
> 
>The Content-Type MUST be `application/x-$servicename-advertisement`.
> 
>  So it is required according to the spec.
> 
> These inconsistencies were easy to miss because of the way the original
> code was written as an inline conditional. Let's pull it out into its
> own function for readability, and improve a few things:
> 
>  - we now predicate the smart/dumb decision entirely on the presence of
>the correct content-type
> 
>  - we do a real pkt-line parse before deciding how to proceed (and die
>if it isn't valid)
> 
>  - use skip_prefix() for comparing service strings, instead of
>constructing expected output in a strbuf; this avoids dealing with
>memory cleanup
> 
> Note that this _is_ tightening what the client will allow. It's all
> according to the spec, but it's possible that other implementations
> might violate these. However, violating these particular rules seems
> like an odd choice for a server to make.
> 
> [1] Documentation/technical/http-protocol.txt, l. 166-167
> [2] Documentation/technical/protocol-v2.txt, l. 63-64
> [3] Documentation/technical/http-protocol.txt, l. 247
> 
> Helped-by: Josh Steadmon 
> Signed-off-by: Jeff King 
> ---
>  remote-curl.c | 93 ---
>  1 file changed, 59 insertions(+), 34 deletions(-)
> 
> diff --git a/remote-curl.c b/remote-curl.c
> index 762a55a75f..dd9bc41aa1 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -330,9 +330,65 @@ static int get_protocol_http_header(enum 
> protocol_version version,
>   return 0;
>  }
>  
> +static void check_smart_http(struct discovery *d, const char *service,
> +  struct strbuf *type)
> +{
> + char *src_buf;
> + size_t src_len;
> + char *line;
> + const char *p;
> +
> + /*
> +  * If we don't see x-$service-advertisement, then it's not smart-http.
> +  * But once we do, we commit to it and assume any other protocol
> +  * violations are hard errors.
> +  */
> + if (!skip_prefix(type->buf, "application/x-", ) ||
> + !skip_prefix(p, service, ) ||
> + strcmp(p, "-advertisement"))
> + return;
> +
> + /*
> +  * "Peek" at the first packet by using a separate buf/len pair; some
> +  * cases below require us leaving the originals intact.
> +  */
> + src_buf = d->buf;
> + src_len = d->len;
> + line = packet_read_line_buf(_buf, _len, NULL);
> + if (!line)
> + die("invalid server response; expected service, got flush 
> packet");
> +
> + if (skip_prefix(line, "# service=", ) && !strcmp(p, service)) {
> + /*
> +  * The header can include additional metadata lines, up
> +  * until a packet flush marker.  Ignore these now, but
> +  * in the future we might start to scan them.
> +  */
> + while (packet_read_line_buf(_buf, _len, NULL))
> + ;
> +
> + /*
> +  * v0 smart http; callers expect us to soak up the
> +  * service and header packets
> +  */
> + d->buf = src_buf;
> + d->len = src_len;
> + d->proto_git = 1;
> +
> + } else if (starts_with(line, "version 2")) {
> + /*
> +  * v2 smart http; do not consume version packet, which will
> +  * be handled elsewhere

Re: [PATCH 0/3] remote-curl smart-http discovery cleanup

[Josh Steadmon]

On 2018.11.16 03:44, Jeff King wrote:
[...]
> Amusingly, this does break the test you just added, because it tries to
> issue an ERR after claiming "text/html" (and after my patch, we
> correctly fall back to dumb-http).

Heh yeah, I copied the script from a dumb-http test without reading the
spec.

[PATCH 2/3] remote-curl: tighten "version 2" check for smart-http

[Jeff King]

In a v2 smart-http conversation, the server should reply to our initial
request with a pkt-line saying "version 2" (this is the start of the
"capabilities advertisement"). We check for the string using
starts_with(), but that's overly permissive (it would match "version
20", for example).

Let's tighten this check to use strcmp(). Note that we don't need to
worry about a trailing newline here, because the ptk-line code will have
chomped it for us already.

Signed-off-by: Jeff King 
---
 remote-curl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/remote-curl.c b/remote-curl.c
index dd9bc41aa1..3c9c4a07c3 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -375,7 +375,7 @@ static void check_smart_http(struct discovery *d, const 
char *service,
d->len = src_len;
d->proto_git = 1;
 
-   } else if (starts_with(line, "version 2")) {
+   } else if (!strcmp(line, "version 2")) {
/*
 * v2 smart http; do not consume version packet, which will
 * be handled elsewhere.
-- 
2.19.1.1636.gc7a073d580

[PATCH 1/3] remote-curl: refactor smart-http discovery

[Jeff King]

After making initial contact with an http server, we have to decide if
the server supports smart-http, and if so, which version. Our rules are
a bit inconsistent:

  1. For v0, we require that the content-type indicates a smart-http
 response. We also require the response to look vaguely like a
 pkt-line starting with "#". If one of those does not match, we fall
 back to dumb-http.

 But according to our http protocol spec[1]:

   Dumb servers MUST NOT return a return type starting with
   `application/x-git-`.

 If we see the expected content-type, we should consider it
 smart-http. At that point we can parse the pkt-line for real, and
 complain if it is not syntactically valid.

  2. For v2, we do not actually check the content-type. Our v2 protocol
 spec says[2]:

   When using the http:// or https:// transport a client makes a
   "smart" info/refs request as described in `http-protocol.txt`[...]

 and the http spec is clear that for a smart-http[3]:

   The Content-Type MUST be `application/x-$servicename-advertisement`.

 So it is required according to the spec.

These inconsistencies were easy to miss because of the way the original
code was written as an inline conditional. Let's pull it out into its
own function for readability, and improve a few things:

 - we now predicate the smart/dumb decision entirely on the presence of
   the correct content-type

 - we do a real pkt-line parse before deciding how to proceed (and die
   if it isn't valid)

 - use skip_prefix() for comparing service strings, instead of
   constructing expected output in a strbuf; this avoids dealing with
   memory cleanup

Note that this _is_ tightening what the client will allow. It's all
according to the spec, but it's possible that other implementations
might violate these. However, violating these particular rules seems
like an odd choice for a server to make.

[1] Documentation/technical/http-protocol.txt, l. 166-167
[2] Documentation/technical/protocol-v2.txt, l. 63-64
[3] Documentation/technical/http-protocol.txt, l. 247

Helped-by: Josh Steadmon 
Signed-off-by: Jeff King 
---
 remote-curl.c | 93 ---
 1 file changed, 59 insertions(+), 34 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 762a55a75f..dd9bc41aa1 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -330,9 +330,65 @@ static int get_protocol_http_header(enum protocol_version 
version,
return 0;
 }
 
+static void check_smart_http(struct discovery *d, const char *service,
+struct strbuf *type)
+{
+   char *src_buf;
+   size_t src_len;
+   char *line;
+   const char *p;
+
+   /*
+* If we don't see x-$service-advertisement, then it's not smart-http.
+* But once we do, we commit to it and assume any other protocol
+* violations are hard errors.
+*/
+   if (!skip_prefix(type->buf, "application/x-", ) ||
+   !skip_prefix(p, service, ) ||
+   strcmp(p, "-advertisement"))
+   return;
+
+   /*
+* "Peek" at the first packet by using a separate buf/len pair; some
+* cases below require us leaving the originals intact.
+*/
+   src_buf = d->buf;
+   src_len = d->len;
+   line = packet_read_line_buf(_buf, _len, NULL);
+   if (!line)
+   die("invalid server response; expected service, got flush 
packet");
+
+   if (skip_prefix(line, "# service=", ) && !strcmp(p, service)) {
+   /*
+* The header can include additional metadata lines, up
+* until a packet flush marker.  Ignore these now, but
+* in the future we might start to scan them.
+*/
+   while (packet_read_line_buf(_buf, _len, NULL))
+   ;
+
+   /*
+* v0 smart http; callers expect us to soak up the
+* service and header packets
+*/
+   d->buf = src_buf;
+   d->len = src_len;
+   d->proto_git = 1;
+
+   } else if (starts_with(line, "version 2")) {
+   /*
+* v2 smart http; do not consume version packet, which will
+* be handled elsewhere.
+*/
+   d->proto_git = 1;
+
+   } else {
+   die("invalid server response; got '%s'", line);
+   }
+}
+
 static struct discovery *discover_refs(const char *service, int for_push)
 {
-   struct strbuf exp = STRBUF_INIT;
struct strbuf type = STRBUF_INIT;
struct strbuf charset = STRBUF_INIT;
struct strbuf buffer = STRBUF_INIT;
@@ -405,38 +461,8 @@ static struct discovery *discover_refs(const char 
*service, int for_push)
last->buf_alloc = str

[PATCH 0/3] remote-curl smart-http discovery cleanup

[Jeff King]

On Thu, Nov 15, 2018 at 01:51:52PM -0800, Josh Steadmon wrote:

> > This patch tightens both of those (I also made a few stylistic tweaks,
> > and added the ERR condition to show where it would go). I dunno. Part of
> > me sees this as a nice cleanup, but maybe it is better to just leave it
> > alone. A lot of these behaviors are just how it happens to work now, and
> > not part of the spec, but we don't know what might be relying on them.
> 
> At least according to the protocol-v2 and http-protocol docs, the
> stricter behavior seems correct:
> 
> For the first point above, dumb servers should never use an
> "application/x-git-*" content type (http-protocol.txt line 163-167).
> 
> For the second point, the docs require v2 servers to use
> "application/x-git-*" content types. protocol-v2.txt lines 63-65 state
> that v2 clients should make a smart http request, while
> http-protocol.txt lines 247-252 state that a smart server's response
> type must be "application/x-git-*".

Thanks for digging into the spec. I agree that it's pretty clear that
the appropriate content-type is expected.

> Of course we don't know if other implementations follow the spec, but
> ISTM that this patch at least doesn't contradict how we've promised the
> protocols should work.

These seem like pretty unlikely ways for a buggy server to behave, so I
think it's a reasonable risk. I also checked GitHub's implementation
(which recently learned to speak v2) and made sure that it works. :)
I didn't check JGit, but given the provenance, I assume it's fine.

Amusingly, this does break the test you just added, because it tries to
issue an ERR after claiming "text/html" (and after my patch, we
correctly fall back to dumb-http).

> If no one has any objections, I'll include the diff below in v2. Thanks
> for the help Jeff!

I think it makes sense to do the refactoring first as a separate step.
And of course it needs a commit message. So how about this series (your
original is rebased on top)?

  [1/3]: remote-curl: refactor smart-http discovery
  [2/3]: remote-curl: tighten "version 2" check for smart-http
  [3/3]: remote-curl: die on server-side errors

 remote-curl.c   | 96 +
 t/lib-httpd.sh  |  1 +
 t/lib-httpd/apache.conf |  4 ++
 t/lib-httpd/error-smart-http.sh |  3 ++
 t/t5551-http-fetch-smart.sh |  5 ++
 5 files changed, 75 insertions(+), 34 deletions(-)
 create mode 100644 t/lib-httpd/error-smart-http.sh

-Peff

Re: [PATCH] smart-http: document flush after "# service" line

[Jeff King]

On Sat, Mar 03, 2018 at 03:28:47AM -0500, Eric Sunshine wrote:

> On Sat, Mar 3, 2018 at 12:27 AM, Jeff King <p...@peff.net> wrote:
> > Subject: smart-http: document flush after "# service" line
> >
> > The http-protocol.txt spec fails to mention that a flush
> > packet comes in the smart server response after sending any
> > the "service" header.
> 
> "any the"?

Oops, should just be "the".

-Peff

Re: [PATCH] smart-http: document flush after "# service" line

[Eric Sunshine]

On Sat, Mar 3, 2018 at 12:27 AM, Jeff King <p...@peff.net> wrote:
> Subject: smart-http: document flush after "# service" line
>
> The http-protocol.txt spec fails to mention that a flush
> packet comes in the smart server response after sending any
> the "service" header.

"any the"?

> Technically the client code is actually ready to receive an
> arbitrary number of headers here, but since we haven't
> introduced any other headers in the past decade (and the
> client would just throw them away), let's not mention it in
> the spec.
>
> This fixes both BNF and the example. While we're fixing the
> latter, let's also add the missing flush after the ref list.
>
> Reported-by: Dorian Taylor <dorian.taylor.li...@gmail.com>
> Signed-off-by: Jeff King <p...@peff.net>

[PATCH] smart-http: document flush after "# service" line

[Jeff King]

On Thu, Feb 22, 2018 at 12:12:54PM -0800, Dorian Taylor wrote:

> This patch exists because I was asked to write it. I don’t know squat
> about this protocol other than the fact that I followed the spec and
> it didn’t work. I traced a known-good protocol endpoint and found it
> contained content that didn’t agree with the spec. I then obliged the
> request to submit a patch with *what I knew to be true* about the
> sample that actually worked. I then followed the recommendations
> *advertised on GitHub* for submitting the patch.

I take it that a revised patch is not forthcoming, then. :-/

Let's wrap up this topic with this, then, which adds a commit message
and fixes the flush/version-1 ordering issue.

-- >8 --
Subject: smart-http: document flush after "# service" line

The http-protocol.txt spec fails to mention that a flush
packet comes in the smart server response after sending any
the "service" header.

Technically the client code is actually ready to receive an
arbitrary number of headers here, but since we haven't
introduced any other headers in the past decade (and the
client would just throw them away), let's not mention it in
the spec.

This fixes both BNF and the example. While we're fixing the
latter, let's also add the missing flush after the ref list.

Reported-by: Dorian Taylor <dorian.taylor.li...@gmail.com>
Signed-off-by: Jeff King <p...@peff.net>
---
 Documentation/technical/http-protocol.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/technical/http-protocol.txt 
b/Documentation/technical/http-protocol.txt
index a0e45f2889..64f49d0bbb 100644
--- a/Documentation/technical/http-protocol.txt
+++ b/Documentation/technical/http-protocol.txt
@@ -214,10 +214,12 @@ smart server reply:
S: Cache-Control: no-cache
S:
S: 001e# service=git-upload-pack\n
+   S: 
S: 004895dcfa3633004da0049d3d0fa03f80589cbcaf31 
refs/heads/maint\0multi_ack\n
S: 0042d049f6c27a2244e12041955e262a404c7faba355 refs/heads/master\n
S: 003c2cb58b79488a98d2721cea644875a8dd0026b115 refs/tags/v1.0\n
S: 003fa3c2e2402b99163d1d59756e5f207ae21cccba4c refs/tags/v1.0^{}\n
+   S: 
 
 The client may send Extra Parameters (see
 Documentation/technical/pack-protocol.txt) as a colon-separated string
@@ -277,6 +279,7 @@ 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
 ""
-- 
2.16.2.708.g0b2ed7f536

Re: Git smart http: parsing commit messages in git-receive-pack

[Junio C Hamano]

Bryan Turner  writes:

> That means if you pair a pre-receive hook with Git 2.11 or newer on
> the server, you should be able to achieve what you're looking for.

It probably is worth mentioning that even without the "quarantine"
changes of 2.11, the new objects brought in by a rejected push
cannot be used because they are not reachable from any ref, and will
be discarded as garbage when the next GC run.  So pre-receive should
be sufficient for the purpose of "not accepting" an undesirable
push, with older versions of Git.  You can view the "quarantine"
feature mostly as "quality of implementation" issue.

Re: Git smart http: parsing commit messages in git-receive-pack

[Bryan Turner]

On Sat, May 6, 2017 at 5:30 AM, akos tajti  wrote:
> Dear All,
>
> we implemented a java servlet around the git-http-backend. This servlet 
> intercepts the requests sent by the git client when pushing. One thing I want 
> to achieve is parsing the commit messages in the pre push phase (request 
> param service==git-receive-pack) and after checking if the commit messages 
> contain a given string reject/accept the request. The problem is that I 
> couldn't find a way of parsing the commit messages in git-receive-pack (I can 
> do this in the post  push phase but in that case I cannot reject the push 
> because the changes are already on the server). Is there a way of getting the 
> commit messages before they're actually on the server so that I can reject 
> the push?

The protocol isn't really intended for use to try and parse pack data
before it's spooled to disk. It might be possible, but it's likely to
be brittle (due to things like delta compression, for example).

I believe what you're looking for is a pre-receive hook[1] on your
server. This is how Bitbucket Server (which is also written in Java)
works. It opens a socket on localhost and installs a pre-receive hook
into each repository which uses a Perl script to pipe all of its input
to the listening socket and then read any response from the server
back. After the push has written all of its objects to disk, Git
invokes any pre-receive hooks and passes in all the ref changes. The
Perl script pipes those to the server, which applies whatever checks
are desired and writes any errors or info back to the hook, which then
prints it for Git to use before exiting with the correct status code
(0 to accept the push, 1 to reject it).

This means the objects are on the server, but in Git 2.11 Peff added
"quarantine" functionality which writes those new objects into an
"incoming" directory[2]. That means, while they're on disk, they're
_not_ in the repository itself. If the pre-receive hook rejects the
push, those objects are immediately deleted from disk.

That means if you pair a pre-receive hook with Git 2.11 or newer on
the server, you should be able to achieve what you're looking for.
Once the objects have been written to the quarantine directory, you
can use normal Git commands, like cat-file, rev-list or log, to review
them. If they don't meet your requirements, just reject the push and
Git will delete the objects automatically

Hope this helps!
Bryan Turner

>
> Thanks in advance,
> Ákos Tajti
>

[1]: https://git-scm.com/docs/githooks includes documentation for
pre-receive inputs and outputs
[2]: 
https://github.com/git/git/blob/master/Documentation/git-receive-pack.txt#L219-L243
includes some additional documentation about the quarantine
environment

Git smart http: parsing commit messages in git-receive-pack

[akos tajti]

Dear All,

we implemented a java servlet around the git-http-backend. This servlet 
intercepts the requests sent by the git client when pushing. One thing I want 
to achieve is parsing the commit messages in the pre push phase (request param 
service==git-receive-pack) and after checking if the commit messages contain a 
given string reject/accept the request. The problem is that I couldn't find a 
way of parsing the commit messages in git-receive-pack (I can do this in the 
post  push phase but in that case I cannot reject the push because the changes 
are already on the server). Is there a way of getting the commit messages 
before they're actually on the server so that I can reject the push?

Thanks in advance,
Ákos Tajti

RE: Smart HTTP push permissions failure

[David McGough]

Thank you for your reply Jeff.  I have moved on to installing GitLab.  It has 
been a success so far.

Thanks,
Dave

-Original Message-
From: Jeff King [mailto:p...@peff.net] 
Sent: Wednesday, August 24, 2016 1:00 PM
To: David McGough <dmcgo...@opentext.com>
Cc: git@vger.kernel.org
Subject: Re: Smart HTTP push permissions failure

On Tue, Aug 23, 2016 at 03:45:33PM +, David McGough wrote:

> When I try to push to the server I get this message:
> remote: error: insufficient permission for adding an object to 
> repository database ./objects
> remote: fatal: failed to write object
> [...]
> So I am pretty confused about what the issue.  Which OS user is git 
> using to write the files?  I hope somebody can help me understand why 
> the project cannot be pushed to the git server.

For a smart-http push, it will be whatever user the web server execs the CGI 
as. So I'd think "apache" would be the default, but it's possible that it runs 
CGIs as a different user, depending on your config.

One possibility may be to add a simple shell script CGI that does something 
like:

  #!/bin/sh
  echo "Content-type: text/plain"
  echo
  id

just to see what's happening.

Based on the data you showed, here are some wild possibilities I can think of:

  - the CGI runs as "apache", but your files are owned by "git".
"apache" is in the "staff" group, and the directories all have write
permission for that group. But are we sure that apache does not shed
any group permissions when running a CGI? The "id" script above
should hopefully show that.

  - You mentioned CentOS. It has been a while since I dealt with RHEL
and its derivatives, but I think selinux is turned on by default
there. Is it possible that the webserver runs in an selinux profile
that does not allow writing to the repository directory?

I don't recall the specifics of debugging selinux problems, but
there may be logs there.

Sorry those are just stabs in the dark, but I don't see anything else obviously 
wrong with what you've posted.

-Peff

Re: Smart HTTP push permissions failure

[Jeff King]

On Tue, Aug 23, 2016 at 03:45:33PM +, David McGough wrote:

> When I try to push to the server I get this message:
> remote: error: insufficient permission for adding an object to repository 
> database ./objects
> remote: fatal: failed to write object
> [...]
> So I am pretty confused about what the issue.  Which OS user is git
> using to write the files?  I hope somebody can help me understand why
> the project cannot be pushed to the git server.

For a smart-http push, it will be whatever user the web server execs the
CGI as. So I'd think "apache" would be the default, but it's possible
that it runs CGIs as a different user, depending on your config.

One possibility may be to add a simple shell script CGI that does
something like:

  #!/bin/sh
  echo "Content-type: text/plain"
  echo
  id

just to see what's happening.

Based on the data you showed, here are some wild possibilities I can
think of:

  - the CGI runs as "apache", but your files are owned by "git".
"apache" is in the "staff" group, and the directories all have write
permission for that group. But are we sure that apache does not shed
any group permissions when running a CGI? The "id" script above
should hopefully show that.

  - You mentioned CentOS. It has been a while since I dealt with RHEL
and its derivatives, but I think selinux is turned on by default
there. Is it possible that the webserver runs in an selinux profile
that does not allow writing to the repository directory?

I don't recall the specifics of debugging selinux problems, but
there may be logs there.

Sorry those are just stabs in the dark, but I don't see anything else
obviously wrong with what you've posted.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Smart HTTP push permissions failure

[David McGough]

Hi Git Community!

I'm trying to get Git on the server.  I installed Git and httpd on Centos 7 and 
configred for smart http.  I created a project on my local git and I cloned it 
to a base repository:
git clone --bare DataConversion DataConversion.git then I scp it to the server: 
scp -r DataConversion g...@xx.xx.xx.xx:/opt/git/repository/product/tools.  Then 
on the server for the project I ran git config core.sharedRepository group  

I added a remote server to my local project: git remote add origin 
http://xx.xx.xx.xx/git/product/tools/DataConversion.git

git remote -v shows:
origin  http://xx.xx.xx.xx/git/product/tools/DataConversion.git (fetch)
origin  http://xx.xx.xx.xx/git/product/tools/DataConversion.git (push)

When I try to push to the server I get this message:
remote: error: insufficient permission for adding an object to repository 
database ./objects
remote: fatal: failed to write object

Fwiw I can clone the project from the server to my local.

Here are the permssions on the project and the objects folder.

[git@services-git DataConversion.git]$ pwd
/opt/git/repos/product/tools/DataConversion.git
[git@services-git DataConversion.git]$ ll
total 24
-rwxrwxr-x.  1 git staff  196 Aug 23 11:24 config
-rwxrwxr-x.  1 git staff   73 Aug 22 15:28 description
-rwxrwxr-x.  1 git staff   23 Aug 22 15:28 HEAD
drwxrwxr-x.  2 git staff 4096 Aug 22 15:28 hooks
drwxrwxr-x.  2 git staff   20 Aug 22 15:28 info
drwxrwxr-x. 65 git staff 4096 Aug 22 16:50 objects
-rwxrwxr-x.  1 git staff   98 Aug 22 15:29 packed-refs
drwxrwxr-x.  4 git staff   29 Aug 22 15:29 refs
[git@services-git DataConversion.git]$
[git@services-git DataConversion.git]$ cd objects
[git@services-git objects]$ ll
total 12
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 06
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 08
drwxrwxr-x. 2 git staff   96 Aug 22 15:28 0a
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 17
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 19
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 1c
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 24
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 29
drwxrwxr-x. 2 git staff 4096 Aug 22 15:28 30
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 32
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 33
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 3d
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 3f
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 41
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 4b
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 57
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 5a
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 5d
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 5f
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 64
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 65
drwxrwxr-x. 2 git staff 4096 Aug 22 15:28 69
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 6d
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 70
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 74
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 7a
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 7b
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 7c
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 84
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 89
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 8a
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 8c
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 93
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 9d
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 a0
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 a2
drwxrwxr-x. 2 git staff 4096 Aug 22 15:28 a3
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 a6
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 ab
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 af
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 b1
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 b7
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 b8
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 c3
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 c8
drwxrwxr-x. 2 git staff   96 Aug 22 15:28 c9
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 cb
drwxrwxr-x. 2 git staff   51 Aug 22 15:28 cf
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 d1
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 d8
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 d9
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 db
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 dc
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 e1
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 e7
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 e9
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 ea
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 ed
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 f0
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 f3
drwxrwxr-x. 2 git staff   51 Aug 22 15:29 f5
drwxrwxr-x. 2 git staff6 Aug 22 15:29 info
drwxrwxr-x. 2 git staff6 Aug 22 15:29 pack

apache and git users are both in the staff group, and staff group is their 
default group.  I have also tried to use the set group id bit but to no avail. 
http://www.gnu.org/software/coreutils/manual/html_node/Directory-Setuid-and-Setgid.html

[root@services-git DataConversion.git]# groups apache
apache : staff git
[root@services-git DataConversion.git]# groups git
git : staff apache

So I am pretty confused about what the issue.  Which

Re: Git Smart HTTP using Nginx

[Dennis Kaarsemaker]

On do, 2016-03-10 at 10:13 -0300, Ben Mezger wrote:

> The git-scm.com only uses apache2 as an example of setting Git's
> Smart
> HTTP, and searching the web for the Nginx's config only gives me old
> configs or not-functional configurations. Has anyone managed to get
> Smart HTTP to work with Nginx and could give me a sample of the
> .conf?

This works for me, using fcgiwrap:

location ~ ^.*/(HEAD|info/refs|objects/info/.*|git-(upload|receive)-pack)$ {
fastcgi_pass unix:/var/run/fcgiwrap.socket;
fastcgi_param SCRIPT_FILENAME   /usr/lib/git-core/git-http-backend;
fastcgi_param PATH_INFO $uri;
fastcgi_param GIT_PROJECT_ROOT  $repo_root;
fastcgi_param REMOTE_USER $remote_user;
include fastcgi_params;
}

-- 
Dennis Kaarsemaker
http://www.kaarsemaker.net


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Git Smart HTTP using Nginx

[Ben Mezger]

Hi all,

The git-scm.com only uses apache2 as an example of setting Git's Smart
HTTP, and searching the web for the Nginx's config only gives me old
configs or not-functional configurations. Has anyone managed to get
Smart HTTP to work with Nginx and could give me a sample of the .conf?

Regards,

Ben Mezger



signature.asc
Description: OpenPGP digital signature

Git Smart HTTP with HTTP/2.0

[ForceCharlie]

As we known, HTTP/2.0 has been released. All Git-Smart-HTTP are currently
implemented using HTTP/1.1.

Frequently used Git developers often feel Git HTTP protocol is not
satisfactory, slow and unstable.This is because the HTTP protocol itself
decides
When HTTP/2.0 is published. We might be able to git developers jointly,
based on HTTP/2.0 Git-Smart-HTTP service and client support.
HTTP/2.0: https://tools.ietf.org/html/rfc7540
Github Mirror: https://httpwg.github.io/specs/rfc7540.html
HTTP/2.0 has Flow Controller like SSH, 
HTTP/2.0 has Server Push POST upload-pack can download large more
object-pack

..
libcurl now has begun to support HTTP/2.0, git is also using curl, as well
libgit2 may use libcurl
I suggest the git of developer joint developer of libgit2 and jgit
developers discussion based on HTTP/2.0 Git-Smart-HTTP
Now Subversion developer begin write a New HTTP Protocol for svn (HTTP
v2):http://svn.apache.org/repos/asf/subversion/trunk/notes/http-and-webdav/
http-protocol-v2.txt
What about git ?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git Smart HTTP with HTTP/2.0

[Ilari Liusvaara]

On Sat, Jul 11, 2015 at 11:10:48AM +0800, ForceCharlie wrote:
 As we known, HTTP/2.0 has been released. All Git-Smart-HTTP are currently
 implemented using HTTP/1.1.

Nit: It is HTTP/2.
 
 Frequently used Git developers often feel Git HTTP protocol is not
 satisfactory, slow and unstable.This is because the HTTP protocol itself
 decides

Note that there are already two versions of HTTP transport, the old dumb
one and the newer smart one.

The smart one is difficult to speed up (due to nature of the negotiations),
but usually is pretty reliable (the efficiency isn't horrible).

Now, the old dumb protocol is pretty unreliable and slow. HTTP/2 probably
can't do anything with the reliability problems, but probably could improve
the speed a bit.

Websockets over HTTP/2 (a.k.a. websockets2) has not been defined yet.
With Websockets(1), it would probably already be possible to tunnel the
native git smart transport protocol over it. Probably not worth it.

 When HTTP/2.0 is published. We might be able to git developers jointly,
 based on HTTP/2.0 Git-Smart-HTTP service and client support.
 HTTP/2.0: https://tools.ietf.org/html/rfc7540

Well, it is published already.
 

-Ilari
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git Smart HTTP with HTTP/2.0

[Shawn Pearce]

On Sat, Jul 11, 2015 at 12:00 AM, Ilari Liusvaara
ilari.liusva...@elisanet.fi wrote:
 On Sat, Jul 11, 2015 at 11:10:48AM +0800, ForceCharlie wrote:
 As we known, HTTP/2.0 has been released. All Git-Smart-HTTP are currently
 implemented using HTTP/1.1.

 Nit: It is HTTP/2.

 Frequently used Git developers often feel Git HTTP protocol is not
 satisfactory, slow and unstable.This is because the HTTP protocol itself
 decides

 Note that there are already two versions of HTTP transport, the old dumb
 one and the newer smart one.

 The smart one is difficult to speed up (due to nature of the negotiations),
 but usually is pretty reliable (the efficiency isn't horrible).

The negotiation in smart-HTTP actually has some bad corner cases. Each
round of negotiation requires a new POST resupplying all previously
agreed upon SHA-1s, and a batch of new SHA-1s. We have observed many
rounds where this POST is MiBs in size because the peers can't quite
agree and have to keep digging through history.

The native protocol on git:// and SSH is not as bad. Negotiation is
still many rounds, but these are pipelined and each round does not
need to repeat the prior round, as the server has a single stream and
is saving state.

 Now, the old dumb protocol is pretty unreliable and slow. HTTP/2 probably
 can't do anything with the reliability problems, but probably could improve
 the speed a bit.

 Websockets over HTTP/2 (a.k.a. websockets2) has not been defined yet.
 With Websockets(1), it would probably already be possible to tunnel the
 native git smart transport protocol over it. Probably not worth it.

Another option is to tunnel using gRPC (grpc.io). libcurl probably
can't do this. And linking grpc.io library into git-core is crazy. So
its probably a non-starter. But food for thought.

But, at $DAY_JOB we tunnel the native bidirectional protocol in
grpc.io's predecessor, and it works quite well for us.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git Smart HTTP with HTTP/2.0

[Ilari Liusvaara]

On Sat, Jul 11, 2015 at 10:23:09AM -0700, Shawn Pearce wrote:
 On Sat, Jul 11, 2015 at 12:00 AM, Ilari Liusvaara
 ilari.liusva...@elisanet.fi wrote:
  On Sat, Jul 11, 2015 at 11:10:48AM +0800, ForceCharlie wrote:
 
  Frequently used Git developers often feel Git HTTP protocol is not
  satisfactory, slow and unstable.This is because the HTTP protocol itself
  decides
 
  Note that there are already two versions of HTTP transport, the old dumb
  one and the newer smart one.
 
  The smart one is difficult to speed up (due to nature of the negotiations),
  but usually is pretty reliable (the efficiency isn't horrible).
 
 The negotiation in smart-HTTP actually has some bad corner cases. Each
 round of negotiation requires a new POST resupplying all previously
 agreed upon SHA-1s, and a batch of new SHA-1s. We have observed many
 rounds where this POST is MiBs in size because the peers can't quite
 agree and have to keep digging through history.

Oh yeah that... Well, that is artifact of HTTP semantics.

  Now, the old dumb protocol is pretty unreliable and slow. HTTP/2 probably
  can't do anything with the reliability problems, but probably could improve
  the speed a bit.
 
  Websockets over HTTP/2 (a.k.a. websockets2) has not been defined yet.
  With Websockets(1), it would probably already be possible to tunnel the
  native git smart transport protocol over it. Probably not worth it.
 
 Another option is to tunnel using gRPC (grpc.io). libcurl probably
 can't do this. And linking grpc.io library into git-core is crazy. So
 its probably a non-starter. But food for thought.

Wouldn't it link into git-remote-http (and on the server side, one
could use pipes to talk to git)?

But supporting websockets in git-remote-http could get annoying,
especially for wss:// (https://). Dunno how bad gRPC would be.



-Ilari
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git Smart HTTP with HTTP/2.0

[Shawn Pearce]

On Sat, Jul 11, 2015 at 11:26 AM, Ilari Liusvaara
ilari.liusva...@elisanet.fi wrote:
 On Sat, Jul 11, 2015 at 10:23:09AM -0700, Shawn Pearce wrote:

  Websockets over HTTP/2 (a.k.a. websockets2) has not been defined yet.
  With Websockets(1), it would probably already be possible to tunnel the
  native git smart transport protocol over it. Probably not worth it.

 Another option is to tunnel using gRPC (grpc.io). libcurl probably
 can't do this. And linking grpc.io library into git-core is crazy. So
 its probably a non-starter. But food for thought.

 Wouldn't it link into git-remote-http (and on the server side, one
 could use pipes to talk to git)?

 But supporting websockets in git-remote-http could get annoying,
 especially for wss:// (https://). Dunno how bad gRPC would be.

We wrote it as git-remote-$THING, invoked with $THING:// URLs. And
git-remote-$THING just implements the connect helper protocol. Its
much simpiler than git-remote-http.

Maybe its done that way in git-core as http2:// aka git-remote-http2.

Or the git-remote-http helper connects to the remote system and tries
to guess if it supports Git on HTTP/2 before responding to the
capabilities request from transport code. If yes, it replies with
connect, if no, it does the current fetch and push protocol.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/7] upload-pack: fix transfer.hiderefs over smart-http

[Junio C Hamano]

Jeff King p...@peff.net writes:

 When upload-pack advertises the refs (either for a normal,
 non-stateless request, or for the initial contact in a
 stateless one), we call for_each_ref with the send_ref
 function as its callback. send_ref, in turn, calls
 mark_our_ref, which checks whether the ref is hidden, and
 sets OUR_REF or HIDDEN_REF on the object as appropriate.  If
 it is hidden, mark_our_ref also returns 1 to signal
 send_ref that the ref should not be advertised.

 If we are not advertising refs, (i.e., the follow-up
 invocation by an http client to send its want lines), we
 use mark_our_ref directly as a callback to for_each_ref. Its
 marking does the right thing, but when it then returns 1
 to for_each_ref, the latter interprets this as an error and
 stops iterating. As a result, we skip marking all of the
 refs that come lexicographically after it. Any want lines
 from the client asking for those objects will fail, as they
 were not properly marked with OUR_REF.

Nicely described in a way that the reason of the breakage and the
fix is obvious to those who already know what the codepath is
supposed to be doing.

 To solve this, we introduce a wrapper callback around
 mark_our_ref which always returns 0 (even if the ref is
 hidden, we want to keep iterating). We also tweak the
 signature of mark_our_ref to exclude unnecessary parameters
 that were present only to conform to the callback interface.
 This should make it less likely for somebody to accidentally
 use it as a callback in the future.

I especially love this kind of future-proofing ;-).

Thanks, will queue.

 Signed-off-by: Jeff King p...@peff.net
 ---
  t/t5551-http-fetch-smart.sh | 11 +++
  upload-pack.c   | 14 ++
  2 files changed, 21 insertions(+), 4 deletions(-)

 diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
 index 6cbc12d..b970773 100755
 --- a/t/t5551-http-fetch-smart.sh
 +++ b/t/t5551-http-fetch-smart.sh
 @@ -213,6 +213,17 @@ test_expect_success 'cookies stored in http.cookiefile 
 when http.savecookies set
   test_cmp expect_cookies.txt cookies_tail.txt
  '
  
 +test_expect_success 'transfer.hiderefs works over smart-http' '
 + test_commit hidden 
 + test_commit visible 
 + git push public HEAD^:refs/heads/a HEAD:refs/heads/b 
 + git --git-dir=$HTTPD_DOCUMENT_ROOT_PATH/repo.git \
 + config transfer.hiderefs refs/heads/a 
 + git clone --bare $HTTPD_URL/smart/repo.git hidden.git 
 + test_must_fail git -C hidden.git rev-parse --verify a 
 + git -C hidden.git rev-parse --verify b
 +'
 +
  test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '
   (
   cd $HTTPD_DOCUMENT_ROOT_PATH/repo.git 
 diff --git a/upload-pack.c b/upload-pack.c
 index b531a32..c8e8713 100644
 --- a/upload-pack.c
 +++ b/upload-pack.c
 @@ -681,7 +681,7 @@ static void receive_needs(void)
  }
  
  /* return non-zero if the ref is hidden, otherwise 0 */
 -static int mark_our_ref(const char *refname, const unsigned char *sha1, int 
 flag, void *cb_data)
 +static int mark_our_ref(const char *refname, const unsigned char *sha1)
  {
   struct object *o = lookup_unknown_object(sha1);
  
 @@ -695,6 +695,12 @@ static int mark_our_ref(const char *refname, const 
 unsigned char *sha1, int flag
   return 0;
  }
  
 +static int check_ref(const char *refname, const unsigned char *sha1, int 
 flag, void *cb_data)
 +{
 + mark_our_ref(refname, sha1);
 + return 0;
 +}
 +
  static void format_symref_info(struct strbuf *buf, struct string_list 
 *symref)
  {
   struct string_list_item *item;
 @@ -713,7 +719,7 @@ static int send_ref(const char *refname, const unsigned 
 char *sha1, int flag, vo
   const char *refname_nons = strip_namespace(refname);
   unsigned char peeled[20];
  
 - if (mark_our_ref(refname, sha1, flag, NULL))
 + if (mark_our_ref(refname, sha1))
   return 0;
  
   if (capabilities) {
 @@ -767,8 +773,8 @@ static void upload_pack(void)
   advertise_shallow_grafts(1);
   packet_flush(1);
   } else {
 - head_ref_namespaced(mark_our_ref, NULL);
 - for_each_namespaced_ref(mark_our_ref, NULL);
 + head_ref_namespaced(check_ref, NULL);
 + for_each_namespaced_ref(check_ref, NULL);
   }
   string_list_clear(symref, 1);
   if (advertise_refs)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] fix transfer.hiderefs with smart http

[Jeff King]

On Fri, Mar 13, 2015 at 12:41:01AM -0400, Jeff King wrote:

 I'm experimenting with using transfer.hiderefs on a server, and it's
 rather easy to cause a git client to hang when fetching from such a repo
 over smart http. Details are in the first patch.

A note on this hang. What happens is that upload-pack sees a bogus
want line and calls die(). The client then sits there forever, but I'm
not exactly sure what it's waiting for.

This series fixes the bug that caused us to erroneously call die() in
upload-pack, so the hang is fixed. But there are other reasons to call
die(); it would probably be a good thing if the client side noticed the
problem and aborted.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/7] upload-pack: fix transfer.hiderefs over smart-http

[Jeff King]

When upload-pack advertises the refs (either for a normal,
non-stateless request, or for the initial contact in a
stateless one), we call for_each_ref with the send_ref
function as its callback. send_ref, in turn, calls
mark_our_ref, which checks whether the ref is hidden, and
sets OUR_REF or HIDDEN_REF on the object as appropriate.  If
it is hidden, mark_our_ref also returns 1 to signal
send_ref that the ref should not be advertised.

If we are not advertising refs, (i.e., the follow-up
invocation by an http client to send its want lines), we
use mark_our_ref directly as a callback to for_each_ref. Its
marking does the right thing, but when it then returns 1
to for_each_ref, the latter interprets this as an error and
stops iterating. As a result, we skip marking all of the
refs that come lexicographically after it. Any want lines
from the client asking for those objects will fail, as they
were not properly marked with OUR_REF.

To solve this, we introduce a wrapper callback around
mark_our_ref which always returns 0 (even if the ref is
hidden, we want to keep iterating). We also tweak the
signature of mark_our_ref to exclude unnecessary parameters
that were present only to conform to the callback interface.
This should make it less likely for somebody to accidentally
use it as a callback in the future.

Signed-off-by: Jeff King p...@peff.net
---
 t/t5551-http-fetch-smart.sh | 11 +++
 upload-pack.c   | 14 ++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index 6cbc12d..b970773 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -213,6 +213,17 @@ test_expect_success 'cookies stored in http.cookiefile 
when http.savecookies set
test_cmp expect_cookies.txt cookies_tail.txt
 '
 
+test_expect_success 'transfer.hiderefs works over smart-http' '
+   test_commit hidden 
+   test_commit visible 
+   git push public HEAD^:refs/heads/a HEAD:refs/heads/b 
+   git --git-dir=$HTTPD_DOCUMENT_ROOT_PATH/repo.git \
+   config transfer.hiderefs refs/heads/a 
+   git clone --bare $HTTPD_URL/smart/repo.git hidden.git 
+   test_must_fail git -C hidden.git rev-parse --verify a 
+   git -C hidden.git rev-parse --verify b
+'
+
 test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '
(
cd $HTTPD_DOCUMENT_ROOT_PATH/repo.git 
diff --git a/upload-pack.c b/upload-pack.c
index b531a32..c8e8713 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -681,7 +681,7 @@ static void receive_needs(void)
 }
 
 /* return non-zero if the ref is hidden, otherwise 0 */
-static int mark_our_ref(const char *refname, const unsigned char *sha1, int 
flag, void *cb_data)
+static int mark_our_ref(const char *refname, const unsigned char *sha1)
 {
struct object *o = lookup_unknown_object(sha1);
 
@@ -695,6 +695,12 @@ static int mark_our_ref(const char *refname, const 
unsigned char *sha1, int flag
return 0;
 }
 
+static int check_ref(const char *refname, const unsigned char *sha1, int flag, 
void *cb_data)
+{
+   mark_our_ref(refname, sha1);
+   return 0;
+}
+
 static void format_symref_info(struct strbuf *buf, struct string_list *symref)
 {
struct string_list_item *item;
@@ -713,7 +719,7 @@ static int send_ref(const char *refname, const unsigned 
char *sha1, int flag, vo
const char *refname_nons = strip_namespace(refname);
unsigned char peeled[20];
 
-   if (mark_our_ref(refname, sha1, flag, NULL))
+   if (mark_our_ref(refname, sha1))
return 0;
 
if (capabilities) {
@@ -767,8 +773,8 @@ static void upload_pack(void)
advertise_shallow_grafts(1);
packet_flush(1);
} else {
-   head_ref_namespaced(mark_our_ref, NULL);
-   for_each_namespaced_ref(mark_our_ref, NULL);
+   head_ref_namespaced(check_ref, NULL);
+   for_each_namespaced_ref(check_ref, NULL);
}
string_list_clear(symref, 1);
if (advertise_refs)
-- 
2.3.2.472.geadab3c

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/7] fix transfer.hiderefs with smart http

[Jeff King]

I'm experimenting with using transfer.hiderefs on a server, and it's
rather easy to cause a git client to hang when fetching from such a repo
over smart http. Details are in the first patch.

There are 7 patches here, but the entirety of the fix is contained in
the first one. The rest are cleanups and test enhancements I found along
the way. I moved the fix to the front of the series as we probably want
it to go to maint, but the others can wait (being mostly test
modifications, they should not cause regressions, but they'd possibly
want more cooking time in case I broke the test suite for somebody).

The patches are:

  [1/7]: upload-pack: fix transfer.hiderefs over smart-http

The fix.

  [2/7]: upload-pack: do not check NULL return of lookup_unknown_object

A nearby cleanup.

  [3/7]: t: translate SIGINT to an exit
  [4/7]: t: redirect stderr GIT_TRACE to descriptor 4
  [5/7]: t: pass GIT_TRACE through Apache

These all solve irritations I had when trying to debug the test.

  [6/7]: t5541: move run_with_cmdline_limit to test-lib.sh
  [7/7]: t5551: make EXPENSIVE test cheaper

I had thought at first that the problem was related to large http
fetches, but it turned out not to be. But I think these cleanups
are a good thing, as they increase the default test coverage.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/7] fix transfer.hiderefs with smart http

[Duy Nguyen]

On Fri, Mar 13, 2015 at 11:59 AM, Jeff King p...@peff.net wrote:
 On Fri, Mar 13, 2015 at 12:41:01AM -0400, Jeff King wrote:

 I'm experimenting with using transfer.hiderefs on a server, and it's
 rather easy to cause a git client to hang when fetching from such a repo
 over smart http. Details are in the first patch.

 A note on this hang. What happens is that upload-pack sees a bogus
 want line and calls die(). The client then sits there forever, but I'm
 not exactly sure what it's waiting for.

 This series fixes the bug that caused us to erroneously call die() in
 upload-pack, so the hang is fixed. But there are other reasons to call
 die(); it would probably be a good thing if the client side noticed the
 problem and aborted.

Maybe we could install a custom die handler that also sends ERR line
to the client before dying. Even with old clients where ERR lines are
not recognized, they would see that as a sign of error and abort. The
only thing to be careful is not sending ERR while we're in the middle
of sending some pkt-line, and that only happens when die() is called
inside packet_write() and we can catch that easily. This is for
upload-pack only as the client side can also use packet_buf_write(), a
bit harder to know if some pkt-line is being sent.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: smart http push setup/egit win7

[Jeff King]

On Wed, Nov 05, 2014 at 07:11:29PM +, Jonathan wrote:

 The client can connect to and successfully fetch the repo from the
 server over https. However, when trying to push egit gives the error
 remote does not support http push. When attempting a push via bash,
 I get return code 22 - fatal: git-http-push failed.

Git will not allow a push unless the client is authenticated (and the
git client may fallback to trying dumb-http push-over-DAV, which you
likely haven't configured; that would explain the second error message).

The authentication check in http-backend checks whether $REMOTE_USER is
set. I see in your config that you set it from $REDIRECT_REMOTE_USER,
but I don't see any actual Auth directives. Do you need to add that
somewhere?

There are some Apache recipes in the git-http-backend manpage that might
help.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Smart HTTP

[John Norris]

Hi,
I guess this comment is aimed at Scott Chacon.
I have read your blog post on Smart HTTP 
(http://git-scm.com/blog/2010/03/04/smart-http.html) and wondered if 
there is any documentation that compares in terms of thoroughness with 
your sections in the book on using SSH, which does explain the basics so 
that anyone can get it working.
I have tried setting up authenticated pull and push with HTTP (not 
HTTPS) and Apache never asks for authentication during a pull and 
refuses a push with a 403 error.

Regards,
John Norris

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Smart HTTP

[Konstantin Khomoutov]

On Mon, 13 Oct 2014 17:29:05 +
John Norris j...@norricorp.f9.co.uk wrote:

 I guess this comment is aimed at Scott Chacon.
 I have read your blog post on Smart HTTP 
 (http://git-scm.com/blog/2010/03/04/smart-http.html) and wondered if 
 there is any documentation that compares in terms of thoroughness
 with your sections in the book on using SSH, which does explain the
 basics so that anyone can get it working.
 I have tried setting up authenticated pull and push with HTTP (not 
 HTTPS) and Apache never asks for authentication during a pull and 
 refuses a push with a 403 error.

Looks like a sort-of followup to this discussion [1].

(John, being a good netizen, you should have included the link to that
discussion yourself, to put your uh comment in context and may be
actually get some useful responses.)

1. https://groups.google.com/d/topic/git-users/zcXYY1Le_F4/discussion
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v6 22/23] signed push: teach smart-HTTP to pass git push --signed around

[Junio C Hamano]

The --signed option received by git push is first passed to the
transport layer, which the native transport directly uses to notice
that a push certificate needs to be sent.  When the transport-helper
is involved, however, the option needs to be told to the helper with
set_helper_option(), and the helper needs to take necessary action.
For the smart-HTTP helper, the necessary action involves spawning
the git send-pack subprocess with the --signed option.

Once the above all gets wired in, the smart-HTTP transport now can
use the push certificate mechanism to authenticate its pushes.

Add a test that is modeled after tests for the native transport in
t5534-push-signed.sh to t5541-http-push-smart.sh.  Update the test
Apache configuration to pass GNUPGHOME environment variable through.
As PassEnv would trigger warnings for an environment variable that
is not set, export it from test-lib.sh set to a harmless value when
GnuPG is not being used in the tests.

Note that the added test is deliberately loose and does not check
the nonce in this step.  This is because the stateless RPC mode is
inevitably flaky and a nonce that comes back in the actual push
processing is one issued by a different process; if the two
interactions with the server crossed a second boundary, the nonces
will not match and such a check will fail.  A later patch in the
series will work around this shortcoming.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 * This used to be after nonce slop patch in v5 (and remote-curl
   integration was missing).

 builtin/send-pack.c|  4 
 remote-curl.c  | 13 -
 t/lib-httpd/apache.conf|  1 +
 t/t5541-http-push-smart.sh | 36 
 t/test-lib.sh  |  3 ++-
 transport-helper.c |  9 -
 6 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index f420b74..ca28d8d 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -153,6 +153,10 @@ int cmd_send_pack(int argc, const char **argv, const char 
*prefix)
args.verbose = 1;
continue;
}
+   if (!strcmp(arg, --signed)) {
+   args.push_cert = 1;
+   continue;
+   }
if (!strcmp(arg, --progress)) {
progress = 1;
continue;
diff --git a/remote-curl.c b/remote-curl.c
index 0fcf2ce..1ea4e95 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -25,7 +25,8 @@ struct options {
update_shallow : 1,
followtags : 1,
dry_run : 1,
-   thin : 1;
+   thin : 1,
+   push_cert : 1;
 };
 static struct options options;
 static struct string_list cas_options = STRING_LIST_INIT_DUP;
@@ -106,6 +107,14 @@ static int set_option(const char *name, const char *value)
else
return -1;
return 0;
+   } else if (!strcmp(name, pushcert)) {
+   if (!strcmp(value, true))
+   options.push_cert = 1;
+   else if (!strcmp(value, false))
+   options.push_cert = 0;
+   else
+   return -1;
+   return 0;
} else {
return 1 /* unsupported */;
}
@@ -872,6 +881,8 @@ static int push_git(struct discovery *heads, int nr_spec, 
char **specs)
argv_array_push(args, --thin);
if (options.dry_run)
argv_array_push(args, --dry-run);
+   if (options.push_cert)
+   argv_array_push(args, --signed);
if (options.verbosity == 0)
argv_array_push(args, --quiet);
else if (options.verbosity  1)
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index b384d79..7713dd2 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -68,6 +68,7 @@ LockFile accept.lock
 
 PassEnv GIT_VALGRIND
 PassEnv GIT_VALGRIND_OPTIONS
+PassEnv GNUPGHOME
 
 Alias /dumb/ www/
 Alias /auth/dumb/ www/auth/dumb/
diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
index 73af16f..24926a4 100755
--- a/t/t5541-http-push-smart.sh
+++ b/t/t5541-http-push-smart.sh
@@ -12,6 +12,7 @@ if test -n $NO_CURL; then
 fi
 
 ROOT_PATH=$PWD
+. $TEST_DIRECTORY/lib-gpg.sh
 . $TEST_DIRECTORY/lib-httpd.sh
 . $TEST_DIRECTORY/lib-terminal.sh
 start_httpd
@@ -323,5 +324,40 @@ test_expect_success 'push into half-auth-complete requires 
password' '
test_cmp expect actual
 '
 
+test_expect_success GPG 'push with post-receive to inspect certificate' '
+   (
+   cd $HTTPD_DOCUMENT_ROOT_PATH/test_repo.git 
+   mkdir -p hooks 
+   write_script hooks/post-receive -\EOF 
+   # discard the update list

[PATCH v5 23/23] t5541: test push --signed to smart HTTP server

[Junio C Hamano]

Currently it seems that somewhere in the transport option setting
chain the --signed bit gets lost and this does not pass.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 t/t5541-http-push-smart.sh | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
index 73af16f..3915f67 100755
--- a/t/t5541-http-push-smart.sh
+++ b/t/t5541-http-push-smart.sh
@@ -14,6 +14,7 @@ fi
 ROOT_PATH=$PWD
 . $TEST_DIRECTORY/lib-httpd.sh
 . $TEST_DIRECTORY/lib-terminal.sh
+. $TEST_DIRECTORY/lib-gpg.sh
 start_httpd
 
 test_expect_success 'setup remote repository' '
@@ -323,5 +324,43 @@ test_expect_success 'push into half-auth-complete requires 
password' '
test_cmp expect actual
 '
 
+test_expect_failure GPG 'push with post-receive to inspect certificate' '
+   (
+   cd $HTTPD_DOCUMENT_ROOT_PATH/test_repo.git 
+   mkdir -p hooks 
+   write_script hooks/post-receive -\EOF 
+   # discard the update list
+   cat /dev/null
+   # record the push certificate
+   if test -n ${GIT_PUSH_CERT-}
+   then
+   git cat-file blob $GIT_PUSH_CERT ../push-cert
+   fi 
+   env ../env 
+   cat ../push-cert-status E_O_F
+   SIGNER=${GIT_PUSH_CERT_SIGNER-nobody}
+   KEY=${GIT_PUSH_CERT_KEY-nokey}
+   STATUS=${GIT_PUSH_CERT_STATUS-nostatus}
+   NONCE=${GIT_PUSH_CERT_NONCE-nononce}
+   E_O_F
+   EOF
+
+   git config receive.certnonceseed sekrit
+   ) 
+   cd $ROOT_PATH/test_repo_clone 
+   test_commit cert-test 
+   git push --signed $HTTPD_URL/smart/test_repo.git 
+   (
+   cd $HTTPD_DOCUMENT_ROOT_PATH 
+   cat -\EOF 
+   SIGNER=C O Mitter commit...@example.com
+   KEY=13B6F51ECDDE430D
+   STATUS=G
+   EOF
+   sed -n -e s/^nonce /NONCE=/p -e /^$/q push-cert
+   ) expect 
+   test_cmp expect $HTTPD_DOCUMENT_ROOT_PATH/push-cert-status
+'
+
 stop_httpd
 test_done
-- 
2.1.0-410-gd72dacd

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: PROPFIND 405 with git-http-backend and Smart HTTP

[lpicquet]

You need to allow the directory to be read?
directory Path/to/your/repositories
Allow from all
/directory




--
View this message in context: 
http://git.661346.n2.nabble.com/PROPFIND-405-with-git-http-backend-and-Smart-HTTP-tp7564017p7616843.html
Sent from the git mailing list archive at Nabble.com.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Returning error message from custom smart http server

[Bryan Turner]

On Sat, May 17, 2014 at 9:01 AM, brian m. carlson
sand...@crustytoothpaste.net wrote:
 On Tue, May 13, 2014 at 09:39:59AM +0200, Ákos, Tajti wrote:
 Dear List,

 we implemented our own git smart http server to be able to check permissions
 and other thing before pushes. It works fine, however, the error messages we
 generate on the server side are not displayed by the command line client. On
 the server we generate error messages like this:

 response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
 response.getWriter().write(msg);

 On the command line we get this:

 Total 0 (delta 0), reused 0 (delta 0)
 POST git-receive-pack (290 bytes)
 efrror: RPC failed; result=22, HTTP code = 401
 atal: The remote end hung up unexpectedly
 fatal: The remote end hung up unexpectedly

 The server message is completely missing. Is there a solution for this?

You should not need a patched git; the wire protocol itself has a
mechanism for sending smart error messages. It's not particularly
_obvious_, but it's there.

For starters, to return an error message, your status must be 200 OK.
You can't return any other status code or Git will interpret your
error as some form of _HTTP_ error rather than a _git_ error.

In the smart protocol the client sends a service to the server as a
query parameter, like ?service=git-receive-pack. For such a request,
you need to:
- Set the content type to application/x-service-advertisement
(e.g. application/x-git-receive-pack-advertisement) (Not all command
line Git versions require this, but JGit does)
- Set the status code as 200 OK
- Write back a payload where the first 4 bytes are the hex-encoded
length of the text (where  is max length for a single packet).
Note that the 4 bytes for the size are _part_ of that length, so if
you're writing Test the length is 8, not 4
- After the size, you write # service=service (e.g. #
service=git-receive-pack; note the space after the #) This is the
metadata. For an error, you don't really have much to say.
- After that, an empty packet, which is  (four zeros) This
separates the metadata from the ref advertisement
- After that you can write your message, beginning with ERR  (note
the trailing space there). The ERR  tells Git what you're writing
isn't a ref, it's an error. I'd recommend appending a newline (and add
1 more to your length for it), because when Git echoes your error
message it doesn't seem to do that

I'm not sure whether there's a document that describes all of this; I
found it by digging into the Git source code (you can find the ERR
handling in connect.c, get_remote_heads). This may be exploiting the
protocol, I'll leave that to someone more knowledgeable on how they
_intended_ this all to be used, but it works for us.

A full example looks something like this: 0036#
service=git-receive-packERR This is a test\n

Hope this helps,
Bryan Turner


 It does look that way.  Does the following patch work for you?

 -- 8 --
 Subject: [PATCH] http: provide server's error message on RPC failure

 The server might provide a custom error message that is useful to the user.
 Provide this message to the user if HTTP RPC fails.

 Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
 ---
  remote-curl.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/remote-curl.c b/remote-curl.c
 index 52c2d96..5984d35 100644
 --- a/remote-curl.c
 +++ b/remote-curl.c
 @@ -426,8 +426,8 @@ static int run_slot(struct active_request_slot *slot,
 err = run_one_slot(slot, results);

 if (err != HTTP_OK  err != HTTP_REAUTH) {
 -   error(RPC failed; result=%d, HTTP code = %ld,
 - results-curl_result, results-http_code);
 +   error(RPC failed; result=%d, HTTP code = %ld (%s),
 + results-curl_result, results-http_code, 
 curl_errorstr);
 }

 return err;
 -- 8 --

 --
 brian m. carlson / brian with sandals: Houston, Texas, US
 +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
 OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Returning error message from custom smart http server

[Carlos Martín Nieto]

On Mon, 2014-05-19 at 18:12 +1000, Bryan Turner wrote:
 On Sat, May 17, 2014 at 9:01 AM, brian m. carlson
 sand...@crustytoothpaste.net wrote:
  On Tue, May 13, 2014 at 09:39:59AM +0200, Ákos, Tajti wrote:
  Dear List,
 
  we implemented our own git smart http server to be able to check 
  permissions
  and other thing before pushes. It works fine, however, the error messages 
  we
  generate on the server side are not displayed by the command line client. 
  On
  the server we generate error messages like this:
 
  response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
  response.getWriter().write(msg);
 
  On the command line we get this:
 
  Total 0 (delta 0), reused 0 (delta 0)
  POST git-receive-pack (290 bytes)
  efrror: RPC failed; result=22, HTTP code = 401
  atal: The remote end hung up unexpectedly
  fatal: The remote end hung up unexpectedly
 
  The server message is completely missing. Is there a solution for this?
 
 You should not need a patched git; the wire protocol itself has a
 mechanism for sending smart error messages. It's not particularly
 _obvious_, but it's there.
 
 For starters, to return an error message, your status must be 200 OK.
 You can't return any other status code or Git will interpret your
 error as some form of _HTTP_ error rather than a _git_ error.
 
 In the smart protocol the client sends a service to the server as a
 query parameter, like ?service=git-receive-pack. For such a request,
 you need to:
 - Set the content type to application/x-service-advertisement
 (e.g. application/x-git-receive-pack-advertisement) (Not all command
 line Git versions require this, but JGit does)
 - Set the status code as 200 OK
 - Write back a payload where the first 4 bytes are the hex-encoded
 length of the text (where  is max length for a single packet).
 Note that the 4 bytes for the size are _part_ of that length, so if
 you're writing Test the length is 8, not 4
 - After the size, you write # service=service (e.g. #
 service=git-receive-pack; note the space after the #) This is the
 metadata. For an error, you don't really have much to say.
 - After that, an empty packet, which is  (four zeros) This
 separates the metadata from the ref advertisement
 - After that you can write your message, beginning with ERR  (note
 the trailing space there). The ERR  tells Git what you're writing
 isn't a ref, it's an error. I'd recommend appending a newline (and add
 1 more to your length for it), because when Git echoes your error
 message it doesn't seem to do that
 
 I'm not sure whether there's a document that describes all of this; I
 found it by digging into the Git source code (you can find the ERR
 handling in connect.c, get_remote_heads). This may be exploiting the
 protocol, I'll leave that to someone more knowledgeable on how they
 _intended_ this all to be used, but it works for us.
 
 A full example looks something like this: 0036#
 service=git-receive-packERR This is a test\n

This is indeed documented, namely in 

Documentation/technical/pack-protocol.txt

I guess it could do with an example, but your usage seems correct. There
are two different places where things could go wrong, either in HTTP,
such as authentication, or in the Git part of the request. If you return
an HTTP 404, then all you're telling the client is that you couldn't
find what it asked for, but that could mean either the
receice-pack/upload-pack program or the repository itself. If something
went wrong at the Git level, whether it's a resource problem in the
server or simply that the repo doesn't exist, then ERR is the right
thing to use.

Particularly, we can't rely on the HTTP 404 response being anything
meaningful, as it could simply be the host's default 404 page, and you
don't want html flying through your terminal.

Cheers,
   cmn



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Returning error message from custom smart http server

[Jeff King]

On Mon, May 19, 2014 at 06:12:10PM +1000, Bryan Turner wrote:

 For starters, to return an error message, your status must be 200 OK.
 You can't return any other status code or Git will interpret your
 error as some form of _HTTP_ error rather than a _git_ error.

As of git v1.8.3, git will show text/plain content sent along with a
a non-200 HTTP code.

However, it does this _only_ for the initial refs fetch (along with
several other error-reporting niceties, including specifically handling
HTTP 401s). The thinking was that the interesting smart-http errors
happen on that initial contact (e.g., failure to login, access denied,
etc). Errors at the HTTP level that happen later during POST requests
mean that the server is misconfigured or broken somehow, and should be
rare. That's the theory anyway.

In the original poster's example, it looks like the server is rejecting
the push with an HTTP 401 during the POST call, after the initial ref
advertisement. This is non-ideal, because it means the client may have
gone to significant work to generate the packfile. It should instead
reject it as soon as it sees a request for
.../info/refs?service=git-receive-pack. Current git clients will
prompt for errors, and will also show the text/plain content.

 - Set the content type to application/x-service-advertisement
 (e.g. application/x-git-receive-pack-advertisement) (Not all command
 line Git versions require this, but JGit does)

A side note, but command-line Git cares about the content-type since
v1.8.1.5.

 [...how git's ERR lines work...]

Your description seemed accurate from my brief read. Sending ERR lines
goes back much further. However, for a 401, I think they really want to
send the HTTP code (and at the right time), so that the client can
recognize this, gather credentials from the user, and try again.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Returning error message from custom smart http server

[brian m. carlson]

On Tue, May 13, 2014 at 09:39:59AM +0200, Ákos, Tajti wrote:
 Dear List,
 
 we implemented our own git smart http server to be able to check permissions
 and other thing before pushes. It works fine, however, the error messages we
 generate on the server side are not displayed by the command line client. On
 the server we generate error messages like this:
 
 response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
 response.getWriter().write(msg);
 
 On the command line we get this:
 
 Total 0 (delta 0), reused 0 (delta 0)
 POST git-receive-pack (290 bytes)
 efrror: RPC failed; result=22, HTTP code = 401
 atal: The remote end hung up unexpectedly
 fatal: The remote end hung up unexpectedly
 
 The server message is completely missing. Is there a solution for this?

It does look that way.  Does the following patch work for you?

-- 8 --
Subject: [PATCH] http: provide server's error message on RPC failure

The server might provide a custom error message that is useful to the user.
Provide this message to the user if HTTP RPC fails.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 remote-curl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 52c2d96..5984d35 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -426,8 +426,8 @@ static int run_slot(struct active_request_slot *slot,
err = run_one_slot(slot, results);
 
if (err != HTTP_OK  err != HTTP_REAUTH) {
-   error(RPC failed; result=%d, HTTP code = %ld,
- results-curl_result, results-http_code);
+   error(RPC failed; result=%d, HTTP code = %ld (%s),
+ results-curl_result, results-http_code, curl_errorstr);
}
 
return err;
-- 8 --

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature

Returning error message from custom smart http server

[Ákos, Tajti]

Dear List,

we implemented our own git smart http server to be able to check 
permissions and other thing before pushes. It works fine, however, the 
error messages we generate on the server side are not displayed by the 
command line client. On the server we generate error messages like this:


response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
response.getWriter().write(msg);

On the command line we get this:

Total 0 (delta 0), reused 0 (delta 0)
POST git-receive-pack (290 bytes)
efrror: RPC failed; result=22, HTTP code = 401
atal: The remote end hung up unexpectedly
fatal: The remote end hung up unexpectedly

The server message is completely missing. Is there a solution for this?

Thanks,
Ákos Tajti


---
A levél vírus, és rosszindulatú kód mentes, mert az avast! Antivirus védelme 
ellenőrizte azt.
http://www.avast.com

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap

[Junio C Hamano]

Duy Nguyen pclo...@gmail.com writes:

 On Fri, Feb 07, 2014 at 10:01:08AM -0800, Junio C Hamano wrote:
 Here is the difference between the posted series and what I queued
 after applying the changes suggested during the review.
 
 Thanks.

 I was going to send a reroll after the received comments. Could you
 put this on top of 6/6, just to make sure future changes in t5537
 (maybe more or less commits created..) does not change the test
 behavior?

 It fixes the test name too. I originally thought, ok let's create
 commits in one test and do fetch in another. But it ended up in the
 same test and I forgot to update test name.

Surely, and thanks for being careful.  Will squash it in.


 -- 8 --
 diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
 index 1413caf..b300383 100755
 --- a/t/t5537-fetch-shallow.sh
 +++ b/t/t5537-fetch-shallow.sh
 @@ -203,7 +203,7 @@ EOF
  # This test is tricky. We need large enough haves that fetch-pack
  # will put pkt-flush in between. Then we need a have the server
  # does not have, it'll send ACK %s ready
 -test_expect_success 'add more commits' '
 +test_expect_success 'no shallow lines after receiving ACK ready' '
   (
   cd shallow 
   for i in $(test_seq 10)
 @@ -224,7 +224,9 @@ test_expect_success 'add more commits' '
   cd clone 
   git checkout --orphan newnew 
   test_commit new-too 
 - git fetch --depth=2
 + GIT_TRACE_PACKET=$TRASH_DIRECTORY/trace git fetch --depth=2 
 + grep fetch-pack ACK .* ready ../trace 
 + ! grep fetch-pack done ../trace
   )
  '
  
 -- 8 -- 
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
 index b0fa738..fb11073 100755
 --- a/t/t5537-fetch-shallow.sh
 +++ b/t/t5537-fetch-shallow.sh
 @@ -200,5 +200,29 @@ EOF
   )
  '
  
 +# This test is tricky. We need large enough haves that fetch-pack
 +# will put pkt-flush in between. Then we need a have the the server
 +# does not have, it'll send ACK %s ready
 +test_expect_success 'add more commits' '
 + (
 + cd shallow 
 + for i in $(seq 10); do
 + git checkout --orphan unrelated$i 
 + test_commit unrelated$i /dev/null 
 + git push -q $HTTPD_DOCUMENT_ROOT_PATH/repo.git 
 refs/heads/unrelated$i:refs/heads/unrelated$i
 + git push -q ../clone/.git refs/heads/unrelated$i:refs/heads/unrelated$i

In addition to two problems Eric and Peff noticed,  chain is
broken between these two pushes.  I initially didn't notice it but
it became obvious after reformatting to fix the indentation.

Here is the difference between the posted series and what I queued
after applying the changes suggested during the review.

Thanks.

 Documentation/technical/pack-protocol.txt |  4 +--
 Documentation/technical/protocol-capabilities.txt | 10 +++
 t/t5537-fetch-shallow.sh  | 34 +--
 3 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index eb0b8a1..39c6410 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -338,8 +338,8 @@ during a prior round.  This helps to ensure that at least 
one common
 ancestor is found before we give up entirely.
 
 Once the 'done' line is read from the client, the server will either
-send a final 'ACK obj-id' or it will send a 'NAK'. 'obj-id' is the
-last SHA-1 determined to be common. The server only sends
+send a final 'ACK obj-id' or it will send a 'NAK'. 'obj-id' is the object
+name of the last commit determined to be common. The server only sends
 ACK after 'done' if there is at least one common base and multi_ack or
 multi_ack_detailed is enabled. The server always sends NAK after 'done'
 if there is no common base found.
diff --git a/Documentation/technical/protocol-capabilities.txt 
b/Documentation/technical/protocol-capabilities.txt
index cb2f5eb..e174343 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -77,15 +77,15 @@ section Packfile Negotiation for more information.
 
 no-done
 ---
-This capability should be only used with smart HTTP protocol. If
+This capability should only be used with the smart HTTP protocol. If
 multi_ack_detailed and no-done are both present, then the sender is
 free to immediately send a pack following its first ACK obj-id ready
 message.
 
-Without no-done in smart HTTP protocol, the server session would end
-and the client has to make another trip to send done and the server
-can send the pack. no-done removes the last round and thus slightly
-reduces latency.
+Without no-done in the smart HTTP protocol, the server session would
+end and the client has to make another trip to send done before
+the server can send the pack. no-done removes the last round and
+thus slightly reduces latency.
 
 thin-pack
 -
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index fb11073..1413caf 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -201,26 +201,30 @@ EOF
 '
 
 # This test is tricky. We need large enough haves that fetch-pack
-# will put pkt-flush in between. Then we need a have the the server
+# will put pkt-flush in between. Then we need a have the server
 # does not have, it'll send ACK %s ready
 test_expect_success 'add more commits' '
(
-   cd shallow 
-   for i in $(seq 10); do
-   git checkout --orphan unrelated$i 
-   test_commit unrelated$i /dev/null 
-   git push -q $HTTPD_DOCUMENT_ROOT_PATH/repo.git 
refs/heads/unrelated$i:refs/heads/unrelated$i
-   git push -q ../clone/.git refs/heads/unrelated$i:refs/heads/unrelated$i
-   done 
-   git checkout master 
-   test_commit new 
-   git push  $HTTPD_DOCUMENT_ROOT_PATH/repo.git master
+   cd shallow 
+   for i in $(test_seq 10)
+   do
+   git checkout --orphan unrelated$i 
+   test_commit unrelated$i 
+   git push -q $HTTPD_DOCUMENT_ROOT_PATH/repo.git \
+   refs/heads/unrelated$i:refs/heads/unrelated$i 
+   git push -q ../clone/.git \
+   refs/heads/unrelated$i:refs/heads/unrelated$i ||
+   exit 1
+   done 
+   git checkout master 
+   test_commit new 
+   git push  $HTTPD_DOCUMENT_ROOT_PATH/repo.git master

Re: [PATCH 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap

[Duy Nguyen]

On Fri, Feb 07, 2014 at 10:01:08AM -0800, Junio C Hamano wrote:
 Here is the difference between the posted series and what I queued
 after applying the changes suggested during the review.
 
 Thanks.

I was going to send a reroll after the received comments. Could you
put this on top of 6/6, just to make sure future changes in t5537
(maybe more or less commits created..) does not change the test
behavior?

It fixes the test name too. I originally thought, ok let's create
commits in one test and do fetch in another. But it ended up in the
same test and I forgot to update test name.

-- 8 --
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index 1413caf..b300383 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -203,7 +203,7 @@ EOF
 # This test is tricky. We need large enough haves that fetch-pack
 # will put pkt-flush in between. Then we need a have the server
 # does not have, it'll send ACK %s ready
-test_expect_success 'add more commits' '
+test_expect_success 'no shallow lines after receiving ACK ready' '
(
cd shallow 
for i in $(test_seq 10)
@@ -224,7 +224,9 @@ test_expect_success 'add more commits' '
cd clone 
git checkout --orphan newnew 
test_commit new-too 
-   git fetch --depth=2
+   GIT_TRACE_PACKET=$TRASH_DIRECTORY/trace git fetch --depth=2 
+   grep fetch-pack ACK .* ready ../trace 
+   ! grep fetch-pack done ../trace
)
 '
 
-- 8 -- 
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap

[Nguyễn Thái Ngọc Duy]

In smart http, upload-pack adds new shallow lines at the beginning of
each rpc response. Only shallow lines from the first rpc call are
useful. After that they are thrown away. It's designed this way
because upload-pack is stateless and has no idea when its shallow
lines are helpful or not.

So after refs are negotiated with multi_ack_detailed and both sides
happy. The server sends ACK obj-id ready, terminates the rpc call
and waits for the final rpc round. The client sends done. The server
sends another response, which also has shallow lines at the beginning,
and the last ACK obj-id line.

When no-done is active, the last round is cut out, the server sends
ACK obj-id ready and ACK obj-id in the same rpc
response. fetch-pack is updated to recognize this and not send
done. However it still tries to consume shallow lines, which are
never sent.

Update the code, make sure to skip consuming shallow lines when
no-done is enabled.

Reported-by: Jeff King p...@peff.net
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 fetch-pack.c |  3 ++-
 t/t5537-fetch-shallow.sh | 24 
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 90fdd49..f061f1f 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -439,7 +439,8 @@ done:
}
strbuf_release(req_buf);
 
-   consume_shallow_list(args, fd[0]);
+   if (!got_ready || !no_done)
+   consume_shallow_list(args, fd[0]);
while (flushes || multi_ack) {
int ack = get_ack(fd[0], result_sha1);
if (ack) {
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index b0fa738..fb11073 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -200,5 +200,29 @@ EOF
)
 '
 
+# This test is tricky. We need large enough haves that fetch-pack
+# will put pkt-flush in between. Then we need a have the the server
+# does not have, it'll send ACK %s ready
+test_expect_success 'add more commits' '
+   (
+   cd shallow 
+   for i in $(seq 10); do
+   git checkout --orphan unrelated$i 
+   test_commit unrelated$i /dev/null 
+   git push -q $HTTPD_DOCUMENT_ROOT_PATH/repo.git 
refs/heads/unrelated$i:refs/heads/unrelated$i
+   git push -q ../clone/.git refs/heads/unrelated$i:refs/heads/unrelated$i
+   done 
+   git checkout master 
+   test_commit new 
+   git push  $HTTPD_DOCUMENT_ROOT_PATH/repo.git master
+   ) 
+   (
+   cd clone 
+   git checkout --orphan newnew 
+   test_commit new-too 
+   git fetch --depth=2
+   )
+'
+
 stop_httpd
 test_done
-- 
1.8.5.2.240.g8478abd

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 In smart http, upload-pack adds new shallow lines at the beginning of
 each rpc response. Only shallow lines from the first rpc call are
 useful. After that they are thrown away. It's designed this way
 because upload-pack is stateless and has no idea when its shallow
 lines are helpful or not.

 So after refs are negotiated with multi_ack_detailed and both sides
 happy. The server sends ACK obj-id ready, terminates the rpc call

Is the above ... and both sides are happy, the server sends ...?
Lack of a verb makes it hard to guess.

 and waits for the final rpc round. The client sends done. The server
 sends another response, which also has shallow lines at the beginning,
 and the last ACK obj-id line.

 When no-done is active, the last round is cut out, the server sends
 ACK obj-id ready and ACK obj-id in the same rpc
 response. fetch-pack is updated to recognize this and not send
 done. However it still tries to consume shallow lines, which are
 never sent.

 Update the code, make sure to skip consuming shallow lines when
 no-done is enabled.

 Reported-by: Jeff King p...@peff.net
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---

Thanks.

Do I understand the situation correctly if I said The call to
consume-shallow-list has been here from the very beginning, but it
should have been adjusted like this patch when no-done was
introduced.?

  fetch-pack.c |  3 ++-
  t/t5537-fetch-shallow.sh | 24 
  2 files changed, 26 insertions(+), 1 deletion(-)

 diff --git a/fetch-pack.c b/fetch-pack.c
 index 90fdd49..f061f1f 100644
 --- a/fetch-pack.c
 +++ b/fetch-pack.c
 @@ -439,7 +439,8 @@ done:
   }
   strbuf_release(req_buf);
  
 - consume_shallow_list(args, fd[0]);
 + if (!got_ready || !no_done)
 + consume_shallow_list(args, fd[0]);
   while (flushes || multi_ack) {
   int ack = get_ack(fd[0], result_sha1);
   if (ack) {
 diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
 index b0fa738..fb11073 100755
 --- a/t/t5537-fetch-shallow.sh
 +++ b/t/t5537-fetch-shallow.sh
 @@ -200,5 +200,29 @@ EOF
   )
  '
  
 +# This test is tricky. We need large enough haves that fetch-pack
 +# will put pkt-flush in between. Then we need a have the the server
 +# does not have, it'll send ACK %s ready
 +test_expect_success 'add more commits' '
 + (
 + cd shallow 
 + for i in $(seq 10); do
 + git checkout --orphan unrelated$i 
 + test_commit unrelated$i /dev/null 
 + git push -q $HTTPD_DOCUMENT_ROOT_PATH/repo.git 
 refs/heads/unrelated$i:refs/heads/unrelated$i
 + git push -q ../clone/.git refs/heads/unrelated$i:refs/heads/unrelated$i
 + done 
 + git checkout master 
 + test_commit new 
 + git push  $HTTPD_DOCUMENT_ROOT_PATH/repo.git master
 + ) 
 + (
 + cd clone 
 + git checkout --orphan newnew 
 + test_commit new-too 
 + git fetch --depth=2
 + )
 +'
 +
  stop_httpd
  test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap

[Eric Sunshine]

On Thu, Feb 6, 2014 at 10:10 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
 diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
 index b0fa738..fb11073 100755
 --- a/t/t5537-fetch-shallow.sh
 +++ b/t/t5537-fetch-shallow.sh
 @@ -200,5 +200,29 @@ EOF
 )
  '

 +# This test is tricky. We need large enough haves that fetch-pack
 +# will put pkt-flush in between. Then we need a have the the server

s/the the/that the/

 +# does not have, it'll send ACK %s ready
 +test_expect_success 'add more commits' '
 +   (
 +   cd shallow 
 +   for i in $(seq 10); do

Do you want to use test_seq here?

 +   git checkout --orphan unrelated$i 
 +   test_commit unrelated$i /dev/null 
 +   git push -q $HTTPD_DOCUMENT_ROOT_PATH/repo.git 
 refs/heads/unrelated$i:refs/heads/unrelated$i
 +   git push -q ../clone/.git 
 refs/heads/unrelated$i:refs/heads/unrelated$i
 +   done 
 +   git checkout master 
 +   test_commit new 
 +   git push  $HTTPD_DOCUMENT_ROOT_PATH/repo.git master
 +   ) 
 +   (
 +   cd clone 
 +   git checkout --orphan newnew 
 +   test_commit new-too 
 +   git fetch --depth=2
 +   )
 +'
 +
  stop_httpd
  test_done
 --
 1.8.5.2.240.g8478abd
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap

[Jeff King]

On Thu, Feb 06, 2014 at 10:10:39PM +0700, Nguyễn Thái Ngọc Duy wrote:

 In smart http, upload-pack adds new shallow lines at the beginning of
 each rpc response. Only shallow lines from the first rpc call are
 useful. After that they are thrown away. It's designed this way
 because upload-pack is stateless and has no idea when its shallow
 lines are helpful or not.
 
 So after refs are negotiated with multi_ack_detailed and both sides
 happy. The server sends ACK obj-id ready, terminates the rpc call
 and waits for the final rpc round. The client sends done. The server
 sends another response, which also has shallow lines at the beginning,
 and the last ACK obj-id line.
 
 When no-done is active, the last round is cut out, the server sends
 ACK obj-id ready and ACK obj-id in the same rpc
 response. fetch-pack is updated to recognize this and not send
 done. However it still tries to consume shallow lines, which are
 never sent.
 
 Update the code, make sure to skip consuming shallow lines when
 no-done is enabled.

Thanks for a nice explanation.

 +# This test is tricky. We need large enough haves that fetch-pack
 +# will put pkt-flush in between. Then we need a have the the server

s/the the/the/

 +# does not have, it'll send ACK %s ready
 +test_expect_success 'add more commits' '
 + (
 + cd shallow 
 + for i in $(seq 10); do

This probably needs to be test_seq for portability.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/6] fetch-pack: fix deepen shallow over smart http with no-done cap

[Duy Nguyen]

On Fri, Feb 7, 2014 at 2:16 AM, Junio C Hamano gits...@pobox.com wrote:
 Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 In smart http, upload-pack adds new shallow lines at the beginning of
 each rpc response. Only shallow lines from the first rpc call are
 useful. After that they are thrown away. It's designed this way
 because upload-pack is stateless and has no idea when its shallow
 lines are helpful or not.

 So after refs are negotiated with multi_ack_detailed and both sides
 happy. The server sends ACK obj-id ready, terminates the rpc call

 Is the above ... and both sides are happy, the server sends ...?

Yes. Although think again, both sides is incorrect. If the server
side is happy, then it'll send ACK.. ready to stop the client. The
client can hardly protest.

 Do I understand the situation correctly if I said The call to
 consume-shallow-list has been here from the very beginning, but it
 should have been adjusted like this patch when no-done was
 introduced.?

It's been there since the introduction of smart http (there are so
many beginnings in git). The rest is right.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 23/28] Support shallow fetch/clone over smart-http

[Jeff King]

On Thu, Dec 05, 2013 at 08:02:50PM +0700, Nguyễn Thái Ngọc Duy wrote:

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  Documentation/gitremote-helpers.txt |  7 +++
  builtin/fetch-pack.c| 16 +---
  remote-curl.c   | 31 +--
  t/t5536-fetch-shallow.sh| 27 +++

I'm getting test failures in 'next' with GIT_TEST_HTTPD set, and they
are bisectable to this patch (actually, the moral equivalent of it, as
it looks like the commit message was tweaked along with the test number
when it was applied). The failures look like this:

  $ GIT_TEST_HTTPD=1 ./t5537-fetch-shallow.sh -v -i
  [...]
  ok 9 - fetch --update-shallow
  
  expecting success: 
  git clone --bare --no-local shallow 
$HTTPD_DOCUMENT_ROOT_PATH/repo.git 
  git clone $HTTPD_URL/smart/repo.git clone 
  (
  cd clone 
  git fsck 
  git log --format=%s origin/master actual 
  cat EOF expect 
  6
  5
  4
  3
  EOF
  test_cmp expect actual
  )
  
  Cloning into bare repository '/home/peff/compile/git/t/trash 
directory.t5537-fetch-shallow/httpd/www/repo.git'...
  remote: Counting objects: 19, done.
  remote: Compressing objects: 100% (7/7), done.
  remote: Total 19 (delta 0), reused 6 (delta 0)
  Receiving objects: 100% (19/19), done.
  Checking connectivity... done.
  Cloning into 'clone'...
  remote: Counting objects: 19, done.
  remote: Compressing objects: 100% (7/7), done.
  remote: Total 19 (delta 0), reused 19 (delta 0)
  Unpacking objects: 100% (19/19), done.
  Checking connectivity... done.
  Checking object directories: 100% (256/256), done.
  --- expect  2014-01-08 11:20:20.178546452 +
  +++ actual  2014-01-08 11:20:20.178546452 +
  @@ -1,3 +1,4 @@
  +7
   6
   5
   4
  not ok 10 - clone http repository


If you do end up tweaking the test, you may also want to fix:

 +LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5536'}
 +. $TEST_DIRECTORY/lib-httpd.sh

Since the test number got switched, it would be nice to tweak the port
number to match it, in case the real t5536 ever starts using lib-httpd.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 23/28] Support shallow fetch/clone over smart-http

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/gitremote-helpers.txt |  7 +++
 builtin/fetch-pack.c| 16 +---
 remote-curl.c   | 31 +--
 t/t5536-fetch-shallow.sh| 27 +++
 transport-helper.c  |  6 ++
 upload-pack.c   |  2 --
 6 files changed, 82 insertions(+), 7 deletions(-)

diff --git a/Documentation/gitremote-helpers.txt 
b/Documentation/gitremote-helpers.txt
index f1f4ca9..c2908db 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -437,6 +437,13 @@ set by Git if the remote helper has the 'option' 
capability.
 'option check-connectivity' \{'true'|'false'\}::
Request the helper to check connectivity of a clone.
 
+'option cloning \{'true'|'false'\}::
+   Notify the helper this is a clone request (i.e. the current
+   repository is guaranteed empty).
+
+'option update-shallow \{'true'|'false'\}::
+   Allow to extend .git/shallow if the new refs require it.
+
 SEE ALSO
 
 linkgit:git-remote[1]
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index aa6e596..81fae38 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -3,6 +3,7 @@
 #include fetch-pack.h
 #include remote.h
 #include connect.h
+#include sha1-array.h
 
 static const char fetch_pack_usage[] =
 git fetch-pack [--all] [--stdin] [--quiet|-q] [--keep|-k] [--thin] 
@@ -46,6 +47,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char 
*prefix)
char **pack_lockfile_ptr = NULL;
struct child_process *conn;
struct fetch_pack_args args;
+   struct sha1_array shallow = SHA1_ARRAY_INIT;
 
packet_trace_identity(fetch-pack);
 
@@ -113,6 +115,14 @@ int cmd_fetch_pack(int argc, const char **argv, const char 
*prefix)
args.check_self_contained_and_connected = 1;
continue;
}
+   if (!strcmp(--cloning, arg)) {
+   args.cloning = 1;
+   continue;
+   }
+   if (!strcmp(--update-shallow, arg)) {
+   args.update_shallow = 1;
+   continue;
+   }
usage(fetch_pack_usage);
}
 
@@ -157,10 +167,10 @@ int cmd_fetch_pack(int argc, const char **argv, const 
char *prefix)
   args.verbose ? CONNECT_VERBOSE : 0);
}
 
-   get_remote_heads(fd[0], NULL, 0, ref, 0, NULL, NULL);
+   get_remote_heads(fd[0], NULL, 0, ref, 0, NULL, shallow);
 
-   ref = fetch_pack(args, fd, conn, ref, dest,
-sought, nr_sought, NULL, pack_lockfile_ptr);
+   ref = fetch_pack(args, fd, conn, ref, dest, sought, nr_sought,
+shallow, pack_lockfile_ptr);
if (pack_lockfile) {
printf(lock %s\n, pack_lockfile);
fflush(stdout);
diff --git a/remote-curl.c b/remote-curl.c
index 25d6730..3d97e3d 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -10,6 +10,7 @@
 #include sideband.h
 #include argv-array.h
 #include credential.h
+#include sha1-array.h
 
 static struct remote *remote;
 /* always ends with a trailing slash */
@@ -20,6 +21,8 @@ struct options {
unsigned long depth;
unsigned progress : 1,
check_self_contained_and_connected : 1,
+   cloning : 1,
+   update_shallow : 1,
followtags : 1,
dry_run : 1,
thin : 1;
@@ -88,6 +91,24 @@ static int set_option(const char *name, const char *value)
strbuf_release(val);
return 0;
}
+   else if (!strcmp(name, cloning)) {
+   if (!strcmp(value, true))
+   options.cloning = 1;
+   else if (!strcmp(value, false))
+   options.cloning = 0;
+   else
+   return -1;
+   return 0;
+   }
+   else if (!strcmp(name, update-shallow)) {
+   if (!strcmp(value, true))
+   options.update_shallow = 1;
+   else if (!strcmp(value, false))
+   options.update_shallow = 0;
+   else
+   return -1;
+   return 0;
+   }
else {
return 1 /* unsupported */;
}
@@ -99,6 +120,7 @@ struct discovery {
char *buf;
size_t len;
struct ref *refs;
+   struct sha1_array shallow;
unsigned proto_git : 1;
 };
 static struct discovery *last_discovery;
@@ -107,7 +129,7 @@ static struct ref *parse_git_refs(struct discovery *heads, 
int for_push)
 {
struct ref *list = NULL;
get_remote_heads(-1, heads-buf, heads-len, list,
-for_push ? REF_NORMAL : 0, NULL, NULL);
+

Use mongoose to test smart-http unconditionally?

[Duy Nguyen]

I was thinking of an alternative to apache for testing smart-http so
that most of http tests could always run. Mongoose [1] looks like a
good candidate to bundle with git. Just one pair of source files,
mongoose.[ch], a mainloop wrapper and we have an http server. Just
wondering, do we rely on any apache-specific features? I'm not so
familiar with lib-httpd.sh..

[1] https://github.com/cesanta/mongoose
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Use mongoose to test smart-http unconditionally?

[Shawn Pearce]

On Wed, Dec 4, 2013 at 2:53 AM, Duy Nguyen pclo...@gmail.com wrote:
 I was thinking of an alternative to apache for testing smart-http so
 that most of http tests could always run. Mongoose [1] looks like a
 good candidate to bundle with git. Just one pair of source files,
 mongoose.[ch], a mainloop wrapper and we have an http server. Just
 wondering, do we rely on any apache-specific features? I'm not so
 familiar with lib-httpd.sh..

I don't think we do anything Apache specific in the test suite. It
basically relies on CGI execution, being able to configure a URL to
serve a directory, and making some URLs 404 or 500 so we can emulate a
broken or failing server to test the client behavior in those
conditions. At worst that 404/500 forced failure mode could be handled
by a CGI.

 [1] https://github.com/cesanta/mongoose
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Use mongoose to test smart-http unconditionally?

[Jeff King]

On Wed, Dec 04, 2013 at 10:13:11AM -0800, Shawn Pearce wrote:

 On Wed, Dec 4, 2013 at 2:53 AM, Duy Nguyen pclo...@gmail.com wrote:
  I was thinking of an alternative to apache for testing smart-http so
  that most of http tests could always run. Mongoose [1] looks like a
  good candidate to bundle with git. Just one pair of source files,
  mongoose.[ch], a mainloop wrapper and we have an http server. Just
  wondering, do we rely on any apache-specific features? I'm not so
  familiar with lib-httpd.sh..
 
 I don't think we do anything Apache specific in the test suite. It
 basically relies on CGI execution, being able to configure a URL to
 serve a directory, and making some URLs 404 or 500 so we can emulate a
 broken or failing server to test the client behavior in those
 conditions. At worst that 404/500 forced failure mode could be handled
 by a CGI.

I don't think there's anything apache specific, but there's a fair bit
of config for handling various auth scenarios. It's stuff I'd expect any
decent server implementation to handle, but somebody actually needs to
go through and translate all of the config to mongoose.

I've been tempted to add lighttpd support, as I generally find its
config much more readable (and less prone to breaking during upgrades).
But I think it would be a mistake to support multiple servers, as it
would mean updates to the tests need to hit all of the servers. If
mongoose gives a sane lowest common denominator, that's fine with me.

I don't know if it is worth all that much effort, though. I suppose it
could get us more exposure to the httpd tests, but I do not know if it
would be a good idea to turn them on by default anyway. They touch
global machine resources (like ports) that can cause conflicts or test
failures. I assume that is the reason we do not turn on git-daemon tests
by default (though perhaps it would be better in both cases to have it
on by default and let people with special needs, like running multiple
test instances at once, turn it off).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Use mongoose to test smart-http unconditionally?

[Junio C Hamano]

Duy Nguyen pclo...@gmail.com writes:

 I was thinking of an alternative to apache for testing smart-http
 so that most of http tests could always run.  Mongoose [1] looks
 like a good candidate to bundle with git. Just one pair of source
 files, mongoose.[ch], a mainloop wrapper and we have an http
 server.

H.  How would the high-level integration look like?

 - we add contrib/mongoose/*;

 - in t/Makefile, we:

   . set GIT_TEST_HTTPD to yes, unless it is already set to another value;

   . set LIB_HTTPD_PATH to $GIT_BUILD_DIR/contrib/mongoose/mongoose,
 unless it is already set to another value;

   . if LIB_HTTPD_PATH is set to our mongoose and if it hasn't been
 built, go ../contrib/mongoose and build it.

 - we teach lib-httpd.sh to trigger the DEFAULT_HTTPD_PATH
   computation when LIB_HTTPD_PATH is set to 'system-apache', so
   that people can test with their installed apache if they choose
   to; and

 - we teach lib-httpd.sh to write out an appropriate configuration
   for the mongoose server.

That would force people to run http tests by turning it from an
opt-in option to an opt-out option.

Or were you thinking about embedding mongoose in the git executable?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Use mongoose to test smart-http unconditionally?

[Jeff King]

On Wed, Dec 04, 2013 at 12:09:09PM -0800, Junio C Hamano wrote:

 Duy Nguyen pclo...@gmail.com writes:
 
  I was thinking of an alternative to apache for testing smart-http
  so that most of http tests could always run.  Mongoose [1] looks
  like a good candidate to bundle with git. Just one pair of source
  files, mongoose.[ch], a mainloop wrapper and we have an http
  server.
 
 H.  How would the high-level integration look like?
 
  - we add contrib/mongoose/*;
 
  - in t/Makefile, we:
 
. set GIT_TEST_HTTPD to yes, unless it is already set to another value;
 
. set LIB_HTTPD_PATH to $GIT_BUILD_DIR/contrib/mongoose/mongoose,
  unless it is already set to another value;
 
. if LIB_HTTPD_PATH is set to our mongoose and if it hasn't been
  built, go ../contrib/mongoose and build it.

I think building it on-demand is probably too much effort. If it is
portable, then it should not be a problem to just build it along with
the rest of git. If it is not, then we should rethink whether it is
worth including.

  - we teach lib-httpd.sh to trigger the DEFAULT_HTTPD_PATH
computation when LIB_HTTPD_PATH is set to 'system-apache', so
that people can test with their installed apache if they choose
to; and

I do not think we want to allow run-time switching between an embedded
solution and apache. That would mean that we have to keep two sets of
http-server config in sync.

 Or were you thinking about embedding mongoose in the git executable?

I don't think it makes sense to embed it in git, but it could easily be
test-httpd.

The rollout would be:

  1. add contrib/mongoose/*

  2. add test-httpd which links against mongoose, built by default in the
 Makefile

  3. convert lib-httpd/apache.conf into mongoose config as necessary

  4. convert lib-httpd.sh to run test-httpd instead of LIB_HTTPD_PATH

  5. delete apache.conf, LIB_HTTPD_PATH and any other apache remnants

  6. default GIT_TEST_HTTPD to yes

Step 3 is the part where I would anticipate trouble (i.e., finding out
that the new server does not do everything the tests expect).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Use mongoose to test smart-http unconditionally?

[Junio C Hamano]

Jeff King p...@peff.net writes:

 The rollout would be:

   1. add contrib/mongoose/*

   2. add test-httpd which links against mongoose, built by default in the
  Makefile

   3. convert lib-httpd/apache.conf into mongoose config as necessary

   4. convert lib-httpd.sh to run test-httpd instead of LIB_HTTPD_PATH

   5. delete apache.conf, LIB_HTTPD_PATH and any other apache remnants

   6. default GIT_TEST_HTTPD to yes

 Step 3 is the part where I would anticipate trouble (i.e., finding out
 that the new server does not do everything the tests expect).

If it involves making things not tested with apache, I'd actually be
less supportive for the whole plan.  I thought the primary objective
was to encourage people who currently are _not_ running httpd tests
by making a lightweight server available out of the box, robbing an
excuse my box does not have apache installed from them.

As long as a server supports bog standard CGI interface, smart-http
should work the same way with any such server.  For that reason, it
should be theoretically sufficient to test with one non-apache
server (i.e. mongoose) for the purpose of making sure _our_ end of
the set-up works, but still...

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Use mongoose to test smart-http unconditionally?

[Jonathan Nieder]

Jeff King wrote:

 I don't know if it is worth all that much effort, though. I suppose it
 could get us more exposure to the httpd tests, but I do not know if it
 would be a good idea to turn them on by default anyway. They touch
 global machine resources (like ports) that can cause conflicts or test
 failures. I assume that is the reason we do not turn on git-daemon tests
 by default

Yup, that's why I don't run them.

For what it's worth, when I build git and run tests I tend to be in an
environment with apache available, but I'm too lazy to configure git's
tests to pick the right port and make sure it is reserved and so on.
Perhaps there's some way to help lazy people in the same boat?  (E.g.,
picking a port randomly and skipping instead of failing a test when
it's taken or something)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Use mongoose to test smart-http unconditionally?

[Duy Nguyen]

On Thu, Dec 5, 2013 at 6:28 AM, Jonathan Nieder jrnie...@gmail.com wrote:
 Jeff King wrote:

 I don't know if it is worth all that much effort, though. I suppose it
 could get us more exposure to the httpd tests, but I do not know if it
 would be a good idea to turn them on by default anyway. They touch
 global machine resources (like ports) that can cause conflicts or test
 failures. I assume that is the reason we do not turn on git-daemon tests
 by default

 Yup, that's why I don't run them.

 For what it's worth, when I build git and run tests I tend to be in an
 environment with apache available, but I'm too lazy to configure git's
 tests to pick the right port and make sure it is reserved and so on.
 Perhaps there's some way to help lazy people in the same boat?  (E.g.,
 picking a port randomly and skipping instead of failing a test when
 it's taken or something)

Sounds like good first steps.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Use mongoose to test smart-http unconditionally?

[Jeff King]

On Wed, Dec 04, 2013 at 02:53:13PM -0800, Junio C Hamano wrote:

 If it involves making things not tested with apache, I'd actually be
 less supportive for the whole plan.

I hadn't really considered that angle. Apache is a much more realistic
real-world deployment. We give advice for it in git-http-backend(1), and
the tests do check that that advice works (OTOH, we also give advice for
lighttpd, but that is not checked in the test scripts).

 I thought the primary objective was to encourage people who currently
 are _not_ running httpd tests by making a lightweight server available
 out of the box, robbing an excuse my box does not have apache
 installed from them.

Whether we get rid of apache or not, I think a new lightweight server
would fulfill that goal. I just did not want the maintenance burden of
managing multiple configs (and our test harness apache config has grown
non-trivial).

 As long as a server supports bog standard CGI interface, smart-http
 should work the same way with any such server.  For that reason, it
 should be theoretically sufficient to test with one non-apache
 server (i.e. mongoose) for the purpose of making sure _our_ end of
 the set-up works, but still...

There are definitely subtleties between servers. For example, when I
worked on fetching bundles over http a while back, there was a big
difference between lighttpd and apache. A request for
http://example.com/foo.bundle/info/refs; would return the bundle under
lighttpd, but not under apache (for an apache server, we would have to
make a fallback request). The client needs to be able to handle both
scenarios gracefully.

That's a case where it would be nice to be able to test _both_ cases,
and that may be an argument for having multiple (or trying to configure
apache to do both behaviors). But it shows that there may be subtle
differences between a fake test server and a real deployment.

So thinking on it more, I'm somewhat less enthusiastic about mongoose.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Use mongoose to test smart-http unconditionally?

[Jeff King]

On Wed, Dec 04, 2013 at 03:28:00PM -0800, Jonathan Nieder wrote:

 For what it's worth, when I build git and run tests I tend to be in an
 environment with apache available, but I'm too lazy to configure git's
 tests to pick the right port and make sure it is reserved and so on.
 Perhaps there's some way to help lazy people in the same boat?  (E.g.,
 picking a port randomly and skipping instead of failing a test when
 it's taken or something)

What level of magic do you need?

For most people, apt-get install apache  make GIT_TEST_HTTPD=yes
test is enough to run the tests. It uses ports in the 5000-range; this
_can_ conflict with other services the user is running, but it should
not usually (we don't match anything in a typical /etc/services file).
And each script uses its own port, so running the tests in parallel is
fine.

If you are planning on running make test from multiple git checkouts
at the same time, though, they will conflict.

The failure mode is reasonable. You should get something like:

  $ GIT_TEST_HTTPD=1 LIB_HTTPD_PORT=80 ./t5540-http-push.sh
  1..0 # SKIP skipping test, web server setup failed

We could do better by retrying with a range of ports, but unless
somebody really needs that, I'd prefer to avoid complicating it more.

The one thing I have occasionally run into is a stale apache hanging
around. But I think that only happens when I am abusing the test script
(e.g., sticking a test_pause in, poking around, and then not letting the
script complete and do its cleanup).

So is the current behavior good enough to meet your needs and you
didn't know it, or do we need more?

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 06/10] fetch: pack v4 support on smart http

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git-fetch-pack.txt|  4 
 Documentation/gitremote-helpers.txt |  3 +++
 builtin/fetch-pack.c|  7 +++
 remote-curl.c   | 14 +-
 t/t5551-http-fetch.sh   |  9 +
 transport-helper.c  |  3 +++
 6 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-fetch-pack.txt b/Documentation/git-fetch-pack.txt
index 1e71754..b68cd45 100644
--- a/Documentation/git-fetch-pack.txt
+++ b/Documentation/git-fetch-pack.txt
@@ -87,6 +87,10 @@ be in a separate packet, and the list must end with a flush 
packet.
'git-upload-pack' treats the special depth 2147483647 as
infinite even if there is an ancestor-chain that long.
 
+--pack-version=n::
+   Define the preferred pack format version for data transfer.
+   Valid values are 2 and 4. Default is 2.
+
 --no-progress::
Do not show the progress.
 
diff --git a/Documentation/gitremote-helpers.txt 
b/Documentation/gitremote-helpers.txt
index 0827f69..90dfc2e 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -416,6 +416,9 @@ set by Git if the remote helper has the 'option' capability.
must not rely on this option being set before
connect request occurs.
 
+'option packv4 \{'true'|'false'\}::
+   Prefer pack version 4 (true) or 2 (false) for data transfer.
+
 SEE ALSO
 
 linkgit:git-remote[1]
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index aba4465..bfe940a 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -100,6 +100,13 @@ int cmd_fetch_pack(int argc, const char **argv, const char 
*prefix)
pack_lockfile_ptr = pack_lockfile;
continue;
}
+   if (!prefixcmp(arg, --pack-version=)) {
+   int ver = strtol(arg + 15, NULL, 0);
+   if (ver != 2  ver != 4)
+   die(_(invalid pack version %d), ver);
+   args.packv4 = ver == 4;
+   continue;
+   }
usage(fetch_pack_usage);
}
 
diff --git a/remote-curl.c b/remote-curl.c
index 5b3ce9e..1a3d215 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -18,6 +18,7 @@ struct options {
unsigned progress : 1,
followtags : 1,
dry_run : 1,
+   packv4 : 1,
thin : 1;
 };
 static struct options options;
@@ -67,6 +68,15 @@ static int set_option(const char *name, const char *value)
return -1;
return 0;
}
+   else if (!strcmp(name, packv4)) {
+   if (!strcmp(value, true))
+   options.packv4 = 1;
+   else if (!strcmp(value, false))
+   options.packv4 = 0;
+   else
+   return -1;
+   return 0;
+   }
else {
return 1 /* unsupported */;
}
@@ -654,7 +664,7 @@ static int fetch_git(struct discovery *heads,
struct strbuf preamble = STRBUF_INIT;
char *depth_arg = NULL;
int argc = 0, i, err;
-   const char *argv[15];
+   const char *argv[16];
 
argv[argc++] = fetch-pack;
argv[argc++] = --stateless-rpc;
@@ -676,6 +686,8 @@ static int fetch_git(struct discovery *heads,
depth_arg = strbuf_detach(buf, NULL);
argv[argc++] = depth_arg;
}
+   if (options.packv4)
+   argv[argc++] = --pack-version=4;
argv[argc++] = url;
argv[argc++] = NULL;
 
diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index 55a866a..5b4e6aa 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -222,5 +222,14 @@ test_expect_success EXPENSIVE 'clone the 50,000 tag repo 
to check OS command lin
)
 '
 
+test_expect_success 'clone http repository with pack v4' '
+   git -c transfer.unpackLimit=1 clone --pack-version=4 
$HTTPD_URL/smart/repo.git pv4 
+   P=`ls pv4/.git/objects/pack/pack-*.pack` 
+   # Offset 4 is pack version
+   test-dump ntohl $P 4 ver.actual 
+   echo 4 ver.expected 
+   test_cmp ver.expected ver.actual
+'
+
 stop_httpd
 test_done
diff --git a/transport-helper.c b/transport-helper.c
index 63cabc3..07fbf6e 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -350,6 +350,9 @@ static int fetch_with_fetch(struct transport *transport,
 
standard_options(transport);
 
+   set_helper_option(transport, packv4,
+ data-transport_options.packv4 ? true : false);
+
for (i = 0; i  nr_heads; i++) {
const struct ref *posn = to_fetch[i];
if (posn-status  REF_STATUS_UPTODATE)
-- 
1.8.2.82.gc24b958

--
To unsubscribe from this list: send the line unsubscribe git in
the body of 

Re: [PATCH] Document smart http

[Jeff King]

On Tue, Aug 20, 2013 at 12:08:08PM +0700, Nguyen Thai Ngoc Duy wrote:

 This may provide some clues for those who want to modify smart http
 code as smart http is pretty much undocumented. Smart http document
 so far is a few commit messages and the source code.

There was also this:

  http://article.gmane.org/gmane.comp.version-control.git/129734

which seems to have never gotten updated enough to be applied along with
the code. But with some updates to make sure it matches the current
behavior, it is probably a more comprehensive description.

But if you don't feel like spending more time on this on top of what
you've already done, I think the patch I'm responding to is better than
what we have now (i.e., nothing).

 +Reference Discovery
 +---
 +
 +The server end always sends the list of references in both push and
 +fetch cases. This ref list is retrieved by the client's sending HTTP
 +GET request to a smart http url ending with
 +/info/refs?service=service where service could be either
 +git-upload-pack or git-receive-pack for fetching or pushing
 +respectively. The output is in pkt-line format.
 +
 +
 +  advertised-refs  =  service
 +   flush-pkt
 +   (no-refs / list-of-refs)
 +   flush-pkt
 +
 +  service  =  PKT-LINE(# service= service-name)
 +  service-name =  (git-upload-pack / git-receive-pack)
 +
 +  no-refs  =  PKT-LINE(zero-id SP capabilities^{}
 +   NUL capability-list LF)
 +
 +  list-of-refs =  first-ref *other-ref
 +  first-ref=  PKT-LINE(obj-id SP refname
 +   NUL capability-list LF)
 +
 +  other-ref=  PKT-LINE(other-tip / other-peeled)
 +  other-tip=  obj-id SP refname LF
 +  other-peeled =  obj-id SP refname ^{} LF
 +
 +  capability-list  =  capability *(SP capability)
 +  capability   =  1*(LC_ALPHA / DIGIT / - / _)
 +  LC_ALPHA =  %x61-7A
 +

Most of this is a repeat of what is in the earlier sections. I don't
think the protocol is changing much and these are not likely to get out of
date with each other, but I wonder if it is easier on the reader to
simply describe the output in terms of what is added on top of the
regular ref advertisement (i.e., the service line). Something like:

  stateless-advertised-refs =  service
   advertised-refs

  service   =  PKT-LINE(# service= service-name)
  service-name  =  (git-upload-pack / git-receive-pack)

where advertised-refs is defined in the earlier BNF. You may also want
to note:

  Servers may respond to a smart request with a regular `advertised-refs`
  response rather than a `stateless-advertised-refs` response. In this
  case, the client MUST assume that the server does not understand smart
  HTTP and either abort or proceed with the non-smart protocol.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Document smart http

[Duy Nguyen]

On Tue, Aug 20, 2013 at 9:16 PM, Jeff King p...@peff.net wrote:
 On Tue, Aug 20, 2013 at 12:08:08PM +0700, Nguyen Thai Ngoc Duy wrote:

 This may provide some clues for those who want to modify smart http
 code as smart http is pretty much undocumented. Smart http document
 so far is a few commit messages and the source code.

 There was also this:

   http://article.gmane.org/gmane.comp.version-control.git/129734

 which seems to have never gotten updated enough to be applied along with
 the code. But with some updates to make sure it matches the current
 behavior, it is probably a more comprehensive description.

If I knew about this patch, it could have saved me a lot of time
reading remote-curl.c. Will check it with current code and resubmit an
update of this version instead.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Document smart http

[Nguyễn Thái Ngọc Duy]

This may provide some clues for those who want to modify smart http
code as smart http is pretty much undocumented. Smart http document
so far is a few commit messages and the source code.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git-fetch-pack.txt  | 11 +++--
 Documentation/git-receive-pack.txt| 12 +-
 Documentation/git-send-pack.txt   |  9 +++-
 Documentation/git-upload-pack.txt |  9 +++-
 Documentation/technical/pack-protocol.txt | 69 +++
 5 files changed, 103 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-fetch-pack.txt b/Documentation/git-fetch-pack.txt
index 1e71754..85a9437 100644
--- a/Documentation/git-fetch-pack.txt
+++ b/Documentation/git-fetch-pack.txt
@@ -9,10 +9,7 @@ git-fetch-pack - Receive missing objects from another 
repository
 SYNOPSIS
 
 [verse]
-'git fetch-pack' [--all] [--quiet|-q] [--keep|-k] [--thin] [--include-tag]
-   [--upload-pack=git-upload-pack]
-   [--depth=n] [--no-progress]
-   [-v] [host:]directory [refs...]
+'git fetch-pack' [options] [host:]directory [refs...]
 
 DESCRIPTION
 ---
@@ -90,6 +87,12 @@ be in a separate packet, and the list must end with a flush 
packet.
 --no-progress::
Do not show the progress.
 
+--stateless-rpc::
+   Smart HTTP mode.
+
+--lock-pack::
+   Issue lock command to the remote helper via stdout.
+
 -v::
Run verbosely.
 
diff --git a/Documentation/git-receive-pack.txt 
b/Documentation/git-receive-pack.txt
index b1f7dc6..0b2029c 100644
--- a/Documentation/git-receive-pack.txt
+++ b/Documentation/git-receive-pack.txt
@@ -9,7 +9,7 @@ git-receive-pack - Receive what is pushed into the repository
 SYNOPSIS
 
 [verse]
-'git-receive-pack' directory
+'git-receive-pack' [options] directory
 
 DESCRIPTION
 ---
@@ -35,6 +35,16 @@ are not fast-forwards.
 
 OPTIONS
 ---
+--stateless-rpc::
+   Smart HTTP mode.
+
+--advertise-refs::
+   Only the initial ref advertisement is output then exits
+   immediately.
+
+--quiet::
+   Make unpack-objects at the receive-pack end quiet.
+
 directory::
The repository to sync into.
 
diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index dc3a568..6cee3d4 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another 
repository
 SYNOPSIS
 
 [verse]
-'git send-pack' [--all] [--dry-run] [--force] 
[--receive-pack=git-receive-pack] [--verbose] [--thin] [host:]directory 
[ref...]
+'git send-pack' [options] [host:]directory [ref...]
 
 DESCRIPTION
 ---
@@ -52,6 +52,13 @@ OPTIONS
Send a thin pack, which records objects in deltified form based
on objects not included in the pack to reduce network traffic.
 
+--stateless-rpc::
+   Smart HTTP mode.
+
+--helper-status:
+   Issue status commands (e.g. ok or error) to the remote
+   helper via stdout.
+
 host::
A remote host to house the repository.  When this
part is specified, 'git-receive-pack' is invoked via
diff --git a/Documentation/git-upload-pack.txt 
b/Documentation/git-upload-pack.txt
index 0abc806..ce9a455 100644
--- a/Documentation/git-upload-pack.txt
+++ b/Documentation/git-upload-pack.txt
@@ -9,7 +9,7 @@ git-upload-pack - Send objects packed back to git-fetch-pack
 SYNOPSIS
 
 [verse]
-'git-upload-pack' [--strict] [--timeout=n] directory
+'git-upload-pack' [options] directory
 
 DESCRIPTION
 ---
@@ -31,6 +31,13 @@ OPTIONS
 --timeout=n::
Interrupt transfer after n seconds of inactivity.
 
+--stateless-rpc::
+   Smart HTTP mode.
+
+--advertise-refs::
+   Only the initial ref advertisement is output then exits
+   immediately.
+
 directory::
The repository to sync from.
 
diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index b898e97..7590394 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -546,3 +546,72 @@ An example client/server communication might look like 
this:
S: 0018ok refs/heads/debug\n
S: 002ang refs/heads/master non-fast-forward\n
 
+
+Smart HTTP Transport
+
+
+Smart HTTP protocol is basically git protocol on top of http. The
+base protocol is modified slightly to fit HTTP processing model: no
+bidirectional full-duplex connections, the program may read the
+request, write a response and must exit. Any negotiation is broken
+down into many separate GET or POST requests. The server is
+stateless. The client must send enough information so that the server
+can recreate the previous state.
+
+Reference Discovery
+---
+
+The server end always sends the list of references in both push and
+fetch cases. This ref list is retrieved by the client's sending HTTP
+GET request to a smart http

[PATCH v3 9/6] push: teach --force-with-lease to smart-http transport

[Junio C Hamano]

We have been passing enough information to enable the
compare-and-swap logic down to the transport layer, but the
transport helper was not passing it to smart-http transport.

Signed-off-by: Junio C Hamano gits...@pobox.com
---

 * I didn't bother with the dumb commit walker push for obvious
   reasons, but if somebody is inclined to add one, it shouldn't be
   too hard to add.

 remote-curl.c| 16 +++-
 t/lib-httpd.sh   |  3 ++-
 t/t5541-http-push.sh |  2 +-
 transport-helper.c   | 24 ++--
 4 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 60eda63..53c8a3d 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -6,6 +6,7 @@
 #include exec_cmd.h
 #include run-command.h
 #include pkt-line.h
+#include string-list.h
 #include sideband.h
 
 static struct remote *remote;
@@ -20,6 +21,7 @@ struct options {
thin : 1;
 };
 static struct options options;
+static struct string_list cas_options = STRING_LIST_INIT_DUP;
 
 static int set_option(const char *name, const char *value)
 {
@@ -66,6 +68,13 @@ static int set_option(const char *name, const char *value)
return -1;
return 0;
}
+   else if (!strcmp(name, cas)) {
+   struct strbuf val = STRBUF_INIT;
+   strbuf_addf(val, -- CAS_OPT_NAME =%s, value);
+   string_list_append(cas_options, val.buf);
+   strbuf_release(val);
+   return 0;
+   }
else {
return 1 /* unsupported */;
}
@@ -789,8 +798,9 @@ static int push_git(struct discovery *heads, int nr_spec, 
char **specs)
struct rpc_state rpc;
const char **argv;
int argc = 0, i, err;
+   struct string_list_item *cas_option;
 
-   argv = xmalloc((10 + nr_spec) * sizeof(char*));
+   argv = xmalloc((10 + nr_spec + cas_options.nr) * sizeof(char *));
argv[argc++] = send-pack;
argv[argc++] = --stateless-rpc;
argv[argc++] = --helper-status;
@@ -803,6 +813,10 @@ static int push_git(struct discovery *heads, int nr_spec, 
char **specs)
else if (options.verbosity  1)
argv[argc++] = --verbose;
argv[argc++] = options.progress ? --progress : --no-progress;
+
+   for_each_string_list_item(cas_option, cas_options)
+   argv[argc++] = cas_option-string;
+
argv[argc++] = url;
for (i = 0; i  nr_spec; i++)
argv[argc++] = specs[i];
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index e2eca1f..dab405d 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -141,10 +141,11 @@ stop_httpd() {
-f $TEST_PATH/apache.conf $HTTPD_PARA -k stop
 }
 
-test_http_push_nonff() {
+test_http_push_nonff () {
REMOTE_REPO=$1
LOCAL_REPO=$2
BRANCH=$3
+   EXPECT_CAS_RESULT=${4-failure}
 
test_expect_success 'non-fast-forward push fails' '
cd $REMOTE_REPO 
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index beb00be..470ac54 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -153,7 +153,7 @@ test_expect_success 'used receive-pack service' '
 '
 
 test_http_push_nonff $HTTPD_DOCUMENT_ROOT_PATH/test_repo.git \
-   $ROOT_PATH/test_repo_clone master
+   $ROOT_PATH/test_repo_clone master success
 
 test_expect_success 'push fails for non-fast-forward refs unmatched by remote 
helper' '
# create a dissimilarly-named remote ref so that git is unable to match 
the
diff --git a/transport-helper.c b/transport-helper.c
index 95d22f8..e3a60d7 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -742,13 +742,15 @@ static void push_update_refs_status(struct helper_data 
*data,
 }
 
 static int push_refs_with_push(struct transport *transport,
-   struct ref *remote_refs, int flags)
+  struct ref *remote_refs, int flags)
 {
int force_all = flags  TRANSPORT_PUSH_FORCE;
int mirror = flags  TRANSPORT_PUSH_MIRROR;
struct helper_data *data = transport-data;
struct strbuf buf = STRBUF_INIT;
struct ref *ref;
+   struct string_list cas_options = STRING_LIST_INIT_DUP;
+   struct string_list_item *cas_option;
 
get_helper(transport);
if (!data-push)
@@ -784,11 +786,29 @@ static int push_refs_with_push(struct transport 
*transport,
strbuf_addch(buf, ':');
strbuf_addstr(buf, ref-name);
strbuf_addch(buf, '\n');
+
+   /*
+* The --force-with-lease options without explicit
+* values to expect have already been expanded into
+* the ref-old_sha1_expect[] field; we can ignore
+* transport-smart_options-cas altogether and instead
+* can enumerate them from the refs.
+*/
+   if (ref-expect_old_sha1) {
+   struct

[PATCH v3 7/6] t5540/5541: smart-http does not support --force-with-lease

[Junio C Hamano]

The push() method in remote-curl.c is not told and does not pass the
necessary information to underlying send-pack, so this extension
does not yet work.  Leave a note in the test suite.

Signed-off-by: Junio C Hamano gits...@pobox.com
---

 * This is primarily to give a target for other people to shoot at;
   patches to allow us to flip this expect_failure to expect_success
   are very much welcomed ;-)

 t/lib-httpd.sh | 16 
 1 file changed, 16 insertions(+)

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 895b925..e2eca1f 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -167,6 +167,22 @@ test_http_push_nonff() {
test_expect_success 'non-fast-forward push shows help message' '
test_i18ngrep Updates were rejected because output
'
+
+   test_expect_failure 'force with lease aka cas' '
+   HEAD=$( cd $REMOTE_REPO  git rev-parse --verify HEAD ) 
+   test_when_finished '\''
+   (cd $REMOTE_REPO  git update-ref HEAD $HEAD)
+   '\'' 
+   (
+   cd $LOCAL_REPO 
+   git push -v --force-with-lease=$BRANCH:$HEAD origin
+   ) 
+   git rev-parse --verify $BRANCH expect 
+   (
+   cd $REMOTE_REPO  git rev-parse --verify HEAD
+   ) actual 
+   test_cmp expect actual
+   '
 }
 
 setup_askpass_helper() {
-- 
1.8.4-rc0-154-gc668397
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Let index-pack help with connectivity check on cloning via smart http

[Nguyễn Thái Ngọc Duy]

This is an extension of c6807a4 (clone: open a shortcut for
connectivity check - 2013-05-26) to reduce the cost of connectivity
check at clone time, this time with smart http protocol.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git-fetch-pack.txt|  4 
 Documentation/gitremote-helpers.txt | 10 ++
 builtin/fetch-pack.c|  9 +
 remote-curl.c   | 15 ++-
 transport-helper.c  | 10 ++
 5 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-fetch-pack.txt b/Documentation/git-fetch-pack.txt
index 1e71754..444b805 100644
--- a/Documentation/git-fetch-pack.txt
+++ b/Documentation/git-fetch-pack.txt
@@ -90,6 +90,10 @@ be in a separate packet, and the list must end with a flush 
packet.
 --no-progress::
Do not show the progress.
 
+--check-self-contained-and-connected::
+   Output connectivity-ok if the received pack is
+   self-contained and connected.
+
 -v::
Run verbosely.
 
diff --git a/Documentation/gitremote-helpers.txt 
b/Documentation/gitremote-helpers.txt
index 0827f69..bc069c2 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -143,6 +143,10 @@ Supported commands: 'list', 'fetch'.
 +
 Supported commands: 'list', 'import'.
 
+'check-connectivity'::
+   Can guarantee that when a clone is requested, the received
+   pack is self contained and is connected.
+
 If a helper advertises 'connect', Git will use it if possible and
 fall back to another capability if the helper requests so when
 connecting (see the 'connect' command under COMMANDS).
@@ -270,6 +274,9 @@ Optionally may output a 'lock file' line indicating a 
file under
 GIT_DIR/objects/pack which is keeping a pack until refs can be
 suitably updated.
 +
+If option 'check-connectivity' is requested, the helper must output
+'connectivity-ok' if the clone is self-contained and connected.
++
 Supported if the helper has the fetch capability.
 
 'push' +src:dst::
@@ -416,6 +423,9 @@ set by Git if the remote helper has the 'option' capability.
must not rely on this option being set before
connect request occurs.
 
+'option check-connectivity' \{'true'|'false'\}::
+   Request the helper to check connectivity of a clone.
+
 SEE ALSO
 
 linkgit:git-remote[1]
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index aba4465..3e19d71 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -100,6 +100,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char 
*prefix)
pack_lockfile_ptr = pack_lockfile;
continue;
}
+   if (!strcmp(--check-self-contained-and-connected, arg)) {
+   args.check_self_contained_and_connected = 1;
+   continue;
+   }
usage(fetch_pack_usage);
}
 
@@ -152,6 +156,11 @@ int cmd_fetch_pack(int argc, const char **argv, const char 
*prefix)
printf(lock %s\n, pack_lockfile);
fflush(stdout);
}
+   if (args.check_self_contained_and_connected 
+   args.self_contained_and_connected) {
+   printf(connectivity-ok\n);
+   fflush(stdout);
+   }
close(fd[0]);
close(fd[1]);
if (finish_connect(conn))
diff --git a/remote-curl.c b/remote-curl.c
index 5b3ce9e..6918668 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -16,6 +16,7 @@ struct options {
int verbosity;
unsigned long depth;
unsigned progress : 1,
+   check_self_contained_and_connected : 1,
followtags : 1,
dry_run : 1,
thin : 1;
@@ -67,6 +68,15 @@ static int set_option(const char *name, const char *value)
return -1;
return 0;
}
+   else if (!strcmp(name, check-connectivity)) {
+   if (!strcmp(value, true))
+   options.check_self_contained_and_connected = 1;
+   else if (!strcmp(value, false))
+   options.check_self_contained_and_connected = 0;
+   else
+   return -1;
+   return 0;
+   }
else {
return 1 /* unsupported */;
}
@@ -654,7 +664,7 @@ static int fetch_git(struct discovery *heads,
struct strbuf preamble = STRBUF_INIT;
char *depth_arg = NULL;
int argc = 0, i, err;
-   const char *argv[15];
+   const char *argv[16];
 
argv[argc++] = fetch-pack;
argv[argc++] = --stateless-rpc;
@@ -668,6 +678,8 @@ static int fetch_git(struct discovery *heads,
argv[argc++] = -v;
argv[argc++] = -v;
}
+   if (options.check_self_contained_and_connected)
+   argv[argc++] = --check-self-contained-and-connected

[PATCH v2 10/16] Add document for command arguments for supporting smart http

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git-fetch-pack.txt   | 11 +++
 Documentation/git-receive-pack.txt | 16 +++-
 Documentation/git-send-pack.txt|  9 -
 Documentation/git-upload-pack.txt  | 13 -
 4 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-fetch-pack.txt b/Documentation/git-fetch-pack.txt
index 1e71754..85a9437 100644
--- a/Documentation/git-fetch-pack.txt
+++ b/Documentation/git-fetch-pack.txt
@@ -9,10 +9,7 @@ git-fetch-pack - Receive missing objects from another 
repository
 SYNOPSIS
 
 [verse]
-'git fetch-pack' [--all] [--quiet|-q] [--keep|-k] [--thin] [--include-tag]
-   [--upload-pack=git-upload-pack]
-   [--depth=n] [--no-progress]
-   [-v] [host:]directory [refs...]
+'git fetch-pack' [options] [host:]directory [refs...]
 
 DESCRIPTION
 ---
@@ -90,6 +87,12 @@ be in a separate packet, and the list must end with a flush 
packet.
 --no-progress::
Do not show the progress.
 
+--stateless-rpc::
+   Smart HTTP mode.
+
+--lock-pack::
+   Issue lock command to the remote helper via stdout.
+
 -v::
Run verbosely.
 
diff --git a/Documentation/git-receive-pack.txt 
b/Documentation/git-receive-pack.txt
index b1f7dc6..b56d2eb 100644
--- a/Documentation/git-receive-pack.txt
+++ b/Documentation/git-receive-pack.txt
@@ -9,7 +9,7 @@ git-receive-pack - Receive what is pushed into the repository
 SYNOPSIS
 
 [verse]
-'git-receive-pack' directory
+'git-receive-pack' [options] directory
 
 DESCRIPTION
 ---
@@ -35,6 +35,20 @@ are not fast-forwards.
 
 OPTIONS
 ---
+--stateless-rpc::
+   git-receive-pack performs only a single read-write cycle with
+   stdin and stdout to fit with the HTTP POST request processing
+   model where a program may read the request, write a response,
+   and must exit.
+
+--advertise-refs::
+   Only the initial ref advertisement is output then exits
+   immediately to fit with the HTTP GET request model, where no
+   request content is received but a response must be produced.
+
+--quiet::
+   Make unpack-objects at the receive-pack end quiet.
+
 directory::
The repository to sync into.
 
diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index dc3a568..a88e7e0 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another 
repository
 SYNOPSIS
 
 [verse]
-'git send-pack' [--all] [--dry-run] [--force] 
[--receive-pack=git-receive-pack] [--verbose] [--thin] [host:]directory 
[ref...]
+'git send-pack' [options] [host:]directory [ref...]
 
 DESCRIPTION
 ---
@@ -52,6 +52,13 @@ OPTIONS
Send a thin pack, which records objects in deltified form based
on objects not included in the pack to reduce network traffic.
 
+--stateless-rpc::
+   Smart HTTP mode.
+
+--helper-status:
+   Issue status commands (e.g. ok or error) to the remote
+   help via stdout.
+
 host::
A remote host to house the repository.  When this
part is specified, 'git-receive-pack' is invoked via
diff --git a/Documentation/git-upload-pack.txt 
b/Documentation/git-upload-pack.txt
index 0abc806..98d73cc 100644
--- a/Documentation/git-upload-pack.txt
+++ b/Documentation/git-upload-pack.txt
@@ -9,7 +9,7 @@ git-upload-pack - Send objects packed back to git-fetch-pack
 SYNOPSIS
 
 [verse]
-'git-upload-pack' [--strict] [--timeout=n] directory
+'git-upload-pack' [options] directory
 
 DESCRIPTION
 ---
@@ -31,6 +31,17 @@ OPTIONS
 --timeout=n::
Interrupt transfer after n seconds of inactivity.
 
+--stateless-rpc::
+   git-upload-pack performs only a single read-write cycle with
+   stdin and stdout to fit with the HTTP POST request processing
+   model where a program may read the request, write a response,
+   and must exit.
+
+--advertise-refs::
+   Only the initial ref advertisement is output then exits
+   immediately to fit with the HTTP GET request model, where no
+   request content is received but a response must be produced.
+
 directory::
The repository to sync from.
 
-- 
1.8.2.83.gc99314b

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 09/16] pack-protocol.txt: a bit about smart http

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/technical/pack-protocol.txt | 66 +++
 1 file changed, 66 insertions(+)

diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index c73b62f..a1672bc 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -551,3 +551,69 @@ An example client/server communication might look like 
this:
S: 0018ok refs/heads/debug\n
S: 002ang refs/heads/master non-fast-forward\n
 
+
+Smart HTTP Transport
+
+
+Smart HTTP protocol is basically git protocol on top of http. The
+base protocol is modified slightly to fit HTTP processing model: no
+bidirectional full-duplex connections, the program may read the
+request, write a response and must exit.
+
+Reference Discovery
+---
+
+The server end always sends the list of references in both push and
+fetch cases. This ref list is retrieved by the client's sending HTTP
+GET request to a smart http url ending with
+/info/refs?service=service where service could be either
+git-upload-pack or git-receive-pack for fetching or pushing
+respectively. The output is in pkt-line format.
+
+
+  advertised-refs  =  service
+ flush-pkt
+ (no-refs / list-of-refs)
+ flush-pkt
+
+  service  =  PKT-LINE(# service= service-name)
+  service-name =  (git-upload-pack / git-receive-pack)
+
+  no-refs  =  PKT-LINE(zero-id SP capabilities^{}
+ NUL capability-list LF)
+
+  list-of-refs =  first-ref *other-ref
+  first-ref=  PKT-LINE(obj-id SP refname
+ NUL capability-list LF)
+
+  other-ref=  PKT-LINE(other-tip / other-peeled)
+  other-tip=  obj-id SP refname LF
+  other-peeled =  obj-id SP refname ^{} LF
+
+  capability-list  =  capability *(SP capability)
+  capability   =  1*(LC_ALPHA / DIGIT / - / _)
+  LC_ALPHA =  %x61-7A
+
+
+Packfile Negotiation
+
+
+For fetching, packet negotiation is via a series of HTTP POST requests
+to an url ending with /git-upload-pack with the content in pkt-line
+format. git-upload-pack's response consists of a service line like
+in Reference Discovery followed by normal git-upload-pack packet
+lines. Capability multi_ack_detailed is required by Smart HTTP.
+
+Common objects that are discovered are appended onto the request as
+have lines and are sent again on the next request. This allows the
+remote side to reinitialize its in-memory list of common objects
+during the next request and the remote does not need to maintain the
+negotiation state.
+
+Reference Update Request
+
+
+For pushing, a HTTP POST request is sent to an url ending with
+/git-receive-pack with the content in pkt-line format.
+git-receive-pack's response consists of a service line like in
+Reference Discovery followed by normal git-receive-pack packet lines.
-- 
1.8.2.83.gc99314b

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 11/16] {fetch,upload}-pack: support fetching from a shallow clone via smart http

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/technical/pack-protocol.txt |  3 +++
 builtin/fetch-pack.c  |  3 +--
 remote-curl.c |  4 +++-
 t/t5536-fetch-shallow.sh  | 27 +++
 upload-pack.c |  2 --
 5 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index a1672bc..5013652 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -574,6 +574,7 @@ respectively. The output is in pkt-line format.
   advertised-refs  =  service
  flush-pkt
  (no-refs / list-of-refs)
+ *shallow
  flush-pkt
 
   service  =  PKT-LINE(# service= service-name)
@@ -590,6 +591,8 @@ respectively. The output is in pkt-line format.
   other-tip=  obj-id SP refname LF
   other-peeled =  obj-id SP refname ^{} LF
 
+  shallow   =  PKT-LINE(shallow SP obj-id)
+
   capability-list  =  capability *(SP capability)
   capability   =  1*(LC_ALPHA / DIGIT / - / _)
   LC_ALPHA =  %x61-7A
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index f6a6d76..b89d753 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -146,8 +146,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char 
*prefix)
}
 
memset(shallow, 0, sizeof(shallow));
-   get_remote_heads(fd[0], NULL, 0, ref, 0, NULL,
-args.stateless_rpc ? NULL : shallow);
+   get_remote_heads(fd[0], NULL, 0, ref, 0, NULL, shallow);
 
ref = fetch_pack(args, fd, conn, ref, dest,
 sought, nr_sought, shallow, pack_lockfile_ptr);
diff --git a/remote-curl.c b/remote-curl.c
index c329bd3..de2cc8a 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -78,6 +78,7 @@ struct discovery {
char *buf;
size_t len;
struct ref *refs;
+   struct extra_have_objects shallow;
unsigned proto_git : 1;
 };
 static struct discovery *last_discovery;
@@ -86,7 +87,7 @@ static struct ref *parse_git_refs(struct discovery *heads, 
int for_push)
 {
struct ref *list = NULL;
get_remote_heads(-1, heads-buf, heads-len, list,
-for_push ? REF_NORMAL : 0, NULL, NULL);
+for_push ? REF_NORMAL : 0, NULL, heads-shallow);
return list;
 }
 
@@ -146,6 +147,7 @@ static void free_discovery(struct discovery *d)
if (d) {
if (d == last_discovery)
last_discovery = NULL;
+   free(d-shallow.array);
free(d-buf_alloc);
free_refs(d-refs);
free(d);
diff --git a/t/t5536-fetch-shallow.sh b/t/t5536-fetch-shallow.sh
index 15a8208..6ea6347 100755
--- a/t/t5536-fetch-shallow.sh
+++ b/t/t5536-fetch-shallow.sh
@@ -102,4 +102,31 @@ EOF
 
 '
 
+if test -n $NO_CURL -o -z $GIT_TEST_HTTPD; then
+   say 'skipping remaining tests, git built without http support'
+   test_done
+fi
+
+LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5536'}
+. $TEST_DIRECTORY/lib-httpd.sh
+start_httpd
+
+test_expect_success 'clone http repository' '
+   git clone --bare --no-local shallow 
$HTTPD_DOCUMENT_ROOT_PATH/repo.git 
+   git clone --quiet $HTTPD_URL/smart/repo.git clone 
+   (
+   cd clone 
+   git fsck 
+   git log --format=%s origin/master actual 
+   cat EOF expect 
+6
+5
+4
+3
+EOF
+   test_cmp expect actual
+   )
+'
+
+stop_httpd
 test_done
diff --git a/upload-pack.c b/upload-pack.c
index c3e68ae..263ae08 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -770,8 +770,6 @@ int main(int argc, char **argv)
 
if (!enter_repo(dir, strict))
die('%s' does not appear to be a git repository, dir);
-   if (is_repository_shallow()  stateless_rpc)
-   die(attempt to push into a shallow repository);
 
git_config(upload_pack_config, NULL);
upload_pack();
-- 
1.8.2.83.gc99314b

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] remote-curl: verify smart-http metadata lines

[Jeff King]

On Sun, Feb 17, 2013 at 04:54:43PM -0800, Jonathan Nieder wrote:

  My intent was that it followed the error convention of negative is
  error, 0 is success, and positive is not used, but reserved for
  future use.
 
 From a maintainability perspective, that kind of contract would be
 dangerous, since some *other* caller could arrive and use the function
 without a  0 without knowing it is doing anything wrong.  When new
 return values appear, the function should be renamed to help the patch
 author and reviewers remember to check all callers.

True. That's why I always write  0. :)

 That is, from the point of view of maintainability, there is no
 distinction between if (read_packets_until_...  0) and
 if (read_packets_until_...) and either form is fine.
 
 My comment was just to say the  0 forced me to pause a moment and
 check out the implementation.  This is basically a stylistic thing and
 if you prefer to keep the  0, that's fine with me.

Interesting. To me, foo()  0 just reads idiomatically as error-check
the foo call.

Anyway, I've redone the patch series to just re-use get_remote_heads,
which is more robust. So this function has gone away in the new version.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] remote-curl: verify smart-http metadata lines

[Jonathan Nieder]

Jeff King wrote:

 --- a/remote-curl.c
 +++ b/remote-curl.c
[...]
 @@ -155,11 +166,13 @@ static struct discovery* discover_refs(const char 
 *service)
[...]
 - strbuf_reset(buffer);
 - while (packet_get_line(buffer, last-buf, last-len)  0)
 - strbuf_reset(buffer);
 + if (read_packets_until_flush(last-buf, last-len)  0)

Style nit: this made me wonder What would it mean if
read_packets_until_flush()  0?  Since the convention for this
function is 0 for success, I would personally find

if (read_packets_until_flush(...))
handle error;

easier to read.

 + die(smart-http metadata lines are invalid at %s,
 + refs_url);

Especially given that other clients would be likely to run into
trouble in the same situation, as long as this cooks in next for a
suitable amount of time to catch bad servers, it looks like a good
idea.

Reviewed-by: Jonathan Nieder jrnie...@gmail.com

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] remote-curl: verify smart-http metadata lines

[Jeff King]

On Sun, Feb 17, 2013 at 02:49:39AM -0800, Jonathan Nieder wrote:

 Jeff King wrote:
 
  --- a/remote-curl.c
  +++ b/remote-curl.c
 [...]
  @@ -155,11 +166,13 @@ static struct discovery* discover_refs(const char 
  *service)
 [...]
  -   strbuf_reset(buffer);
  -   while (packet_get_line(buffer, last-buf, last-len)  0)
  -   strbuf_reset(buffer);
  +   if (read_packets_until_flush(last-buf, last-len)  0)
 
 Style nit: this made me wonder What would it mean if
 read_packets_until_flush()  0?  Since the convention for this
 function is 0 for success, I would personally find
 
   if (read_packets_until_flush(...))
   handle error;
 
 easier to read.

My intent was that it followed the error convention of negative is
error, 0 is success, and positive is not used, but reserved for
future use. And I tend to think the  0 makes it obvious that we are
interested in error. But I don't feel that strongly, so if people would
rather see it the other way, I can live with it.

  +   die(smart-http metadata lines are invalid at %s,
  +   refs_url);
 
 Especially given that other clients would be likely to run into
 trouble in the same situation, as long as this cooks in next for a
 suitable amount of time to catch bad servers, it looks like a good
 idea.

Yeah, I have a slight concern that this series would break something in
another implementation, so I would like to see this cook in next for a
while (and would be slated for master probably not in this release, but
in the next one). But I think this change is pretty straightforward. If
an implementation is producing bogus packet lines and expecting us not
to complain, it really needs to be fixed.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] remote-curl: verify smart-http metadata lines

[Jonathan Nieder]

Jeff King wrote:
 On Sun, Feb 17, 2013 at 02:49:39AM -0800, Jonathan Nieder wrote:
 Jeff King wrote:

 --- a/remote-curl.c
[...]
 +   if (read_packets_until_flush(last-buf, last-len)  0)
 
 Style nit: this made me wonder What would it mean if
 read_packets_until_flush()  0?
[...]
 My intent was that it followed the error convention of negative is
 error, 0 is success, and positive is not used, but reserved for
 future use.

From a maintainability perspective, that kind of contract would be
dangerous, since some *other* caller could arrive and use the function
without a  0 without knowing it is doing anything wrong.  When new
return values appear, the function should be renamed to help the patch
author and reviewers remember to check all callers.

That is, from the point of view of maintainability, there is no
distinction between if (read_packets_until_...  0) and
if (read_packets_until_...) and either form is fine.

My comment was just to say the  0 forced me to pause a moment and
check out the implementation.  This is basically a stylistic thing and
if you prefer to keep the  0, that's fine with me.

  If
 an implementation is producing bogus packet lines and expecting us not
 to complain, it really needs to be fixed.

Agreed completely.  Thanks again for the patch.

Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] make smart-http more robust against bogus server input

[Jeff King]

For the most part, smart-http just passes data to fetch-pack and
send-pack, which take care of the heavy lifting. However, I did find a
few corner cases around truncated data from the server, one of which can
actually cause a deadlock.

I found these because I was trying to figure out what was going on with
some hung git processes which were in a deadlock like the one described
in patch 3. But having experimented and read the code, I don't think
that it is triggerable from a normal clone, but rather only when you
poke git-remote-curl in the right way. So it may or may not be my
culprit, but these patches do make remote-curl more robust, which is a
good thing.

  [1/3]: pkt-line: teach packet_get_line a no-op mode
  [2/3]: remote-curl: verify smart-http metadata lines
  [3/3]: remote-curl: sanity check ref advertisement from server

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] remote-curl: verify smart-http metadata lines

[Jeff King]

A smart http ref advertisement starts with a packet
containing the service header, followed by an arbitrary
number of packets containing other metadata headers,
followed by a flush packet.

We don't currently recognize any other metadata headers, so
we just parse through any extra packets, throwing away their
contents. However, we don't do so very carefully, and just
stop at the first error or flush packet.

Let's flag any errors we see here, which might be a sign of
truncated or corrupted output. Since the rest of the data
should be the ref advertisement, and since we pass that
along to our helper programs (like fetch-pack), they will
probably notice the error, as whatever cruft is in the
buffer will not parse. However, it's nice to note problems
as early as possible, which can help in debugging the root
cause.

Signed-off-by: Jeff King p...@peff.net
---
 remote-curl.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 933c69a..73134f5 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -90,6 +90,17 @@ static void free_discovery(struct discovery *d)
}
 }
 
+static int read_packets_until_flush(char **buf, size_t *len)
+{
+   while (1) {
+   int r = packet_get_line(NULL, buf, len);
+   if (r  0)
+   return -1;
+   if (r == 0)
+   return 0;
+   }
+}
+
 static struct discovery* discover_refs(const char *service)
 {
struct strbuf exp = STRBUF_INIT;
@@ -155,11 +166,13 @@ static struct discovery* discover_refs(const char 
*service)
 
/* The header can include additional metadata lines, up
 * until a packet flush marker.  Ignore these now, but
-* in the future we might start to scan them.
+* in the future we might start to scan them. However, we do
+* still check to make sure we are getting valid packet lines,
+* ending with a flush.
 */
-   strbuf_reset(buffer);
-   while (packet_get_line(buffer, last-buf, last-len)  0)
-   strbuf_reset(buffer);
+   if (read_packets_until_flush(last-buf, last-len)  0)
+   die(smart-http metadata lines are invalid at %s,
+   refs_url);
 
last-proto_git = 1;
}
-- 
1.8.1.2.11.g1a2f572

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Verify Content-Type from smart HTTP servers

[Michael Schubert]

On 01/31/2013 11:09 PM, Junio C Hamano wrote:

  
 -static int http_request_reauth(const char *url, void *result, int target,
 +static int http_request_reauth(const char *url,
 +struct strbuf *type,
 +void *result, int target,
  int options)
  {
 - int ret = http_request(url, result, target, options);
 + int ret = http_request(url, type, result, target, options);
   if (ret != HTTP_REAUTH)
   return ret;
 - return http_request(url, result, target, options);
 + return http_request(url, type, result, target, options);
  }

This needs something like

diff --git a/http.c b/http.c
index d868d8b..da43be3 100644
--- a/http.c
+++ b/http.c
@@ -860,6 +860,8 @@ static int http_request_reauth(const char *url,
int ret = http_request(url, type, result, target, options);
if (ret != HTTP_REAUTH)
return ret;
+   if (type)
+   strbuf_reset(type);
return http_request(url, type, result, target, options);
 }

on top. Otherwise we get

text/plainapplication/x-git-receive-pack-advertisement

when doing HTTP auth.

Thanks.

 -int http_get_strbuf(const char *url, struct strbuf *result, int options)
 +int http_get_strbuf(const char *url,
 + struct strbuf *type,
 + struct strbuf *result, int options)
  {
 - return http_request_reauth(url, result, HTTP_REQUEST_STRBUF, options);
 + return http_request_reauth(url, type, result,
 +HTTP_REQUEST_STRBUF, options);
  }
  
  /*
 @@ -878,7 +891,7 @@ static int http_get_file(const char *url, const char 
 *filename, int options)
   goto cleanup;
   }
  
 - ret = http_request_reauth(url, result, HTTP_REQUEST_FILE, options);
 + ret = http_request_reauth(url, NULL, result, HTTP_REQUEST_FILE, 
 options);
   fclose(result);
  
   if ((ret == HTTP_OK)  move_temp_to_file(tmpfile.buf, filename))
 @@ -904,7 +917,7 @@ int http_fetch_ref(const char *base, struct ref *ref)
   int ret = -1;
  
   url = quote_ref_url(base, ref-name);
 - if (http_get_strbuf(url, buffer, HTTP_NO_CACHE) == HTTP_OK) {
 + if (http_get_strbuf(url, NULL, buffer, HTTP_NO_CACHE) == HTTP_OK) {
   strbuf_rtrim(buffer);
   if (buffer.len == 40)
   ret = get_sha1_hex(buffer.buf, ref-old_sha1);
 @@ -997,7 +1010,7 @@ int http_get_info_packs(const char *base_url, struct 
 packed_git **packs_head)
   strbuf_addstr(buf, objects/info/packs);
   url = strbuf_detach(buf, NULL);
  
 - ret = http_get_strbuf(url, buf, HTTP_NO_CACHE);
 + ret = http_get_strbuf(url, NULL, buf, HTTP_NO_CACHE);
   if (ret != HTTP_OK)
   goto cleanup;
  
 diff --git a/http.h b/http.h
 index 0a80d30..25d1931 100644
 --- a/http.h
 +++ b/http.h
 @@ -132,7 +132,7 @@ extern char *get_remote_object_url(const char *url, const 
 char *hex,
   *
   * If the result pointer is NULL, a HTTP HEAD request is made instead of GET.
   */
 -int http_get_strbuf(const char *url, struct strbuf *result, int options);
 +int http_get_strbuf(const char *url, struct strbuf *content_type, struct 
 strbuf *result, int options);
  
  /*
   * Prints an error message using error() containing url and curl_errorstr,
 diff --git a/remote-curl.c b/remote-curl.c
 index 9a8b123..e6f3b63 100644
 --- a/remote-curl.c
 +++ b/remote-curl.c
 @@ -92,6 +92,8 @@ static void free_discovery(struct discovery *d)
  
  static struct discovery* discover_refs(const char *service)
  {
 + struct strbuf exp = STRBUF_INIT;
 + struct strbuf type = STRBUF_INIT;
   struct strbuf buffer = STRBUF_INIT;
   struct discovery *last = last_discovery;
   char *refs_url;
 @@ -113,7 +115,7 @@ static struct discovery* discover_refs(const char 
 *service)
   }
   refs_url = strbuf_detach(buffer, NULL);
  
 - http_ret = http_get_strbuf(refs_url, buffer, HTTP_NO_CACHE);
 + http_ret = http_get_strbuf(refs_url, type, buffer, HTTP_NO_CACHE);
   switch (http_ret) {
   case HTTP_OK:
   break;
 @@ -133,16 +135,19 @@ static struct discovery* discover_refs(const char 
 *service)
   last-buf = last-buf_alloc;
  
   if (maybe_smart  5 = last-len  last-buf[4] == '#') {
 - /* smart HTTP response; validate that the service
 + /*
 +  * smart HTTP response; validate that the service
* pkt-line matches our request.
*/
 - struct strbuf exp = STRBUF_INIT;
 -
 + strbuf_addf(exp, application/x-%s-advertisement, service);
 + if (strbuf_cmp(exp, type))
 + die(invalid content-type %s, type.buf);
   if (packet_get_line(buffer, last-buf, last-len) = 0)
   die(%s has invalid packet header, refs_url);
   if (buffer.len  buffer.buf[buffer.len - 1] == '\n

Re: [PATCH] Verify Content-Type from smart HTTP servers

[Jeff King]

On Wed, Feb 06, 2013 at 11:24:41AM +0100, Michael Schubert wrote:

 On 01/31/2013 11:09 PM, Junio C Hamano wrote:
 
   
  -static int http_request_reauth(const char *url, void *result, int target,
  +static int http_request_reauth(const char *url,
  +  struct strbuf *type,
  +  void *result, int target,
 int options)
   {
  -   int ret = http_request(url, result, target, options);
  +   int ret = http_request(url, type, result, target, options);
  if (ret != HTTP_REAUTH)
  return ret;
  -   return http_request(url, result, target, options);
  +   return http_request(url, type, result, target, options);
   }
 
 This needs something like
 
 diff --git a/http.c b/http.c
 index d868d8b..da43be3 100644
 --- a/http.c
 +++ b/http.c
 @@ -860,6 +860,8 @@ static int http_request_reauth(const char *url,
 int ret = http_request(url, type, result, target, options);
 if (ret != HTTP_REAUTH)
 return ret;
 +   if (type)
 +   strbuf_reset(type);
 return http_request(url, type, result, target, options);
  }
 
 on top. Otherwise we get
 
 text/plainapplication/x-git-receive-pack-advertisement
 
 when doing HTTP auth.

Good catch. It probably makes sense to put it in http_request, so that
we also protect against any existing cruft from the callers of
http_get_*, like:

-- 8 --
Subject: [PATCH] http_request: reset type strbuf before adding

Callers may pass us a strbuf which we use to record the
content-type of the response. However, we simply appended to
it rather than overwriting its contents, meaning that cruft
in the strbuf gave us a bogus type. E.g., the multiple
requests triggered by http_request could yield a type like
text/plainapplication/x-git-receive-pack-advertisement.

Reported-by: Michael Schubert msc...@elegosoft.com
Signed-off-by: Jeff King p...@peff.net
---
Is it worth having a strbuf_set* family of functions to match the
strbuf_add*? We seem to have these sorts of errors with strbuf from time
to time, and I wonder if that would make it easier (and more readable)
to do the right thing.

 http.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/http.c b/http.c
index d868d8b..d9d1aad 100644
--- a/http.c
+++ b/http.c
@@ -841,6 +841,7 @@ static int http_request(const char *url, struct strbuf 
*type,
 
if (type) {
char *t;
+   strbuf_reset(type);
curl_easy_getinfo(slot-curl, CURLINFO_CONTENT_TYPE, t);
if (t)
strbuf_addstr(type, t);
-- 
1.8.1.2.11.g1a2f572

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Verify Content-Type from smart HTTP servers

[Junio C Hamano]

Jeff King p...@peff.net writes:

 Is it worth having a strbuf_set* family of functions to match the
 strbuf_add*? We seem to have these sorts of errors with strbuf from time
 to time, and I wonder if that would make it easier (and more readable)
 to do the right thing.

Possibly.

The callsite below may be a poor example, though; you would need the
_reset() even if you change the _addstr() we can see in the context
to _setstr() to make sure later strbuf_*(type) will start from a
clean slate when !t anyway, no?


  http.c | 1 +
  1 file changed, 1 insertion(+)

 diff --git a/http.c b/http.c
 index d868d8b..d9d1aad 100644
 --- a/http.c
 +++ b/http.c
 @@ -841,6 +841,7 @@ static int http_request(const char *url, struct strbuf 
 *type,
  
   if (type) {
   char *t;
 + strbuf_reset(type);
   curl_easy_getinfo(slot-curl, CURLINFO_CONTENT_TYPE, t);
   if (t)
   strbuf_addstr(type, t);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Verify Content-Type from smart HTTP servers

[Jeff King]

On Wed, Feb 06, 2013 at 07:56:08AM -0800, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  Is it worth having a strbuf_set* family of functions to match the
  strbuf_add*? We seem to have these sorts of errors with strbuf from time
  to time, and I wonder if that would make it easier (and more readable)
  to do the right thing.
 
 Possibly.
 
 The callsite below may be a poor example, though; you would need the
 _reset() even if you change the _addstr() we can see in the context
 to _setstr() to make sure later strbuf_*(type) will start from a
 clean slate when !t anyway, no?

Ah, true. Let's not worry about it, then.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Verify Content-Type from smart HTTP servers

[Jeff King]

On Sun, Feb 03, 2013 at 11:17:33PM -0800, Junio C Hamano wrote:

 Does this look good to both of you (relative to Shawn's patch)?
 
  remote-curl.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)
 
 diff --git a/remote-curl.c b/remote-curl.c
 index e6f3b63..933c69a 100644
 --- a/remote-curl.c
 +++ b/remote-curl.c
 @@ -134,14 +134,14 @@ static struct discovery* discover_refs(const char 
 *service)
   last-buf_alloc = strbuf_detach(buffer, last-len);
   last-buf = last-buf_alloc;
  
 - if (maybe_smart  5 = last-len  last-buf[4] == '#') {
 + strbuf_addf(exp, application/x-%s-advertisement, service);
 + if (maybe_smart 
 + (5 = last-len  last-buf[4] == '#') 
 + !strbuf_cmp(exp, type)) {
   /*
* smart HTTP response; validate that the service
* pkt-line matches our request.
*/
 - strbuf_addf(exp, application/x-%s-advertisement, service);
 - if (strbuf_cmp(exp, type))
 - die(invalid content-type %s, type.buf);
   if (packet_get_line(buffer, last-buf, last-len) = 0)
   die(%s has invalid packet header, refs_url);
   if (buffer.len  buffer.buf[buffer.len - 1] == '\n')

Yeah, I think that's fine. Thanks.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Verify Content-Type from smart HTTP servers

[Shawn Pearce]

On Mon, Feb 4, 2013 at 12:38 AM, Jeff King p...@peff.net wrote:
 On Sun, Feb 03, 2013 at 11:17:33PM -0800, Junio C Hamano wrote:

  Does this look good to both of you (relative to Shawn's patch)?
 
   remote-curl.c | 8 
   1 file changed, 4 insertions(+), 4 deletions(-)
 
  diff --git a/remote-curl.c b/remote-curl.c
  index e6f3b63..933c69a 100644
  --- a/remote-curl.c
  +++ b/remote-curl.c
  @@ -134,14 +134,14 @@ static struct discovery* discover_refs(const char 
  *service)
last-buf_alloc = strbuf_detach(buffer, last-len);
last-buf = last-buf_alloc;
 
  - if (maybe_smart  5 = last-len  last-buf[4] == '#') {
  + strbuf_addf(exp, application/x-%s-advertisement, service);
  + if (maybe_smart 
  + (5 = last-len  last-buf[4] == '#') 
  + !strbuf_cmp(exp, type)) {
/*
 * smart HTTP response; validate that the service
 * pkt-line matches our request.
 */
  - strbuf_addf(exp, application/x-%s-advertisement, service);
  - if (strbuf_cmp(exp, type))
  - die(invalid content-type %s, type.buf);
if (packet_get_line(buffer, last-buf, last-len) = 0)
die(%s has invalid packet header, refs_url);
if (buffer.len  buffer.buf[buffer.len - 1] == '\n')

 Yeah, I think that's fine. Thanks.

Looks fine to me too, but I think the test won't work now. :-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Verify Content-Type from smart HTTP servers

[Junio C Hamano]

Shawn Pearce spea...@spearce.org writes:

 Looks fine to me too, but I think the test won't work now. :-)

Heh, that's amusing ;-)

 t/t5551-http-fetch.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index cb95b95..47eb769 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -158,9 +158,8 @@ test_expect_success 'GIT_SMART_HTTP can disable smart http' 
'
 '
 
 test_expect_success 'invalid Content-Type rejected' '
-   echo fatal: invalid content-type text/html expect
test_must_fail git clone $HTTPD_URL/broken_smart/repo.git 2actual
-   test_cmp expect actual
+   grep not valid: actual
 '
 
 test -n $GIT_TEST_LONG  test_set_prereq EXPENSIVE

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Verify Content-Type from smart HTTP servers

[Junio C Hamano]

Jeff King p...@peff.net writes:

 I was specifically thinking of this (on top of your patch):

 diff --git a/remote-curl.c b/remote-curl.c
 index e6f3b63..63680a8 100644
 --- a/remote-curl.c
 +++ b/remote-curl.c
 @@ -134,14 +134,12 @@ static struct discovery* discover_refs(const char 
 *service)
   last-buf_alloc = strbuf_detach(buffer, last-len);
   last-buf = last-buf_alloc;
  
 - if (maybe_smart  5 = last-len  last-buf[4] == '#') {
 + strbuf_addf(exp, application/x-%s-advertisement, service);
 + if (maybe_smart  !strbuf_cmp(exp, type)) {
   /*
* smart HTTP response; validate that the service
* pkt-line matches our request.
*/
 - strbuf_addf(exp, application/x-%s-advertisement, service);
 - if (strbuf_cmp(exp, type))
 - die(invalid content-type %s, type.buf);
   if (packet_get_line(buffer, last-buf, last-len) = 0)
   die(%s has invalid packet header, refs_url);
   if (buffer.len  buffer.buf[buffer.len - 1] == '\n')

 To just follow the dumb path if we don't get the content-type we expect.
 We may want to keep the '#' format check in addition (packet_get_line
 will check it and die, anyway, but we may want to drop back to
 considering it dumb, just to protect against a badly configured dumb
 server which uses our mime type, but I do not think it likely).

Yeah, but it doesn't cost anything to check, so let's do so.

Does this look good to both of you (relative to Shawn's patch)?

 remote-curl.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index e6f3b63..933c69a 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -134,14 +134,14 @@ static struct discovery* discover_refs(const char 
*service)
last-buf_alloc = strbuf_detach(buffer, last-len);
last-buf = last-buf_alloc;
 
-   if (maybe_smart  5 = last-len  last-buf[4] == '#') {
+   strbuf_addf(exp, application/x-%s-advertisement, service);
+   if (maybe_smart 
+   (5 = last-len  last-buf[4] == '#') 
+   !strbuf_cmp(exp, type)) {
/*
 * smart HTTP response; validate that the service
 * pkt-line matches our request.
 */
-   strbuf_addf(exp, application/x-%s-advertisement, service);
-   if (strbuf_cmp(exp, type))
-   die(invalid content-type %s, type.buf);
if (packet_get_line(buffer, last-buf, last-len) = 0)
die(%s has invalid packet header, refs_url);
if (buffer.len  buffer.buf[buffer.len - 1] == '\n')
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Verify Content-Type from smart HTTP servers

[Jeff King]

On Thu, Jan 31, 2013 at 02:09:40PM -0800, Junio C Hamano wrote:

 Before parsing a suspected smart-HTTP response verify the returned
 Content-Type matches the standard. This protects a client from
 attempting to process a payload that smells like a smart-HTTP
 server response.
 
 JGit has been doing this check on all responses since the dawn of
 time. I mistakenly failed to include it in git-core when smart HTTP
 was introduced. At the time I didn't know how to get the Content-Type
 from libcurl. I punted, meant to circle back and fix this, and just
 plain forgot about it.
 
 Signed-off-by: Shawn Pearce spea...@spearce.org
 Signed-off-by: Junio C Hamano gits...@pobox.com

Should this be From: Shawn? The tone of the message and the S-O-B
order makes it look like it.

 @@ -133,16 +135,19 @@ static struct discovery* discover_refs(const char 
 *service)
   last-buf = last-buf_alloc;
  
   if (maybe_smart  5 = last-len  last-buf[4] == '#') {
 - /* smart HTTP response; validate that the service
 + /*
 +  * smart HTTP response; validate that the service
* pkt-line matches our request.
*/
 - struct strbuf exp = STRBUF_INIT;
 -
 + strbuf_addf(exp, application/x-%s-advertisement, service);
 + if (strbuf_cmp(exp, type))
 + die(invalid content-type %s, type.buf);

Hmm. I wondered if it is possible for a non-smart server to send us down
this code path, which would now complain of the bogus content-type.
Something like an info/refs file with:

  # 1
  # the comment above is meaningless, but puts a # at position 4.

But I note that we would already die in the next line:

   if (packet_get_line(buffer, last-buf, last-len) = 0)
   die(%s has invalid packet header, refs_url);

so I do not think the patch makes anything worse. However, should we
take this opportunity to make the did we get a smart response test
more robust? That is, should we actually be checking the content-type
in the outer conditional, and going down the smart code-path if it is
application/x-%s-advertisement, and otherwise treating the result as
dumb?

It's probably not a big deal, as the false positive example above is
quite specific and unlikely, but it just seems cleaner to me.

As a side note, should we (can we) care about the content-type for dumb
http? It should probably be text/plain or application/octet-stream, but
I would not be surprised if we get a variety of random junk in the real
world, though.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Verify Content-Type from smart HTTP servers

[Junio C Hamano]

Jeff King p...@peff.net writes:

 Should this be From: Shawn? The tone of the message and the S-O-B
 order makes it look like it.

Yes. I should have left that line when edited the format-patch
output in my MUA to say I was resending something that vger rejected
and people did not see after tweaking the patch to slip their taboo
list.

 @@ -133,16 +135,19 @@ static struct discovery* discover_refs(const char 
 *service)
  last-buf = last-buf_alloc;
  
  if (maybe_smart  5 = last-len  last-buf[4] == '#') {
 -/* smart HTTP response; validate that the service
 +/*
 + * smart HTTP response; validate that the service
   * pkt-line matches our request.
   */
 -struct strbuf exp = STRBUF_INIT;
 -
 +strbuf_addf(exp, application/x-%s-advertisement, service);
 +if (strbuf_cmp(exp, type))
 +die(invalid content-type %s, type.buf);

 Hmm. I wondered if it is possible for a non-smart server to send us down
 this code path, which would now complain of the bogus content-type.
 Something like an info/refs file with:

   # 1
   # the comment above is meaningless, but puts a # at position 4.

 But I note that we would already die in the next line:

  if (packet_get_line(buffer, last-buf, last-len) = 0)
  die(%s has invalid packet header, refs_url);

 so I do not think the patch makes anything worse. However, should we
 take this opportunity to make the did we get a smart response test
 more robust? That is, should we actually be checking the content-type
 in the outer conditional, and going down the smart code-path if it is
 application/x-%s-advertisement, and otherwise treating the result as
 dumb?

Does the outer caller that switches between dumb and smart actually
know what service type it is requesting (I am not familiar with the
callchain involved)?  Even if it doesn't, it may still make sense.

 As a side note, should we (can we) care about the content-type for dumb
 http? It should probably be text/plain or application/octet-stream, but
 I would not be surprised if we get a variety of random junk in the real
 world, though.

The design objective of dumb http protocol was to allow working with
any dumb bit transfer thing, so I'd prefer to keep it lenient and
allow application/x-git-loose-object-file and somesuch.

Thanks.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Verify Content-Type from smart HTTP servers

[Jeff King]

On Fri, Feb 01, 2013 at 10:09:26AM -0800, Junio C Hamano wrote:

  so I do not think the patch makes anything worse. However, should we
  take this opportunity to make the did we get a smart response test
  more robust? That is, should we actually be checking the content-type
  in the outer conditional, and going down the smart code-path if it is
  application/x-%s-advertisement, and otherwise treating the result as
  dumb?
 
 Does the outer caller that switches between dumb and smart actually
 know what service type it is requesting (I am not familiar with the
 callchain involved)?  Even if it doesn't, it may still make sense.

I was specifically thinking of this (on top of your patch):

diff --git a/remote-curl.c b/remote-curl.c
index e6f3b63..63680a8 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -134,14 +134,12 @@ static struct discovery* discover_refs(const char 
*service)
last-buf_alloc = strbuf_detach(buffer, last-len);
last-buf = last-buf_alloc;
 
-   if (maybe_smart  5 = last-len  last-buf[4] == '#') {
+   strbuf_addf(exp, application/x-%s-advertisement, service);
+   if (maybe_smart  !strbuf_cmp(exp, type)) {
/*
 * smart HTTP response; validate that the service
 * pkt-line matches our request.
 */
-   strbuf_addf(exp, application/x-%s-advertisement, service);
-   if (strbuf_cmp(exp, type))
-   die(invalid content-type %s, type.buf);
if (packet_get_line(buffer, last-buf, last-len) = 0)
die(%s has invalid packet header, refs_url);
if (buffer.len  buffer.buf[buffer.len - 1] == '\n')

To just follow the dumb path if we don't get the content-type we expect.
We may want to keep the '#' format check in addition (packet_get_line
will check it and die, anyway, but we may want to drop back to
considering it dumb, just to protect against a badly configured dumb
server which uses our mime type, but I do not think it likely).

  As a side note, should we (can we) care about the content-type for dumb
  http? It should probably be text/plain or application/octet-stream, but
  I would not be surprised if we get a variety of random junk in the real
  world, though.
 
 The design objective of dumb http protocol was to allow working with
 any dumb bit transfer thing, so I'd prefer to keep it lenient and
 allow application/x-git-loose-object-file and somesuch.

Yeah, I do not think it really buys us anything in practice, and we have
no way of knowing what kind of crap is in the wild. Not worth it.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Verify Content-Type from smart HTTP servers

[Junio C Hamano]

Before parsing a suspected smart-HTTP response verify the returned
Content-Type matches the standard. This protects a client from
attempting to process a payload that smells like a smart-HTTP
server response.

JGit has been doing this check on all responses since the dawn of
time. I mistakenly failed to include it in git-core when smart HTTP
was introduced. At the time I didn't know how to get the Content-Type
from libcurl. I punted, meant to circle back and fix this, and just
plain forgot about it.

Signed-off-by: Shawn Pearce spea...@spearce.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 * The original was picked up by majordomo taboo rules; resending
   after minor style fix.

   Was there a report of an attempted attack by malicious server or
   something that triggered this, or is this just a common sense
   thing to do in general?

 http-push.c  |  4 ++--
 http.c   | 31 ++-
 http.h   |  2 +-
 remote-curl.c| 15 +++
 t/lib-httpd.sh   |  1 +
 t/lib-httpd/apache.conf  |  4 
 t/lib-httpd/broken-smart-http.sh | 11 +++
 t/t5551-http-fetch.sh|  6 ++
 8 files changed, 58 insertions(+), 16 deletions(-)
 create mode 100755 t/lib-httpd/broken-smart-http.sh

diff --git a/http-push.c b/http-push.c
index 8701c12..ba45b7b 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1560,7 +1560,7 @@ static int remote_exists(const char *path)
 
sprintf(url, %s%s, repo-url, path);
 
-   switch (http_get_strbuf(url, NULL, 0)) {
+   switch (http_get_strbuf(url, NULL, NULL, 0)) {
case HTTP_OK:
ret = 1;
break;
@@ -1584,7 +1584,7 @@ static void fetch_symref(const char *path, char **symref, 
unsigned char *sha1)
url = xmalloc(strlen(repo-url) + strlen(path) + 1);
sprintf(url, %s%s, repo-url, path);
 
-   if (http_get_strbuf(url, buffer, 0) != HTTP_OK)
+   if (http_get_strbuf(url, NULL, buffer, 0) != HTTP_OK)
die(Couldn't get %s for remote symref\n%s, url,
curl_errorstr);
free(url);
diff --git a/http.c b/http.c
index 44f3525..d868d8b 100644
--- a/http.c
+++ b/http.c
@@ -788,7 +788,8 @@ int handle_curl_result(struct slot_results *results)
 #define HTTP_REQUEST_STRBUF0
 #define HTTP_REQUEST_FILE  1
 
-static int http_request(const char *url, void *result, int target, int options)
+static int http_request(const char *url, struct strbuf *type,
+   void *result, int target, int options)
 {
struct active_request_slot *slot;
struct slot_results results;
@@ -838,24 +839,36 @@ static int http_request(const char *url, void *result, 
int target, int options)
ret = HTTP_START_FAILED;
}
 
+   if (type) {
+   char *t;
+   curl_easy_getinfo(slot-curl, CURLINFO_CONTENT_TYPE, t);
+   if (t)
+   strbuf_addstr(type, t);
+   }
+
curl_slist_free_all(headers);
strbuf_release(buf);
 
return ret;
 }
 
-static int http_request_reauth(const char *url, void *result, int target,
+static int http_request_reauth(const char *url,
+  struct strbuf *type,
+  void *result, int target,
   int options)
 {
-   int ret = http_request(url, result, target, options);
+   int ret = http_request(url, type, result, target, options);
if (ret != HTTP_REAUTH)
return ret;
-   return http_request(url, result, target, options);
+   return http_request(url, type, result, target, options);
 }
 
-int http_get_strbuf(const char *url, struct strbuf *result, int options)
+int http_get_strbuf(const char *url,
+   struct strbuf *type,
+   struct strbuf *result, int options)
 {
-   return http_request_reauth(url, result, HTTP_REQUEST_STRBUF, options);
+   return http_request_reauth(url, type, result,
+  HTTP_REQUEST_STRBUF, options);
 }
 
 /*
@@ -878,7 +891,7 @@ static int http_get_file(const char *url, const char 
*filename, int options)
goto cleanup;
}
 
-   ret = http_request_reauth(url, result, HTTP_REQUEST_FILE, options);
+   ret = http_request_reauth(url, NULL, result, HTTP_REQUEST_FILE, 
options);
fclose(result);
 
if ((ret == HTTP_OK)  move_temp_to_file(tmpfile.buf, filename))
@@ -904,7 +917,7 @@ int http_fetch_ref(const char *base, struct ref *ref)
int ret = -1;
 
url = quote_ref_url(base, ref-name);
-   if (http_get_strbuf(url, buffer, HTTP_NO_CACHE) == HTTP_OK) {
+   if (http_get_strbuf(url, NULL, buffer, HTTP_NO_CACHE) == HTTP_OK) {
strbuf_rtrim(buffer);
if (buffer.len == 40)
ret = get_sha1_hex(buffer.buf

Re: [PATCH] Verify Content-Type from smart HTTP servers

[Shawn Pearce]

On Thu, Jan 31, 2013 at 1:59 PM, Junio C Hamano gits...@pobox.com wrote:
 Shawn Pearce spea...@spearce.org writes:

 Before parsing a suspected smart-HTTP response verify the returned
 Content-Type matches the standard. This protects a client from
 attempting to process a payload that smells like a smart-HTTP
 server response.

 JGit has been doing this check on all responses since the dawn of
 time. I mistakenly failed to include it in git-core when smart HTTP
 was introduced. At the time I didn't know how to get the Content-Type
 from libcurl. I punted, meant to circle back and fix this, and just
 plain forgot about it.

 Signed-off-by: Shawn Pearce spea...@spearce.org
 ---

 Sounds sensible.  Was there a report of attack attempts by malicious
 servers or something, or is it just a general common sense thing?

Common-sense cleanup.

I had a report a while ago about JGit not working with the Git servers
at Codeplex. This failure was caused by their HTTP servers returning
an invalid Content-Type, making JGit refuse to continue parsing. This
has since been fixed, I verified this morning that Codeplex is
returning the correct Content-Type.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-archive fails against smart-http repos

[Jeff King]

On Wed, Jan 09, 2013 at 10:52:48AM -0800, Bruce Lysik wrote:

 Trying to run git-archive fails against smart-http based repos.  Example:
 
 $ git archive --verbose --format=zip 
 --remote=http://code.toofishes.net/git/dan/initscripts.git
 fatal: Operation not supported by protocol.
 Unexpected end of command stream
 
 This problem was brought up against my internal repos as well.

Right. Neither the client nor server for the http transport knows how to
handle the git-upload-archive service (as opposed to the regular
git-upload-pack or git-receive-pack services). I don't think there's
anything technical standing in the way; it is has simply never been
implemented.

Currently, you can do remote git-archive only locally, via ssh, or over
git:// (but then only if the server side has explicitly enabled it).

Patches welcome.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

git-archive fails against smart-http repos

[Bruce Lysik]

Hi,

Trying to run git-archive fails against smart-http based repos.  Example:

$ git archive --verbose --format=zip 
--remote=http://code.toofishes.net/git/dan/initscripts.git
fatal: Operation not supported by protocol.
Unexpected end of command stream

This problem was brought up against my internal repos as well.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git smart-http do not authent to allow git ls-remote to be called anonymously

[Jeff King]

[+cc git@vger; please keep discussion on the list]

On Sun, Oct 28, 2012 at 01:26:51PM +0800, 乙酸鋰 wrote:

  POST /git/Cat1/SubCat1/xsp.git/git-upload-pack HTTP/1.1
 User-Agent: git/1.8.0
 Host: localhost
 Accept-Encoding: gzip
 Content-Type: application/x-git-upload-pack-request
 Accept: application/x-git-upload-pack-result
 Content-Length: 190
 
 * The requested URL returned error: 401
 * Closing connection #0
 Username for 'http://localhost': user
 Password for 'http://user@localhost':
 fatal: The remote end hung up unexpectedly

OK, I see what is going on. The code in b81401c to retry POST requests
does not handle gzipped contents, and upload-pack tends to gzip what it
sends.

Your apache configuration is not really something that we ever intended
to support, and I am a little dubious of the security tradeoff being
made. But it is actually pretty easy for us to support, and it
eliminates a special case from the code, so I am tempted to do so.

The following patch series (on top of the current master, as they
require some cleanup that did not make it into 1.8.0) seems to fix it
for me.

  [1/2]: remote-curl: hoist gzip buffer size to top of post_rpc
  [2/2]: remote-curl: retry failed requests for auth even with gzip

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git smart-http do not authent to allow git ls-remote to be called anonymously

[Jeff King]

[re-adding git@vger; please keep discussion on-list]

On Sun, Oct 14, 2012 at 01:29:13PM +0800, 乙酸鋰 wrote:

 Sorry, it does not serve the request. It returns http 401.
 But if I add the username and password as a part of the URL, it succeeds.

In that case, then you probably need to upgrade your client version of
git, as I mentioned here:

  Or is it that the server tells git that it needs authorization, but git
  does not prompt, and instead just fails with Authentication failed. In
  that case, the issue is that you need a newer git client. Traditionally
  the client expected to handle authentication during the initial
  info/refs request. I added support for handling authentication during
  later requests in commit b81401c, which is in git v1.7.11.7 and
  v1.7.12.1.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

git smart-http do not authent to allow git ls-remote to be called anonymously

[乙酸鋰]

Hi,

I use smart-http on Apache.
If nothing to be pushed / pulled, I do not want password to be
supplied. And allow git ls-remote to run without password

*.git/info/refs?service=git-upload-pack
*.git/info/refs?service=git-receive-pack

I only need authentication on

*.git/git-upload-pack
*.git/git-receive-pack

/etc/apache/httpd.conf

LocationMatch ^/git/.*/git-(upload|receive)-pack$
AuthType Basic
AuthName staff only
AuthUserFile /etc/apache/apache.pwd
Require valid-user
/LocationMatch

However this does not work. It does not ask for password at all.

I use Ubuntu 10.04, Apache 2.2.14, Git 1.7.11.3.

Directory structure: any depth (more than 1 subdir) of path from /git
to .git folders

Could you advise how to configure this? Is this a bug?

Regards,
ch3cooli
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git smart-http do not authent to allow git ls-remote to be called anonymously

[Shawn Pearce]

On Sun, Sep 30, 2012 at 7:35 AM, 乙酸鋰 ch3co...@gmail.com wrote:
 I use smart-http on Apache.
 If nothing to be pushed / pulled, I do not want password to be
 supplied. And allow git ls-remote to run without password

 *.git/info/refs?service=git-upload-pack
 *.git/info/refs?service=git-receive-pack

 I only need authentication on

 *.git/git-upload-pack
 *.git/git-receive-pack

 /etc/apache/httpd.conf

 LocationMatch ^/git/.*/git-(upload|receive)-pack$
 AuthType Basic
 AuthName staff only
 AuthUserFile /etc/apache/apache.pwd
 Require valid-user
 /LocationMatch

 However this does not work. It does not ask for password at all.

This sounds like a bug in your Apache configuration. I would verify it
prompts for a password as expected before worrying about the Git
client:

  curl -v http://localhost/git/blah/git-upload-pack

should fail with a 401 requesting access to staff only. Once this is
working, git will present authorization as necessary during the
/git-upload-pack|git-receive-pack calls.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git smart-http do not authent to allow git ls-remote to be called anonymously

[Jeff King]

On Sun, Sep 30, 2012 at 10:35:35PM +0800, 乙酸鋰 wrote:

 I use smart-http on Apache.
 If nothing to be pushed / pulled, I do not want password to be
 supplied. And allow git ls-remote to run without password
 
 *.git/info/refs?service=git-upload-pack
 *.git/info/refs?service=git-receive-pack
 
 I only need authentication on
 
 *.git/git-upload-pack
 *.git/git-receive-pack
 
 /etc/apache/httpd.conf
 
 LocationMatch ^/git/.*/git-(upload|receive)-pack$
 AuthType Basic
 AuthName staff only
 AuthUserFile /etc/apache/apache.pwd
 Require valid-user
 /LocationMatch
 
 However this does not work. It does not ask for password at all.

What is it in the final sentence? Does the server not tell the git
client that authorization is required, and actually serve the request?
If so, then that is a bug in your apache config.

Or is it that the server tells git that it needs authorization, but git
does not prompt, and instead just fails with Authentication failed. In
that case, the issue is that you need a newer git client. Traditionally
the client expected to handle authentication during the initial
info/refs request. I added support for handling authentication during
later requests in commit b81401c, which is in git v1.7.11.7 and
v1.7.12.1.

You should reconsider whether this is what you really want, though. With
the configuration you showed, anyone will be able to get a list of all
refs and their sha1s. So they would know all your branch names, and they
could even potentially find out what's in your branches by making
offline guesses and comparing them to your branch sha1s (the feasibility
of this would depend on exactly what you're storing).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] remote-curl: let users turn off smart http

[Junio C Hamano]

Jeff King p...@peff.net writes:

 On Thu, Sep 20, 2012 at 02:15:20PM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  I'm half-tempted to just drop the config entirely, leave
  GIT_SMART_HTTP=false as an escape hatch, and see if anybody even cares.
 
 Sounds like a very attractive minimalistic way to go forward.  We
 can always add per-remote configuration when we find it necessary,
 but once we add support, we cannot easily yank it out.

 Like this?

Yeah.  Will queue this one instead.  The simpler, the better ;-)


 -- 8 --
 Subject: [PATCH] remote-curl: let users turn off smart http

 Usually there is no need for users to specify whether an
 http remote is smart or dumb; the protocol is designed so
 that a single initial request is made, and the client can
 determine the server's capability from the response.

 However, some misconfigured dumb-only servers may not like
 the initial request by a smart client, as it contains a
 query string. Until recently, commit 703e6e7 worked around
 this by making a second request. However, that commit was
 recently reverted due to its side effect of masking the
 initial request's error code.

 Since git has had that workaround for several years, we
 don't know exactly how many such misconfigured servers are
 out there. The reversion of 703e6e7 assumes they are rare
 enough not to worry about. Still, that reversion leaves
 somebody who does run into such a server with no escape
 hatch at all. Let's give them an environment variable they
 can tweak to perform the dumb request.

 This is intentionally not a documented interface. It's
 overly simple and is really there for debugging in case
 somebody does complain about git not working with their
 server. A real user-facing interface would entail a
 per-remote or per-URL config variable.

 Signed-off-by: Jeff King p...@peff.net
 ---
  remote-curl.c |  3 ++-
  t/t5551-http-fetch.sh | 12 
  2 files changed, 14 insertions(+), 1 deletion(-)

 diff --git a/remote-curl.c b/remote-curl.c
 index c0b98cc..7b19ebb 100644
 --- a/remote-curl.c
 +++ b/remote-curl.c
 @@ -102,7 +102,8 @@ static struct discovery* discover_refs(const char 
 *service)
   free_discovery(last);
  
   strbuf_addf(buffer, %sinfo/refs, url);
 - if (!prefixcmp(url, http://;) || !prefixcmp(url, https://;)) {
 + if ((!prefixcmp(url, http://;) || !prefixcmp(url, https://;)) 
 +  git_env_bool(GIT_SMART_HTTP, 1)) {
   maybe_smart = 1;
   if (!strchr(url, '?'))
   strbuf_addch(buffer, '?');
 diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
 index 2db5c35..8427943 100755
 --- a/t/t5551-http-fetch.sh
 +++ b/t/t5551-http-fetch.sh
 @@ -129,6 +129,18 @@ test_expect_success 'clone from auth-only-for-push 
 repository' '
   test_cmp expect actual
  '
  
 +test_expect_success 'disable dumb http on server' '
 + git --git-dir=$HTTPD_DOCUMENT_ROOT_PATH/repo.git \
 + config http.getanyfile false
 +'
 +
 +test_expect_success 'GIT_SMART_HTTP can disable smart http' '
 + (GIT_SMART_HTTP=0 
 +  export GIT_SMART_HTTP 
 +  cd clone 
 +  test_must_fail git fetch)
 +'
 +
  test -n $GIT_TEST_LONG  test_set_prereq EXPENSIVE
  
  test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] remote-curl: let users turn off smart http

[Jeff King]

On Fri, Sep 21, 2012 at 10:34:22AM -0700, Junio C Hamano wrote:

   I'm half-tempted to just drop the config entirely, leave
   GIT_SMART_HTTP=false as an escape hatch, and see if anybody even cares.
  
  Sounds like a very attractive minimalistic way to go forward.  We
  can always add per-remote configuration when we find it necessary,
  but once we add support, we cannot easily yank it out.
 
  Like this?
 
 Yeah.  Will queue this one instead.  The simpler, the better ;-)

Thanks. I almost followed up with a rebased version of my config patch,
should we want to apply it later separately. But I think I would really
rather gather data from even a single bug report before we move any
further (and with any luck, there will be zero bug reports :) ).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2] smart http toggle switch fails

[Jeff King]

On Thu, Sep 20, 2012 at 12:24:56PM -0400, Jeff King wrote:

 I think Shawn's revert is the right thing to do. But it is not complete
 without the manual workaround. I'm putting that patch together now and
 should have it out in a few minutes.

And here it is. This goes on top of Shawn's revert patch (it might be
nice to mention the commit id of that in the commit message of the
second patch. I couldn't do so because it is not yet in your repo).

  [1/2]: remote-curl: rename is_http variable
  [2/2]: remote-curl: let users turn off smart http

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] remote-curl: let users turn off smart http

[Jeff King]

Usually there is no need for users to specify whether an
http remote is smart or dumb; the protocol is designed so
that a single initial request is made, and the client can
determine the server's capability from the response.

However, some misconfigured dumb-only servers may not like
the initial request by a smart client, as it contains a
query string. Until recently, commit 703e6e7 worked around
this by making a second request. However, that commit was
recently reverted due to its side effect of masking the
initial request's error code.

This patch takes a different approach to the workaround. We
assume that the common case is that the server is either
smart-http or a reasonably configured dumb-http. If that is
not the case, we provide both a per-remote config option and
an environment variable with which the user can manually
work around the issue.

Signed-off-by: Jeff King p...@peff.net
---
I added the config item as remote.foo.smarthttp. You could also allow
http.$url.smart (and just http.smart, for that matter), which could
be more flexible if you have multiple remotes pointing to the same
broken server. However, it is also more complex to use, and is a lot
more code. Since we don't know if any such servers even exist, I tried
to give the minimal escape hatch, and we can easily build more features
on it later if people complain.

 Documentation/config.txt | 11 +++
 remote-curl.c|  3 ++-
 remote.c |  3 +++
 remote.h |  1 +
 t/t5551-http-fetch.sh| 17 +
 5 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6416cae..651b23c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1871,6 +1871,17 @@ remote.name.uploadpack::
The default program to execute on the remote side when fetching.  See
option \--upload-pack of linkgit:git-fetch-pack[1].
 
+remote.name.smartHttp::
+   If true, this remote will attempt to use git's smart http
+   protocol when making remote http requests. Normally git sends an
+   initial smart-http request, and falls back to the older dumb
+   protocol if the server does not claim to support the smart
+   protocol. However, some misconfigured dumb-only servers may
+   produce confusing results for the initial request. Setting this
+   option to false disables the initial smart request, which can
+   workaround problems with such servers. You should not generally
+   need to set this. Defaults to `true`.
+
 remote.name.tagopt::
Setting this value to \--no-tags disables automatic tag following when
fetching from remote name. Setting it to \--tags will fetch every
diff --git a/remote-curl.c b/remote-curl.c
index c0b98cc..8829bfb 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -102,7 +102,8 @@ static struct discovery* discover_refs(const char *service)
free_discovery(last);
 
strbuf_addf(buffer, %sinfo/refs, url);
-   if (!prefixcmp(url, http://;) || !prefixcmp(url, https://;)) {
+   if ((!prefixcmp(url, http://;) || !prefixcmp(url, https://;)) 
+git_env_bool(GIT_SMART_HTTP, remote-smart_http)) {
maybe_smart = 1;
if (!strchr(url, '?'))
strbuf_addch(buffer, '?');
diff --git a/remote.c b/remote.c
index 04fd9ea..a334390 100644
--- a/remote.c
+++ b/remote.c
@@ -152,6 +152,7 @@ static struct remote *make_remote(const char *name, int len)
ret-name = xstrndup(name, len);
else
ret-name = xstrdup(name);
+   ret-smart_http = 1;
return ret;
 }
 
@@ -453,6 +454,8 @@ static int handle_config(const char *key, const char 
*value, void *cb)
 key, value);
} else if (!strcmp(subkey, .vcs)) {
return git_config_string(remote-foreign_vcs, key, value);
+   } else if (!strcmp(subkey, .smarthttp)) {
+   remote-smart_http = git_config_bool(key, value);
}
return 0;
 }
diff --git a/remote.h b/remote.h
index 251d8fd..9031d18 100644
--- a/remote.h
+++ b/remote.h
@@ -40,6 +40,7 @@ struct remote {
int fetch_tags;
int skip_default_update;
int mirror;
+   int smart_http;
 
const char *receivepack;
const char *uploadpack;
diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index 2db5c35..48173ed 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -129,6 +129,23 @@ test_expect_success 'clone from auth-only-for-push 
repository' '
test_cmp expect actual
 '
 
+test_expect_success 'disable dumb http on server' '
+   git --git-dir=$HTTPD_DOCUMENT_ROOT_PATH/repo.git \
+   config http.getanyfile false
+'
+
+test_expect_success 'GIT_SMART_HTTP can disable smart http' '
+   (GIT_SMART_HTTP=0 
+export GIT_SMART_HTTP 
+cd clone 
+test_must_fail git fetch

Re: [PATCH 2/2] remote-curl: let users turn off smart http

[Jeff King]

On Thu, Sep 20, 2012 at 10:53:15AM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  I added the config item as remote.foo.smarthttp. You could also allow
  http.$url.smart (and just http.smart, for that matter), which could
  be more flexible if you have multiple remotes pointing to the same
  broken server.
 
 What would the user experience be when we introduce even smarter
 http server protocol extension?  Will we add remote.foo.starterhttp?

I would hope that it would actually be negotiated reliably at the
protocol level so we do not have to deal with this mess again.

 Perhaps
 
 remote.$name.httpvariants = [smart] [dumb]
 
 to allow users to say smart only, dumb only, or smart and/or
 dumb might be more code but less burden on the users.

I don't mind that format if we are going that direction, but is there
anybody who actually wants to say smart only?

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] remote-curl: let users turn off smart http

[Junio C Hamano]

Jeff King p...@peff.net writes:

 On Thu, Sep 20, 2012 at 10:53:15AM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  I added the config item as remote.foo.smarthttp. You could also allow
  http.$url.smart (and just http.smart, for that matter), which could
  be more flexible if you have multiple remotes pointing to the same
  broken server.
 
 What would the user experience be when we introduce even smarter
 http server protocol extension?  Will we add remote.foo.starterhttp?

 I would hope that it would actually be negotiated reliably at the
 protocol level so we do not have to deal with this mess again.

The original dumb vs smart was supposed to be negotiated reliably
at the protocol level, no?  Yet we need this band-aid, so...

 Perhaps
 
 remote.$name.httpvariants = [smart] [dumb]
 
 to allow users to say smart only, dumb only, or smart and/or
 dumb might be more code but less burden on the users.

 I don't mind that format if we are going that direction, but is there
 anybody who actually wants to say smart only?

With 703e6e7 reverted, we take a failure from the initial smart
request to mean the server is simply not serving, so smart only to
fail quickly without trying dumb fallback is not needed.  smart
only to say I wouldn't want to talk to dumb-only server---I do not
have infinite amount of time, and I'd rather try another server is
still a possibility, but likely not worth supporting.



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html