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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@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')