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

Jeff King [off-list ref] 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.

quoted

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

quoted

 		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.