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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]