Jeff King <peff@xxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html