# [RFC/PATCH git] http: avoid empty error messages for some curl errors 6 messages from 2011-09-05 to 2011-09-06. Participants: Jonathan Nieder, Daniel Stenberg, Junio C Hamano. Thread: https://gitlist.dev/t/28310 ## Jonathan Nieder, 2011-09-05 22:22 Subject: [RFC/PATCH git] http: avoid empty error messages for some curl errors Message-ID: <20110905222202.GA32071@elie> URL: https://gitlist.dev/e/20110905222202.GA32071%40elie ``` When asked to fetch over SSL without a valid /etc/ssl/certs/ca-certificates.crt file, "git fetch" writes error: while accessing https://github.com/torvalds/linux.git/info/refs which is a little disconcerting. Better to fall back to curl_easy_strerror(result) when the error string is empty, like the curl utility does: error: Problem with the SSL CA cert (path? access rights?) while accessing https://github.com/torvalds/linux.git/info/refs Signed-off-by: Jonathan Nieder --- Hi, I ran into this error today because this machine has no certs enabled. I'm not thrilled with the following patch because the error string buffer is not cleared very often so it seems possible for it to be not empty but stale at the relevant moment. I would be happier if we could rely on libcurl always filling the error buffer on errors. What do you think? Jonathan http.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/http.c b/http.c index a59cac45..dec3c60a 100644 --- a/http.c +++ b/http.c @@ -851,8 +851,13 @@ static int http_request(const char *url, void *result, int target, int options) init_curl_http_auth(slot->curl); ret = HTTP_REAUTH; } - } else + } else { + if (!curl_errorstr[0]) + strlcpy(curl_errorstr, + curl_easy_strerror(results.curl_result), + sizeof(curl_errorstr)); ret = HTTP_ERROR; + } } else { error("Unable to start HTTP request for %s", url); ret = HTTP_START_FAILED; -- 1.7.6 ``` ## Jonathan Nieder, 2011-09-05 22:29 Subject: [PATCH] http: remove extra newline in error message Message-ID: <20110905222934.GA20084@elie> URL: https://gitlist.dev/e/20110905222934.GA20084%40elie In-Reply-To: <20110905222202.GA32071@elie> ``` There is no need for a blank line between the detailed error message and the later "fatal: HTTP request failed" notice. Keep the newline written by error() itself and eliminate the extra one. Signed-off-by: Jonathan Nieder --- > I ran into this error today Here's a patch for a simpler buglet noticed at the same time. http.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/http.c b/http.c index dec3c60a..fb3465f5 100644 --- a/http.c +++ b/http.c @@ -913,7 +913,7 @@ int http_error(const char *url, int ret) { /* http_request has already handled HTTP_START_FAILED. */ if (ret != HTTP_START_FAILED) - error("%s while accessing %s\n", curl_errorstr, url); + error("%s while accessing %s", curl_errorstr, url); return ret; } -- 1.7.6 ``` ## Daniel Stenberg, 2011-09-06 12:20 Subject: Re: [RFC/PATCH git] http: avoid empty error messages for some curl errors Message-ID: URL: https://gitlist.dev/e/alpine.DEB.2.00.1109061023010.3841%40tvnag.unkk.fr In-Reply-To: <20110905222202.GA32071@elie> ``` On Mon, 5 Sep 2011, Jonathan Nieder wrote: > + if (!curl_errorstr[0]) > + strlcpy(curl_errorstr, > + curl_easy_strerror(results.curl_result), > + sizeof(curl_errorstr)); (as libcurl hacker) For the record, in libcurl we work on providing "extra" information in the error buffer when there is additional info to provide that would help. In some cases we deem there isn't (or we just to provide any) and then the generic error message is good enough and could indeed be used like this... -- / daniel.haxx.se ``` ## Junio C Hamano, 2011-09-06 17:02 Subject: Re: [RFC/PATCH git] http: avoid empty error messages for some curl errors Message-ID: <7v62l5a9ke.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v62l5a9ke.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Daniel Stenberg writes: > On Mon, 5 Sep 2011, Jonathan Nieder wrote: > >> + if (!curl_errorstr[0]) >> + strlcpy(curl_errorstr, >> + curl_easy_strerror(results.curl_result), >> + sizeof(curl_errorstr)); > > (as libcurl hacker) > > For the record, in libcurl we work on providing "extra" information in > the error buffer when there is additional info to provide that would > help. In some cases we deem there isn't (or we just to provide any) > and then the generic error message is good enough and could indeed be > used like this... Sorry if I am a bit slow but are you saying: (1) we provide "extra" but your patch is not using it which is bad? (2) the above is Ok but there are better ways to do it? (3) something else? ``` ## Daniel Stenberg, 2011-09-06 18:00 Subject: Re: [RFC/PATCH git] http: avoid empty error messages for some curl errors Message-ID: URL: https://gitlist.dev/e/alpine.DEB.2.00.1109061957120.29735%40tvnag.unkk.fr In-Reply-To: <7v62l5a9ke.fsf@alter.siamese.dyndns.org> ``` On Tue, 6 Sep 2011, Junio C Hamano wrote: >> >> For the record, in libcurl we work on providing "extra" information in >> the error buffer when there is additional info to provide that would >> help. In some cases we deem there isn't (or we just to provide any) >> and then the generic error message is good enough and could indeed be >> used like this... > > Sorry if I am a bit slow but are you saying: > > (1) we provide "extra" but your patch is not using it which is bad? > (2) the above is Ok but there are better ways to do it? > (3) something else? Sorry for being unclear. I was trying to explain why the proposed patch makes sense and that it will continue to make sense even if in future libcurl versions it would start returning error messages for errors where currently it returns none. Thus, I am in favour of the general idea of the patch - I have no comment for the exact implementation as I haven't checked the details. -- / daniel.haxx.se ``` ## Junio C Hamano, 2011-09-06 22:49 Subject: Re: [RFC/PATCH git] http: avoid empty error messages for some curl errors Message-ID: <7vehzt70c1.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vehzt70c1.fsf%40alter.siamese.dyndns.org In-Reply-To: <20110905222202.GA32071@elie> ``` Jonathan Nieder writes: > When asked to fetch over SSL without a valid > /etc/ssl/certs/ca-certificates.crt file, "git fetch" writes > > error: while accessing https://github.com/torvalds/linux.git/info/refs > > which is a little disconcerting. Better to fall back to > curl_easy_strerror(result) when the error string is empty, like the > curl utility does: > > error: Problem with the SSL CA cert (path? access rights?) while > accessing https://github.com/torvalds/linux.git/info/refs > > Signed-off-by: Jonathan Nieder > --- > Hi, > > I ran into this error today because this machine has no certs enabled. > I'm not thrilled with the following patch because the error string > buffer is not cleared very often so it seems possible for it to be > not empty but stale at the relevant moment. I would be happier if we > could rely on libcurl always filling the error buffer on errors. Will queue both patches; thanks. ```