garbage error output from failed curl

[ryandesign (Ryan Carsten Schmidt)]

I'm trying to fix an old port by fetching its distfiles from archive.org. archive.org is having some problem today where I have to refresh its pages several times before it shows me the information I want; other times I get various connection errors. In addition, connections to "distfiles.macports.org" will not succeed because on my network that is mapped to a custom server that is offline right now.

Trying to fetch the modified port with MacPorts (on High Sierra) I got this output:

--->  Attempting to fetch p4d from https://web.archive.org/web/20101114082044if_/http://filehost.perforce.com/perforce/r10.1/bin.darwin80u/
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  125k    0  125k    0     0   112k      0 --:--:--  0:00:01 --:--:--  112k
DEBUG: Fetching distfile failed: transfer closed with outstanding read data remaining
--->  Attempting to fetch p4d from https://distfiles.macports.org/perforce/2010.1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0DEBUG: Fetching distfile failed: ‚½œÅ
Error: Failed to fetch perforce: ‚½œÅ
DEBUG: Error code: NONE
DEBUG: Backtrace: ‚½œÅ
    while executing
"error $lastError"
    (procedure "portfetch::fetchfiles" line 66)
    invoked from within
"portfetch::fetchfiles"
    (procedure "portfetch::fetch_main" line 17)
    invoked from within
"$procedure $targetname"
Error: See /opt/local/var/macports/logs/_Users_rschmidt_macports_macports-ports-svn-trunk-new_devel_perforce/perforce/main.log for details.
Error: Follow https://guide.macports.org/#project.tickets to report a bug.
Error: Processing of port perforce failed

Regardless of how badly the servers from which I'm downloading are behaving, I should never see an "error message" like ½œÅ.

[ryandesign (Ryan Carsten Schmidt)]

Looking through the code in CurlFetchCmd in src/pextlib1.0/curl.c, I see this:

	char theErrorString[CURL_ERROR_SIZE];

		theCurlCode = curl_easy_setopt(theHandle, CURLOPT_ERRORBUFFER, theErrorString);
		if (theCurlCode != CURLE_OK) {
			theResult = SetResultFromCurlErrorCode(interp, theCurlCode);
			break;
		}

This tells curl to fill theErrorString with an error message if it occurs.

We later use that variable like this:

		if (info->data.result != CURLE_OK) {
			/* execution failed, use the error string */
			Tcl_SetResult(interp, theErrorString, TCL_VOLATILE);
			theResult = TCL_ERROR;
			break;
		}

I think the problem is that curl does not guarantee that it has filled the error buffer with anything, even if an error occurred. From ​the documentation:

Pass a char * to a buffer that libcurl may store human readable error messages [in] on failures or problems. [...] Do not rely on the contents of the buffer unless an error code was returned. Since 7.60.0 libcurl will initialize the contents of the error buffer to an empty string before performing the transfer. For earlier versions if an error code was returned but there was no error detail then the buffer is untouched.

I was using High Sierra, whose libcurl version if 7.54.0, which means it does not initialize the error buffer to the empty string. It would probably be a good idea for us to initialize it to the empty string. And later when we want to pass it to Tcl, we might want to check if it is still the empty string and if so substitute a generic error message.

[ryandesign (Ryan Carsten Schmidt)]

The example code in the documentation shows that the buffer should be initialized to the empty string using:

  char errbuf[CURL_ERROR_SIZE];

  /* set the error buffer as empty before performing a request */
  errbuf[0] = 0;

[neverpanic (Clemens Lang)]

​https://github.com/macports/macports-base/pull/186

[neverpanic (Clemens Lang)]

In f535c26af9e12302c7eb316c8aab1aa5f2d149d9/macports-base (master):

pextlib1.0: init err buffer, use err code if empty

curl < 7.60.0 does not initialize the error buffer, and cases have been
reported where a curl download failed without the error buffer being
set. This was the case on older systems that did not yet have curl
7.60.0, and caused random garbage to be printed.

0-initialize the buffer and check whether the buffer has a useful
message before returning it. Generate a substitute message using
curl_easy_strerror() from the returned error code if the error buffer is
not set.

Closes: #60581

[neverpanic (Clemens Lang)]

Cherry-pick to release-2.6 in [cd583fee1071e93367abc89f8bd4e7128621404a/macports-base].

[neverpanic (Clemens Lang)]

Just seen this in build output generated by Travis, and the change seems to work as expected. See ​https://paste.macports.org/75dc136dfd83:

Error: Failed to fetch py38-lxml: curl_multi_info_read() returned {.msg = CURLMSG_DONE, .data.result = 7 (!= CURLE_OK)}, but the error buffer is not set. curl_easy_strerror(.data.result): Couldn't connect to server