Re: [PATCH] Verify Content-Type from smart HTTP servers
- From
Jeff King <peff@peff.net>
- Date
- Feb 6, 2013, 10:39 UTC
- Message-ID
- <20130206103952.GA5267@sigill.intra.peff.net>
- In-Reply-To
- <51122F69.9060704@elegosoft.com>
On Wed, Feb 06, 2013 at 11:24:41AM +0100, Michael Schubert wrote:
Show 37 quoted lines
> On 01/31/2013 11:09 PM, Junio C Hamano wrote:
>
> >
> > -static int http_request_reauth(const char *url, void *result, int target,
> > +static int http_request_reauth(const char *url,
> > + struct strbuf *type,
> > + void *result, int target,
> > int options)
> > {
> > - int ret = http_request(url, result, target, options);
> > + int ret = http_request(url, type, result, target, options);
> > if (ret != HTTP_REAUTH)
> > return ret;
> > - return http_request(url, result, target, options);
> > + return http_request(url, type, result, target, options);
> > }
>
> This needs something like
>
> diff --git a/http.c b/http.c
> index d868d8b..da43be3 100644
> --- a/http.c
> +++ b/http.c
> @@ -860,6 +860,8 @@ static int http_request_reauth(const char *url,
> int ret = http_request(url, type, result, target, options);
> if (ret != HTTP_REAUTH)
> return ret;
> + if (type)
> + strbuf_reset(type);
> return http_request(url, type, result, target, options);
> }
>
> on top. Otherwise we get
>
> "text/plainapplication/x-git-receive-pack-advertisement"
>
> when doing HTTP auth.Good catch. It probably makes sense to put it in http_request, so that we also protect against any existing cruft from the callers of http_get_*, like:
-- >8 -- Subject: [PATCH] http_request: reset "type" strbuf before adding
Callers may pass us a strbuf which we use to record the content-type of the response. However, we simply appended to it rather than overwriting its contents, meaning that cruft in the strbuf gave us a bogus type. E.g., the multiple requests triggered by http_request could yield a type like "text/plainapplication/x-git-receive-pack-advertisement".
Reported-by: Michael Schubert <mschub@elegosoft.com> Signed-off-by: Jeff King <peff@peff.net> --- Is it worth having a strbuf_set* family of functions to match the strbuf_add*? We seem to have these sorts of errors with strbuf from time to time, and I wonder if that would make it easier (and more readable) to do the right thing.
http.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/http.c b/http.c index d868d8b..d9d1aad 100644 --- a/http.c +++ b/http.c @@ -841,6 +841,7 @@ static int http_request(const char *url, struct strbuf *type, if (type) { char *t; + strbuf_reset(type); curl_easy_getinfo(slot->curl, CURLINFO_CONTENT_TYPE, &t); if (t) strbuf_addstr(type, t);
-- 1.8.1.2.11.g1a2f572