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
