Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#60581 closed defect (fixed)

garbage error output from failed curl

Reported by: ryandesign (Ryan Schmidt) Owned by: neverpanic (Clemens Lang)
Priority: Normal Milestone: MacPorts 2.6.3
Component: base Version: 2.6.2
Keywords: Cc: neverpanic (Clemens Lang)
Port:

Description

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 ½œÅ.

Change History (6)

comment:1 Changed 2 years ago by ryandesign (Ryan Schmidt)

Cc: neverpanic added

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 is 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.

Last edited 2 years ago by ryandesign (Ryan Schmidt) (previous) (diff)

comment:2 Changed 2 years ago by ryandesign (Ryan 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;

comment:4 Changed 2 years ago by neverpanic (Clemens Lang)

Owner: set to neverpanic
Resolution: fixed
Status: newclosed

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

comment:5 Changed 2 years ago by neverpanic (Clemens Lang)

Milestone: MacPorts 2.6.3

comment:6 Changed 2 years ago by 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
Note: See TracTickets for help on using tickets.