Re: [Bug debuginfod/27277] Describe retrieved files when verbose

[Mark Wielaard <mark@klomp.org>]

Hi Noah,

On Thu, 2022-07-14 at 11:32 -0400, Noah Sanci via Elfutils-devel wrote:
> Please find the patch for pr28284 attached
> 
>     Debuginfod and debuginfod clients are now equipped to send
>     and receive http headers prefixed with X-DEBUGINFOD and
>     print them in verbose mode for more context

OK, so this patch does 3 things:

- Introduce a debuginfod_get_headers public api for libdebuginfod
  that provides any X-DEBUGINFOD prefixed headers for the last
  retrieved file if the http(s) protocol was used (but not for the file
  protocol nor when the file was found in the cache).

- It changes debuginfod-find to use this new api to show those headers
  in verbose mode.

- Changes the debuginfod server to use the api to add any
  X-DEBUGINFOD headers to the response if the request was
  delegated.

So the last two are obvious once we have the first. I am slightly
worried about the "inconsistency" in when these headers are available.
Should we maybe cache them? Or do they only really make sense during
download/in the progress callback?

There currently are 3 special headers:

- X-DEBUGINFOD-FILE the (base) file name of the original file
- X-DEBUGINFOD-SIZE the on-disk size of the file
- X-DEBUGINFOD-ARCHIVE the archive name the file came from

All three are helpful for debug/verbose output. The size is independent
of the Content-Encoding and so is useful for progress
callback/filtering. The archive could be helpful if we had a mechanism
for getting all content such an archive.

The description of the new debuginfod_get_headers api is:

+.BR \%debuginfod_get_headers ()
+may be called with a debuginfod client. This function will return the
+http response headers prefixed with
+.BR X-DEBUGINFOD
+received from the first handle to get a response from a debuginfod server.
+Note that all other http headers aren't stored in the libcurl header
+callback function since they aren't of as much interest. The caller should
+copy the returned string if it is needed beyond the release of the client object.
+The returned string may be NULL if no headers are prefixed with
+.BR X-DEBUGINFOD
+\.

Looking at the headers the headers are cleaned up when a new request is
made using the same client object (not just when the client object is
released). 

Also I am not getting any headers because they seem to be turned into
lower-case by curl. e.g. from debuginfod.fedoraproject.org:

< date: Thu, 04 Aug 2022 12:56:20 GMT
< content-type: application/octet-stream
< x-debuginfod-size: 4896240
< x-debuginfod-archive: /mnt/fedora_koji_prod/koji/packages/bash/5.1.16/2.fc36/x86_64/bash-debuginfo-5.1.16-2.fc36.x86_64.rpm
< x-debuginfod-file: /usr/lib/debug/usr/bin/bash-5.1.16-2.fc36.x86_64.debug
< last-modified: Wed, 19 Jan 2022 22:15:52 GMT

And according to the RFC "Each header field consists of a name followed by a colon (":") and the field value. Field names are case-insensitive." So at least in the code we always need to compare case-insensitive.

+  // X-DEBUGINFOD is 11 characters long.
+  // Some basic checks to ensure the headers received are of the expected format
+  if ( strncmp(buffer, "X-DEBUGINFOD", 11) || buffer[numitems-1] != '\n'
+       || (buffer == strstr(buffer, ":")) ){
+    return numitems;
+  }

and in debuginfod.cxx

+              // Clean winning headers to add all X-DEBUGINFOD lines to the pac
kage we'll send
+              while( (pos = header_dup.find("X-DEBUGINFOD")) != string::npos)
+                {
+                  // Focus on where X-DEBUGINFOD- begins

I do wonder if we should simply say that the debuginfod_get_headers is only valid during a progress function callback. But maybe that is too restrictive. If not there is a slight difference between calling debuginfod_get_headers during a progress function and afterwards if there was an error during download (then we seem to clean them). Which probably good, but should be documented.

Also I probably won't mind providing all header, at least during the progress function callback. But you might be right that they aren't as informative.

Cheers,

Mark