debuginfod: Query debuginfod servers before printing response

debuginfod: Query debuginfod servers before printing response

[Noah Sanci]

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

Hello,

While checking PR27277 on some buildbots, greping would fail in
run-debuginfod-response-headers.sh. This was because querying the
debuginfod server occurs after checking if the response headers had
arrived, leaving the possibility to leave the querying loop before
outputting the headers which caused the grep failure. Querying now
occurs before checking if response headers have arrived, so that they
will certainly be printed and grep will find them.

Regards,

Noah Sanci

[-- Attachment #2: 0001-debuginfod-Query-debuginfod-servers-before-printing-.patch --]
[-- Type: text/x-patch, Size: 2086 bytes --]

From 8626a4786c1e79e1b4891ea31966bc124e029378 Mon Sep 17 00:00:00 2001
From: Noah Sanci <nsanci@redhat.com>
Date: Fri, 17 Sep 2021 10:45:39 -0400
Subject: [PATCH] debuginfod: Query debuginfod servers before printing response

While checking PR27277 on some buildbots, greping would fail in
run-debuginfod-response-headers.sh. This was because querying the
debuginfod server occurs after checking if the responseh headers had
arrived, leaving the possibility to leave the querying loop before
outputting the headers which caused the grep failure. Querying now
occurs before checking if response headers have arrived, so that they
will certainly be printed and grep will find them.

Signed-off-by: Noah Sanci <nsanci@redhat.com>
---
 debuginfod/ChangeLog           | 5 +++++
 debuginfod/debuginfod-client.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index e2e6c5f8..c2bfce98 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,8 @@
+2021-09-17  Noah Sanci  <nsanci@redhat.com>
+
+	* debuginfod-client.c (debuginfod_query_server): curl_multi_perform
+	now occurs before checking if response headers have arrived.
+
 2021-09-14  Frank Ch. Eigler <fche@redhat.com>
 
 	PRPR28339
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 4d5dbd95..88e45567 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -1046,6 +1046,7 @@ debuginfod_query_server (debuginfod_client *c,
         }
       /* Wait 1 second, the minimum DEBUGINFOD_TIMEOUT.  */
       curl_multi_wait(curlm, NULL, 0, 1000, NULL);
+      CURLMcode curlm_res = curl_multi_perform(curlm, &still_running);
 
       /* If the target file has been found, abort the other queries.  */
       if (target_handle != NULL)
@@ -1077,7 +1078,6 @@ debuginfod_query_server (debuginfod_client *c,
 	  verbose_reported = true;
 	}
 
-      CURLMcode curlm_res = curl_multi_perform(curlm, &still_running);
       if (curlm_res != CURLM_OK)
         {
           switch (curlm_res)
-- 
2.31.1

Re: debuginfod: Query debuginfod servers before printing response

[Mark Wielaard]

Hi Noah,

On Fri, 2021-09-17 at 10:55 -0400, Noah Sanci via Elfutils-devel wrote:
> While checking PR27277 on some buildbots, greping would fail in
> run-debuginfod-response-headers.sh. This was because querying the
> debuginfod server occurs after checking if the response headers had
> arrived, leaving the possibility to leave the querying loop before
> outputting the headers which caused the grep failure. Querying now
> occurs before checking if response headers have arrived, so that they
> will certainly be printed and grep will find them.

Thanks for finding this issue. Please push.

Cheers,

Mark

Re: debuginfod: Query debuginfod servers before printing response

[Noah Sanci]

[-- Attachment #1: Type: text/plain, Size: 248 bytes --]

Hello,

Here is an updated patch, removing checks for http response headers
which debuginfod is not guaranteed
to respond with. These headers caused tests to fail despite receiving
sufficient headers to confirm functionality.

Regards,

Noah Sanci

[-- Attachment #2: 0001-debuginfod-Remove-checking-for-unsafe-headers.patch --]
[-- Type: text/x-patch, Size: 2443 bytes --]

From 28db5f16c44fa7bbd24b221b65aa4d133753355c Mon Sep 17 00:00:00 2001
From: Noah Sanci <nsanci@redhat.com>
Date: Fri, 17 Sep 2021 10:45:39 -0400
Subject: [PATCH] debuginfod: Remove checking for unsafe headers

Some http response header checks were removed such as checking for
Connection and Cache-Control. These headers are not guarenteed to be
received and depend on proxy and libmicrohttpd versions. Checking for
the existance of Content-Length and DEBUGINFOD-* headers is sufficient
since Content-Length is added upon creation of an MHD_Response object
and DEBUGINFOD-* are added manually.
(source on Content-Length being added:
https://www.gnu.org/software/libmicrohttpd/manual/libmicrohttpd.html#
    microhttpd_002dresponse-headers )

Signed-off-by: Noah Sanci <nsanci@redhat.com>
---
 tests/ChangeLog                          | 5 +++++
 tests/run-debuginfod-response-headers.sh | 4 ----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/ChangeLog b/tests/ChangeLog
index c73f2534..b62bb350 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,8 @@
+2021-09-17  Noah Sanci  <nsanci@redhat.com>
+
+	* run-debuginfod-response-header.sh: removed checking for Connection
+	and Cache-Control in response headers.
+
 2021-09-08  Mark Wielaard  <mark@klomp.org>
 
 	* run-varlocs-vars.sh: New test.
diff --git a/tests/run-debuginfod-response-headers.sh b/tests/run-debuginfod-response-headers.sh
index bdb39b4d..10b2ab49 100755
--- a/tests/run-debuginfod-response-headers.sh
+++ b/tests/run-debuginfod-response-headers.sh
@@ -74,8 +74,6 @@ env DEBUGINFOD_URLS="http://127.0.0.1:"$PORT1 LD_LIBRARY_PATH=$ldpath ${abs_top_
     -vvv executable F/prog > vlog-find$PORT1.1 2>&1
 tempfiles vlog-find$PORT1.1
 grep 'Content-Length: ' vlog-find$PORT1.1
-grep 'Connection: ' vlog-find$PORT1.1
-grep 'Cache-Control: ' vlog-find$PORT1.1
 grep 'X-DEBUGINFOD-FILE: ' vlog-find$PORT1.1
 grep 'X-DEBUGINFOD-SIZE: ' vlog-find$PORT1.1
 
@@ -84,8 +82,6 @@ env DEBUGINFOD_URLS="http://127.0.0.1:"$PORT1 LD_LIBRARY_PATH=$ldpath ${abs_top_
     -vvv executable c36708a78618d597dee15d0dc989f093ca5f9120 > vlog-find$PORT1.2 2>&1
 tempfiles vlog-find$PORT1.2
 grep 'Content-Length: ' vlog-find$PORT1.2
-grep 'Connection: ' vlog-find$PORT1.2
-grep 'Cache-Control: ' vlog-find$PORT1.2
 grep 'X-DEBUGINFOD-FILE: ' vlog-find$PORT1.2
 grep 'X-DEBUGINFOD-SIZE: ' vlog-find$PORT1.2
 grep 'X-DEBUGINFOD-ARCHIVE: ' vlog-find$PORT1.2
-- 
2.31.1

Re: debuginfod: Query debuginfod servers before printing response

[Mark Wielaard]

Hi Noah,

On Fri, Sep 17, 2021 at 01:38:50PM -0400, Noah Sanci via Elfutils-devel wrote: 
> Here is an updated patch, removing checks for http response headers
> which debuginfod is not guaranteed
> to respond with. These headers caused tests to fail despite receiving
> sufficient headers to confirm functionality.

Yes, this looks good. Just checking for Content-Length and the new
X-DEBUGINFOD- headers should be good enough.

Nice commit message. Please push.

Thanks,

Mark