From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id BB5363858439 for ; Mon, 8 Aug 2022 14:35:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BB5363858439 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: by gnu.wildebeest.org (Postfix, from userid 1000) id BC10930012BF; Mon, 8 Aug 2022 16:35:01 +0200 (CEST) Date: Mon, 8 Aug 2022 16:35:01 +0200 From: Mark Wielaard To: Noah Sanci Cc: elfutils-devel@sourceware.org Subject: Re: [Bug debuginfod/27277] Describe retrieved files when verbose Message-ID: <20220808143501.GA553@gnu.wildebeest.org> References: <35f2073dfeed8f008d42a78bf60b7efcf13164eb.camel@klomp.org> <20210806185459.GE4195@redhat.com> <9ac621fee207ef233873c40843b3d34ced9019cc.camel@klomp.org> <20210922203331.GC13236@redhat.com> <599234fd5a36629b580d0a615ac069835295111f.camel@klomp.org> <20210929212847.GA11484@redhat.com> <625c4cfc6232c747e6b708aa42eaf0018d821f29.camel@klomp.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: elfutils-devel@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Elfutils-devel mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Aug 2022 14:35:04 -0000 Hi Noah, On Fri, Aug 05, 2022 at 03:21:48PM -0400, Noah Sanci wrote: > On Thu, Aug 4, 2022 at 9:12 AM Mark Wielaard wrote: > > > > 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? > > Not quite sure what you mean here, they're available from the time a server > is chosen and the file downloaded until when debuginfod_end is called. Are > you referring to caching outside the lifetime of the debuginfod_client > or within its lifetime? I am trying to figure out what the use case is. If the use case is simply providing information during the download then having/promising it is just available during the progress callback function might make things easier. There are two quirks making the winning headers available after the download has concluded. If there was an error during download the headers are erased. Likewise when the handle is reused. This might be fine, but then has to be documented clearly. And if there isn't really a usecase then to have the headers available after the download has succeeded then it can also simply be documented as only being valid during/from a progress function callback. > > 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). > > If I understand what's being said, the implication is that there is > potential for a single client to make multiple requests which would > unintentionally overwrite previous winning_headers. If the concern > is overwriting, winning_headers can be expanded into an array of all > winning headers and a function created to search for headers based > on header data such as X-DEBUGINFOD-FILE. It isn't bad when that happens as long it is clear to the user. It also means that it might be simpler to just guarantee access to the headers during the progress function callback and not promise anything about what the call returns after the (possible) download. > > 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. > > Both solutions implemented in the below patch will fix the issue > assuming that the case is consistent (i.e the prefix isn't > X-dEbUgInfod), That is unlikely, but isn't really an invalid header. I can imagine someone using X-debuginfod or X-Debuginfod for example. So why not simply use strncasecmp. > however, > for the debuginfod.cxx header cleaning the case of the original > headers are forced to upper case and not preserved. I hope that is not > an issue. That shouldn't be a problem. > @@ -1111,8 +1118,6 @@ debuginfod_query_server (debuginfod_client *c, > if (c->winning_headers == NULL) > { > c->winning_headers = data[committed_to].response_data; > - if (vfd >= 0 && c->winning_headers != NULL) > - dprintf(vfd, "\n%s", c->winning_headers); > data[committed_to].response_data = NULL; > data[committed_to].response_data_size = 0; > } Note that previously this provided the full http headers in verbose mode before/during the actualy download. While now debuginfod-find --verbose will only provide the X-Debuginfod- headers after the download. I can see how only the X-debuginfod headers are useful after the call, but during the progress callback function maybe the full headers are useful to determine whether or not to abort the download early? Cheers, Mark