Returning error message from custom smart http server

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

Re: Returning error message from custom smart http server

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2018 bytes --]

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 <sandals@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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Returning error message from custom smart http server

[Bryan Turner]

On Sat, May 17, 2014 at 9:01 AM, brian m. carlson
<sandals@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 "FFFF" 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 "0000" (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-pack0000ERR 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 <sandals@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

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
> <sandals@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 "FFFF" 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 "0000" (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-pack0000ERR 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

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