public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
* [Bug debuginfod/27982] added DEBUGINFOD_MAXSIZE and DEBUGINFOD_MAXTIME
@ 2021-07-26 18:56 Noah Sanci
  2021-07-29 13:36 ` Mark Wielaard
  0 siblings, 1 reply; 7+ messages in thread
From: Noah Sanci @ 2021-07-26 18:56 UTC (permalink / raw)
  To: elfutils-devel

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

Hello,

Please find the attached patch for pr 27982. DEBUGINFOD_MAXSIZE and
MAXTIME were added in this patch to allow users to use more
constraints when downloading debuginfo.

Noah

[-- Attachment #2: 0001-debuginfod-PR27982-added-DEBUGINFOD_MAXSIZE-and-DEBU.patch --]
[-- Type: text/x-patch, Size: 12270 bytes --]

From 93523eb4c8f3f3ca16f3a673369689aec62324cd Mon Sep 17 00:00:00 2001
From: Noah Sanci <nsanci@redhat.com>
Date: Mon, 26 Jul 2021 13:29:11 -0400
Subject: [PATCH] debuginfod: PR27982 - added DEBUGINFOD_MAXSIZE and
 DEBUGINFOD_MAXTIME

DEBUGINFOD_TIMEOUT is a good way to catch servers that are too slow to
*start* transmitting a file.  But we have no way of limiting total
download time or space.  A user might prefer to have his debugger fetch
only quick & small files, and make do without the bigger ones.  Some
transitive dependencies of e.g. gnome programs are huge: 3GB of LLVM
debuginfo, 1GB of webkitgtk, etc. etc.
DEBUGINFOD_MAXSIZE and DEBUGINFOD_MAXTIME were added to dictate the
max download size and time of a debuginfod client. DEBUGINFOD_MAXSIZE
is handled server-side and is sent using the http header:
X-DEBUGINFOD-MAXSIZE.

https://sourceware.org/bugzilla/show_bug.cgi?id=27982

Signed-off-by: Noah Sanci <nsanci@redhat.com>
---
 debuginfod/ChangeLog           | 14 +++++++
 debuginfod/debuginfod-client.c | 71 ++++++++++++++++++++++++++++++++++
 debuginfod/debuginfod.cxx      | 13 +++++++
 debuginfod/debuginfod.h.in     |  2 +
 doc/ChangeLog                  |  6 +++
 doc/debuginfod-find.1          | 16 +++++++-
 tests/ChangeLog                |  7 ++++
 tests/run-debuginfod-find.sh   | 29 ++++++++++++++
 8 files changed, 157 insertions(+), 1 deletion(-)

diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 7709c24d..bc9b4ceb 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,17 @@
+2021-07-26  Noah Sanci  <nsanci@redhat.com>
+
+	PR27982
+	* debuginfod-client.c (globals): added default_maxsize and
+	default_maxtime.
+	(debuginfod_query_server): Added DEBUGINFOD_MAXSIZE and
+	DEBUGINFOD_MAXTIME envvar processing. DEBUGINFOD_MAXSIZE, if
+	set, sends an additional http header to the server named
+	X-DEBUGINFOD-MAXSIZE.
+	* debuginfod.cxx (handler_cb): If the requested file exceeds
+	maxsize return code 406, signaling the file was too large.
+	* debuginfod.h.in: Added DEBUGINFOD_MAXSIZE_ENV_VAR and
+	DEBUGINFOD_MAXTIME_ENV_VAR.
+
 2021-07-16  Noah Sanci  <nsanci@redhat.com>
 
 	PR28034
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 26ba1891..cc28d2a3 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -157,6 +157,10 @@ static const char url_delim_char = ' ';
 /* Timeout for debuginfods, in seconds (to get at least 100K). */
 static const long default_timeout = 90;
 
+/* Max size for debuginfods in bytes (B). Default is 0 (infinite). */
+static const long default_maxsize = 0;
+/* Max time to query servers for. Default is MAX_LONG. */
+static const long default_maxtime = LONG_MAX;
 /* Default retry count for download error. */
 static const long default_retry_limit = 2;
 
@@ -556,6 +560,39 @@ debuginfod_query_server (debuginfod_client *c,
   free (c->url);
   c->url = NULL;
 
+  /* PR 27982: Add max size if DEBUGINFOD_MAXSIZE is set. */
+  long maxsize = default_maxsize;
+  const char *maxsize_envvar;
+  maxsize_envvar = getenv(DEBUGINFOD_MAXSIZE_ENV_VAR);
+  if (maxsize_envvar != NULL)
+    maxsize = atol (maxsize_envvar);
+
+  /* PR 27982: Add max time if DEBUGINFOD_MAXTIME is set. */
+  long maxtime = default_maxtime;
+  const char *maxtime_envvar;
+  maxtime_envvar = getenv(DEBUGINFOD_MAXTIME_ENV_VAR);
+  if (maxtime_envvar != NULL)
+    maxtime = atol (maxtime_envvar);
+  if (vfd >= 0)
+    dprintf(vfd, "using max time %lds\n", maxtime);
+
+  /* Default maxsize is valid*/
+  if (maxsize > default_maxsize)
+    {
+      if (vfd)
+        dprintf (vfd, "using max size %ldB\n", maxsize);
+      char *size_header = NULL;
+      rc = asprintf (&size_header, "X-DEBUGINFOD-MAXSIZE: %ld", maxsize);
+      if (rc < 0)
+        {
+          rc = -ENOMEM;
+          goto out;
+        }
+      rc = debuginfod_add_http_header(c, size_header);
+      free(size_header);
+      if (rc < 0)
+        goto out;
+    }
   add_default_headers(c);
 
   /* Copy lowercase hex representation of build_id into buf.  */
@@ -893,8 +930,28 @@ debuginfod_query_server (debuginfod_client *c,
   long loops = 0;
   int committed_to = -1;
   bool verbose_reported = false;
+  struct timespec start_time, cur_time;
+  if (clock_gettime(CLOCK_MONOTONIC_RAW, &start_time) == -1)
+    {
+      rc = errno;
+      goto out1;
+    }
+  double delta = 0.0;
   do
     {
+      /* Check to see how long querying is taking. */
+      if (clock_gettime(CLOCK_MONOTONIC_RAW, &cur_time) == -1)
+        {
+          rc = errno;
+          goto out1;
+        }
+      delta = (double)(cur_time.tv_nsec - start_time.tv_nsec) / 1000000000;
+      if (delta > (double) maxtime)
+        {
+          dprintf(vfd, "Timeout with max time=%lds and transfer time=%fs\n", maxtime, delta );
+          rc = -ETIME;
+          goto out1;
+        }
       /* Wait 1 second, the minimum DEBUGINFOD_TIMEOUT.  */
       curl_multi_wait(curlm, NULL, 0, 1000, NULL);
 
@@ -1009,6 +1066,8 @@ debuginfod_query_server (debuginfod_client *c,
 
           if (msg->data.result != CURLE_OK)
             {
+              long resp_code;
+              CURLcode ok0;
               /* Unsuccessful query, determine error code.  */
               switch (msg->data.result)
                 {
@@ -1023,6 +1082,16 @@ debuginfod_query_server (debuginfod_client *c,
                 case CURLE_SEND_ERROR: rc = -ECONNRESET; break;
                 case CURLE_RECV_ERROR: rc = -ECONNRESET; break;
                 case CURLE_OPERATION_TIMEDOUT: rc = -ETIME; break;
+                case CURLE_HTTP_RETURNED_ERROR:
+                  ok0 = curl_easy_getinfo (msg->easy_handle,
+                                          CURLINFO_RESPONSE_CODE,
+				          &resp_code);
+                  /* 406 signals that the requested file was too large */
+                  if ( ok0 == CURLE_OK && resp_code == 406)
+                    rc = -EFBIG;
+                  else
+                    rc = -ENOENT;
+                  break;
                 default: rc = -ENOENT; break;
                 }
             }
@@ -1095,6 +1164,8 @@ debuginfod_query_server (debuginfod_client *c,
       if (efd >= 0)
         close(efd);
     }
+  else if (rc == -EFBIG)
+    goto out1;
 
   /* If the verified_handle is NULL and rc != -ENOENT, the query fails with
    * an error code other than 404, then do several retry within the retry_limit.
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 503edba7..4ddd9255 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -2102,6 +2102,13 @@ handler_cb (void * /*cls*/,
     }
   *ptr = NULL;                     /* reset when done */
   
+  const char *maxsize_string = MHD_lookup_connection_value(connection, MHD_HEADER_KIND, "X-DEBUGINFOD-MAXSIZE");
+  long maxsize = 0;
+  if (maxsize_string != NULL && maxsize_string[0] != '\0')
+    maxsize = atol(maxsize_string);
+  else
+    maxsize = 0;
+
 #if MHD_VERSION >= 0x00097002
   enum MHD_Result rc;
 #else
@@ -2190,6 +2197,12 @@ handler_cb (void * /*cls*/,
       if (r == 0)
         throw reportable_exception("internal error, missing response");
 
+      if (maxsize > 0 && http_size > maxsize)
+        {
+          MHD_destroy_response(r);
+          throw reportable_exception(406, "File too large, max size=" + std::to_string(maxsize));
+        }
+
       rc = MHD_queue_response (connection, MHD_HTTP_OK, r);
       http_code = MHD_HTTP_OK;
       MHD_destroy_response (r);
diff --git a/debuginfod/debuginfod.h.in b/debuginfod/debuginfod.h.in
index 282e523d..c358df4d 100644
--- a/debuginfod/debuginfod.h.in
+++ b/debuginfod/debuginfod.h.in
@@ -36,6 +36,8 @@
 #define DEBUGINFOD_PROGRESS_ENV_VAR "DEBUGINFOD_PROGRESS"
 #define DEBUGINFOD_VERBOSE_ENV_VAR "DEBUGINFOD_VERBOSE"
 #define DEBUGINFOD_RETRY_LIMIT_ENV_VAR "DEBUGINFOD_RETRY_LIMIT"
+#define DEBUGINFOD_MAXSIZE_ENV_VAR "DEBUGINFOD_MAXSIZE"
+#define DEBUGINFOD_MAXTIME_ENV_VAR "DEBUGINFOD_MAXTIME"
 
 /* The libdebuginfod soname.  */
 #define DEBUGINFOD_SONAME "@LIBDEBUGINFOD_SONAME@"
diff --git a/doc/ChangeLog b/doc/ChangeLog
index 05fcd23d..1822fc6b 100644
--- a/doc/ChangeLog
+++ b/doc/ChangeLog
@@ -1,3 +1,9 @@
+2021-07-26  Noah Sanci <nsanci@redhat.com>
+
+	PR27982
+	* debuginfod-find.1: Document DEBUGINFOD_MAXTIME
+	and DEBUGINFOD_MAXSIZE.
+
 2021-04-23  Frank Ch. Eigler <fche@redhat.com>
 
 	PR27701
diff --git a/doc/debuginfod-find.1 b/doc/debuginfod-find.1
index 12d4ec2d..112f2551 100644
--- a/doc/debuginfod-find.1
+++ b/doc/debuginfod-find.1
@@ -38,7 +38,6 @@ output, and exits with a success status of 0.  In case of any error,
 it exits with a failure status and an error message to standard error.
 
 .\" Much of the following text is duplicated with debuginfod.8
-
 The debuginfod system uses buildids to identify debuginfo-related data.
 These are stored as binary notes in ELF/DWARF files, and are
 represented as lowercase hexadecimal.  For example, for a program
@@ -147,6 +146,21 @@ is reexecuted.  Cache management parameters may be set by files under
 this directory: see the \fBdebuginfod_find_debuginfo(3)\fP man page
 for details.  The default is $HOME/.debuginfod_client_cache.
 
+.TP 21
+.B DEBUGINFOD_MAXTIME
+This environment variable dictates how long the client will wait to
+download a file found on a server in seconds. It is best used to ensure
+that a file is downloaded quickly or be rejected. The default is
+\fBLONG_MAX\fP.
+
+.TP 21
+.B DEBUGINFOD_MAXSIZE
+This environment variable dictates the maximum size of a file to
+download in bytes. This is best used if the user would like to ensure
+only small files are downloaded. A value of 0 causes no consideration
+for size, and the client may attempt to download a file of any size.
+The default is 0.
+
 .SH "FILES"
 .LP
 .PD .1v
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 3be9ee48..07ec8bbc 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,10 @@
+2021-07-26  Noah Sanci  <nsanci@redhat.com>
+
+	PR27982
+	* run-debuginfod-find.sh: Added a test to ensure that
+	DEBUGINFOD_MAXSIZE and DEBUGINFOD_MAXTIME work properly
+	by searching server and client logs for prompts.
+
 2021-07-16  Noah Sanci  <nsanci@redhat.com>
 
 	PR28034
diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
index 947c1261..12081700 100755
--- a/tests/run-debuginfod-find.sh
+++ b/tests/run-debuginfod-find.sh
@@ -32,6 +32,7 @@ VERBOSE=-vvv
 DB=${PWD}/.debuginfod_tmp.sqlite
 tempfiles $DB
 export DEBUGINFOD_CACHE_PATH=${PWD}/.client_cache
+export DEBUGINFOD_MAXSIZE=0
 
 PID1=0
 PID2=0
@@ -177,7 +178,35 @@ wait_ready $PORT1 'thread_busy{role="scan"}' 0
 testrun ${abs_builddir}/debuginfod_build_id_find -e F/p+r%o\$g 1
 
 ########################################################################
+## PR27892
+# 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 debuginfo F/p+r%o\$g.debug 2> find-vlog$PORT1 || true
+tempfiles find-vlog$PORT1
+# wait for the server to fail the same number of times the query is retried.
+wait_ready $PORT1 'http_responses_after_you_milliseconds_count{code="406"}' 1
+# quickly ensure all reporting is functional
+grep 'serving file '${PWD}'/F/p+r%o\$g.debug' vlog$PORT1
+grep 'File too large' vlog$PORT1
+grep 'using max size 1B' find-vlog$PORT1
+if [ -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID} ]; then
+  echo "File cached after maxsize and maxtime checks"
+  err
+fi
+
+# Ensure DEBUGINFOD_MAXTIME is functional
+env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS="http://127.0.0.1:$PORT1/" DEBUGINFOD_MAXTIME=0 \
+    ${abs_top_builddir}/debuginfod/debuginfod-find -v debuginfo F/p+r%o\$g.debug 2> find-vlog$PORT1 || true
+grep 'using max time 0s' find-vlog$PORT1
+grep 'Timeout with max time=0' find-vlog$PORT1
+# Ensure p+r%o\$g.debug is NOT cached
+if [ -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID} ]; then
+  echo "File cached after maxsize and maxtime checks"
+  err
+fi
 
+########################################################################
 # PR25628
 rm -rf $DEBUGINFOD_CACHE_PATH # clean it from previous tests
 
-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-04 13:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-26 18:56 [Bug debuginfod/27982] added DEBUGINFOD_MAXSIZE and DEBUGINFOD_MAXTIME Noah Sanci
2021-07-29 13:36 ` Mark Wielaard
2021-07-29 13:58   ` Frank Ch. Eigler
2021-07-29 20:29   ` Noah Sanci
2021-07-30 11:11     ` Mark Wielaard
2021-07-30 13:39       ` Noah Sanci
2021-08-04 13:39         ` Mark Wielaard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).