PR28284: add tweaks on previous debuginfod x-debuginfod* header forwarding work

Embrace case-independent headers, more fully document, handle HTTP \r.
In addition to test case, hand-tested against fedora debuginfod
instances, running federated servers under valgrind.

Signed-off-by: Frank Ch. Eigler <fche@redhat.com>

11 files changed, 96 insertions, 65 deletions

diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 8c843f3a..bc6c11c9 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,11 +1,21 @@
+2022-09-06  Frank Ch. Eigler  <fche@redhat.com>
+
+	* debuginfod-client.c (header_callback): Don't copy \r in x-d headers.
+	Print all headers in verbose_fd mode.
+	* debuginfod-find.c (parse_opt): Set verbose_fd only at verbosity >= 2.
+	* debuginfod.cxx (handle_buildid): Clean up header forwarding
+	string processing.
+	* debuginfod.h.in: (debuginfod_get_headers): Tweak wording.
+	* libdebuginfod.map: Use ELFUTILS_0.188 for new function.
+
 2022-07-15  Noah Sanci  <nsanci@redhat.com>
 
-	* debuginfod-client.c (header_callback): Handle headers without
+	* debuginfod-client.c (header_callback): Ignore headers without
 	X-DEBUGINFOD prefix.
 	(debuginfod_query_server): Removed verbose printing headers when
 	undesired.
 	(debuginfod_get_headers): Created.
-	* debuginfod-find.c (main): Verboes printing headers.
+	* debuginfod-find.c (main): Verbose printing headers.
 	* debuginfod.cxx (handle_buildid): Add headers prefixed with
 	X-DEBUGINFOD from federated servers to this server's response
 	headers.
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index a3565f57..54dc3f2f 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -499,33 +499,37 @@ default_progressfn (debuginfod_client *c, long a, long b)
 static size_t
 header_callback (char * buffer, size_t size, size_t numitems, void * userdata)
 {
+  struct handle_data *data = (struct handle_data *) userdata;
   if (size != 1)
     return 0;
-  // X-DEBUGINFOD is 11 characters long.
+  if (data->client && data->client->verbose_fd >= 0)
+    dprintf (data->client->verbose_fd, "header %.*s", (int)numitems, buffer);
   // 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, ":")) ){
+  if (strncasecmp(buffer, "X-DEBUGINFOD", 11)
+      || buffer[numitems-2] != '\r'
+      || buffer[numitems-1] != '\n'
+      || (buffer == strstr(buffer, ":")) ){
     return numitems;
   }
   /* Temporary buffer for realloc */
   char *temp = NULL;
-  struct handle_data *data = (struct handle_data *) userdata;
   if (data->response_data == NULL)
     {
-      temp = malloc(numitems+1);
+      temp = malloc(numitems);
       if (temp == NULL)
         return 0;
     }
   else
     {
-      temp = realloc(data->response_data, data->response_data_size + numitems + 1);
+      temp = realloc(data->response_data, data->response_data_size + numitems);
       if (temp == NULL)
         return 0;
     }
 
-  memcpy(temp + data->response_data_size, buffer, numitems);
+  memcpy(temp + data->response_data_size, buffer, numitems-1);
   data->response_data = temp;
-  data->response_data_size += numitems;
+  data->response_data_size += numitems-1;
+  data->response_data[data->response_data_size-1] = '\n';
   data->response_data[data->response_data_size] = '\0';
   return numitems;
 }
@@ -1072,11 +1076,9 @@ debuginfod_query_server (debuginfod_client *c,
   int committed_to = -1;
   bool verbose_reported = false;
   struct timespec start_time, cur_time;
-  if (c->winning_headers != NULL)
-    {
-      free (c->winning_headers);
-      c->winning_headers = NULL;
-    }
+
+  free (c->winning_headers);
+  c->winning_headers = NULL;
   if ( maxtime > 0 && clock_gettime(CLOCK_MONOTONIC_RAW, &start_time) == -1)
     {
       rc = errno;
diff --git a/debuginfod/debuginfod-find.c b/debuginfod/debuginfod-find.c
index fb1f294c..778fb09b 100644
--- a/debuginfod/debuginfod-find.c
+++ b/debuginfod/debuginfod-find.c
@@ -99,7 +99,8 @@ static error_t parse_opt (int key, char *arg, struct argp_state *state)
     {
     case 'v': verbose++;
       debuginfod_set_progressfn (client, & progressfn);
-      debuginfod_set_verbose_fd (client, STDERR_FILENO);
+      if (verbose > 1)
+        debuginfod_set_verbose_fd (client, STDERR_FILENO);
       break;
     default: return ARGP_ERR_UNKNOWN;
     }
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 27b671d3..000a41c4 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -2093,22 +2093,26 @@ and will not query the upstream servers");
             {
               add_mhd_response_header (r, "Content-Type",
 				       "application/octet-stream");
+              // Copy the incoming headers
               const char * hdrs = debuginfod_get_headers(client);
               string header_dup;
               if (hdrs)
                 header_dup = string(hdrs);
-              size_t pos = 0;
-              // Clean winning headers to add all X-DEBUGINFOD lines to the package we'll send
-              while( (pos = header_dup.find("X-DEBUGINFOD")) != string::npos)
+              // Parse the "header: value\n" lines into (h,v) tuples and pass on
+              while(1)
                 {
-                  // Focus on where X-DEBUGINFOD- begins
-                  header_dup = header_dup.substr(pos);
-                  size_t newline =  header_dup.find('\n');
-                  if (newline == string::npos)
-                    break;
-                  add_mhd_response_header(r, header_dup.substr(0,header_dup.find(':')).c_str(),
-                                             header_dup.substr(header_dup.find(':')).c_str());
-                  header_dup = header_dup.substr(newline);
+                  size_t newline = header_dup.find('\n');
+                  if (newline == string::npos) break;
+                  size_t colon = header_dup.find(':');
+                  if (colon == string::npos) break;
+                  string header = header_dup.substr(0,colon);
+                  string value = header_dup.substr(colon+1,newline-colon-1);
+                  // strip leading spaces from value
+                  size_t nonspace = value.find_first_not_of(" ");
+                  if (nonspace != string::npos)
+                    value = value.substr(nonspace);
+                  add_mhd_response_header(r, header.c_str(), value.c_str());
+                  header_dup = header_dup.substr(newline+1);
                 }
 
               add_mhd_last_modified (r, s.st_mtime);
diff --git a/debuginfod/debuginfod.h.in b/debuginfod/debuginfod.h.in
index 6ae8b91c..40b1ea00 100644
--- a/debuginfod/debuginfod.h.in
+++ b/debuginfod/debuginfod.h.in
@@ -93,8 +93,8 @@ void* debuginfod_get_user_data (debuginfod_client *client);
 /* Get the current or last active URL, if known.  */
 const char* debuginfod_get_url (debuginfod_client *client);
 
-/* Returns all headers sent to this client which were prefixed
- * with X-DEBUGINFOD */
+/* Returns set of x-debuginfod* header lines received from current or
+   last active transfer, \n separated, if known. */
 const char* debuginfod_get_headers(debuginfod_client *client);
 
 /* Add an outgoing HTTP request  "Header: Value".  Copies string.  */
diff --git a/debuginfod/libdebuginfod.map b/debuginfod/libdebuginfod.map
index f95b5b9a..93964167 100644
--- a/debuginfod/libdebuginfod.map
+++ b/debuginfod/libdebuginfod.map
@@ -18,6 +18,6 @@ ELFUTILS_0.179 {
 ELFUTILS_0.183 {
   debuginfod_set_verbose_fd;
 } ELFUTILS_0.179;
-ELFUTILS_0.189 {
+ELFUTILS_0.188 {
   debuginfod_get_headers;
 } ELFUTILS_0.183;
diff --git a/doc/ChangeLog b/doc/ChangeLog
index c2a01a04..b2bb4890 100644
--- a/doc/ChangeLog
+++ b/doc/ChangeLog
@@ -1,4 +1,9 @@
+2022-09-02  Frank Ch. Eigler  <fche@redhat.com>
+
+	* debuginfod_find_debuginfo.3: Tweaked debuginfod_get_headers docs.
+
 2022-07-15  Noah Sanci  <nsanci@redhat.com>
+
 	* debuginfod_find_debuginfo.3: Explained debuginfod_get_headers
 	usage.
 	* debuginfod_get_headers.3: Created.
diff --git a/doc/debuginfod_find_debuginfo.3 b/doc/debuginfod_find_debuginfo.3
index 984fda12..aebbec3f 100644
--- a/doc/debuginfod_find_debuginfo.3
+++ b/doc/debuginfod_find_debuginfo.3
@@ -199,17 +199,18 @@ By default, the library adds a descriptive \fIUser-Agent:\fP
 header to outgoing requests.  If the client application adds
 a header with the same name, this default is suppressed.
 
+During or after a lookup, a client application may call
 .BR \%debuginfod_get_headers ()
-may be called with a debuginfod client. This function will return the
-http response headers prefixed with
+to gather the subset of HTTP response headers received from the
+current or most recent debuginfod server.  Only those 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
-\.
+(case-insensitive) are kept.  They are returned as a single string,
+with each "header: value" terminated with a \\n (not \\r\\n as in
+HTTP).  It may be NULL.  The resulting string is owned by the library,
+and must not be modified or freed.  The caller should copy the
+returned string if it is needed beyond the release of the client
+object.
 
 .SH "MACROS"
 
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 659bfa19..8d87f05d 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,10 @@
+2022-09-02  Frank Ch. Eigler  <fche@redhat.com>
+
+	* run-debuginfod-response-headers.sh: Use case-insensitive
+	header name matches.  Use socat & sleep for greater
+	portability.
+	* run-debuginfod-sizetime.sh: Update for debuginfod-find -v -v.
+
 2022-07-15  Noah Sanci  <nsanci@redhat.com>
 
 	* run-debuginfod-response-headers.sh: Added test
diff --git a/tests/run-debuginfod-response-headers.sh b/tests/run-debuginfod-response-headers.sh
index e5698cc9..63f2f241 100755
--- a/tests/run-debuginfod-response-headers.sh
+++ b/tests/run-debuginfod-response-headers.sh
@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 #
-# Copyright (C) 2019-2021 Red Hat, Inc.
+# Copyright (C) 2022 Red Hat, Inc.
 # This file is part of elfutils.
 #
 # This file is free software; you can redistribute it and/or modify
@@ -16,6 +16,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+type socat 2>/dev/null || exit 77
+
 . $srcdir/debuginfod-subr.sh  # includes set -e
 
 # for test case debugging, uncomment:
@@ -76,8 +78,8 @@ tempfiles vlog-find$PORT1.1
 errfiles vlog-find$PORT1.1
 cat vlog-find$PORT1.1
 grep 'Headers:' vlog-find$PORT1.1
-grep 'X-DEBUGINFOD-FILE: prog' vlog-find$PORT1.1
-grep 'X-DEBUGINFOD-SIZE: '     vlog-find$PORT1.1
+grep -i 'X-DEBUGINFOD-FILE: prog' vlog-find$PORT1.1
+grep -i 'X-DEBUGINFOD-SIZE: '     vlog-find$PORT1.1
 
 # Check to see if an executable file located in an archive prints the file's description and archive
 env DEBUGINFOD_URLS="http://127.0.0.1:"$PORT1 LD_LIBRARY_PATH=$ldpath ${abs_top_builddir}/debuginfod/debuginfod-find\
@@ -86,15 +88,15 @@ tempfiles vlog-find$PORT1.2
 errfiles vlog-find$PORT1.2
 cat vlog-find$PORT1.2
 grep 'Headers:'               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
+grep -i 'X-DEBUGINFOD-FILE: '    vlog-find$PORT1.2
+grep -i 'X-DEBUGINFOD-SIZE: '    vlog-find$PORT1.2
+grep -i 'X-DEBUGINFOD-ARCHIVE: ' vlog-find$PORT1.2
 
 # Check that X-DEBUGINFOD-SIZE matches the size of each file
 for file in vlog-find$PORT1.1 vlog-find$PORT1.2
 do
     st_size=$(stat -c%s $(tail -n 1 $file))
-    x_debuginfod_size=$(grep 'X-DEBUGINFOD-SIZE' $file | egrep -o '[0-9]+')
+    x_debuginfod_size=$(grep -i 'X-DEBUGINFOD-SIZE' $file | head -1 | egrep -o '[0-9]+')
     test $st_size -eq $x_debuginfod_size
 done
 
@@ -106,31 +108,30 @@ mkdir -p ${PWD}/$netcat_dir
 cp F/prog ${PWD}/$netcat_dir/executable
 tempfiles F/prog
 
-# Netcat dies after answering the request
-nc -l -p $PORT2 -c 'echo -e "HTTP/1.1 200 OK\nX-DEBUGINFOD-SIZE: ba:d_size\nX-DEBUGINFOD-\rFILE:\=\+ \r213\n\n $(date)"' & < ${PWD}/$netcat_dir"executable" &
-# Wait until the netcat port is in use. Otherwise debuginfod-find can query
+# socat should after answering one request
+(echo -e "HTTP/1.1 200 OK\r\nX-DEBUGINFOD-SIZE: ba:d_size\nX-DEBUGINFOD-\rFILE:\=\+ \r213\n\n $(date)" | socat -u - tcp-listen:$PORT2) &
+PID2=$!
+# Wait a bit until the netcat port is in use. Otherwise debuginfod-find can query
 # before netcat is ready.
-SECONDS=0
-nc_start=$SECONDS
-while [ ! $(lsof -i -P -n | grep LISTEN | grep "nc.*$PORT2")  ]
-do
-  # If it takes longer than 5 seconds for netcat to start up, then fail
-  duration=$(( SECONDS - nc_start ))
-  if [ $SECONDS -gt 5 ]
-  then
-    err
-  fi
-done
+sleep 5
 
-env DEBUGINFOD_URLS="http://127.0.0.1:"$PORT2 LD_LIBRARY_PATH=$ldpath ${abs_top_builddir}/debuginfod/debuginfod-find\
-    -vvv executable $BUILDID > vlog-find$PORT2 2>&1
+touch vlog-find$PORT2
 errfiles vlog-find$PORT2
 tempfiles vlog-find$PORT2
-cat vlog-find$PORT2 | grep "X-DEBUGINFOD-"
+
+# calling out to valgrind deliberately, because this process will be forced to parse broken http headers
+${VALGRIND_CMD} env DEBUGINFOD_URLS="http://127.0.0.1:"$PORT2 LD_LIBRARY_PATH=$ldpath ${abs_top_builddir}/debuginfod/debuginfod-find\
+    -vvv executable $BUILDID > vlog-find$PORT2 2>&1 || true # permit curl rejection of the bad headers
+cat vlog-find$PORT2 # won't have any valid x-debuginfod* headers
 rm -f "$netcat_dir"executable
 rmdir -p $netcat_dir
 
+kill $PID2 || true
+wait $PID2 || true
+PID2=0
+
 kill $PID1
 wait $PID1
 PID1=0
+
 exit 0
diff --git a/tests/run-debuginfod-sizetime.sh b/tests/run-debuginfod-sizetime.sh
index 2cf6f252..17307c02 100755
--- a/tests/run-debuginfod-sizetime.sh
+++ b/tests/run-debuginfod-sizetime.sh
@@ -51,7 +51,7 @@ wait_ready $PORT1 'thread_busy{role="scan"}' 0
 # Ensure DEBUGINFOD_MAXSIZE is functional and sends back the correct http
 # code
 env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_RETRY_LIMIT=1 DEBUGINFOD_URLS="http://127.0.0.1:$PORT1/" DEBUGINFOD_MAXSIZE=1 \
-    ${abs_top_builddir}/debuginfod/debuginfod-find -v executable ${PWD}/prog 2> find-vlog$PORT1 || true
+    ${abs_top_builddir}/debuginfod/debuginfod-find -v -v executable ${PWD}/prog 2> find-vlog$PORT1 || true
 tempfiles find-vlog$PORT1
 errfiles  find-vlog$PORT1
 echo "Checking maxsize"
@@ -66,7 +66,7 @@ if [ -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID} ]; then
 fi
 # Ensure no file is downloaded for longer than DEBUGINFOD_MAXTIME
 env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS="http://127.0.0.1:$PORT1/" DEBUGINFOD_MAXTIME=1 \
-    ${abs_top_builddir}/debuginfod/debuginfod-find -v debuginfo ${PWD}/prog.debug 2> find-vlog$PORT1 || true
+    ${abs_top_builddir}/debuginfod/debuginfod-find -v -v debuginfo ${PWD}/prog.debug 2> find-vlog$PORT1 || true
 tempfiles find-vlog$PORT1
 grep 'using max time' find-vlog$PORT1
 # Ensure p+r%o\$g.debug is NOT cached