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

* Re: [Bug debuginfod/27982] added DEBUGINFOD_MAXSIZE and DEBUGINFOD_MAXTIME
  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
  0 siblings, 2 replies; 7+ messages in thread
From: Mark Wielaard @ 2021-07-29 13:36 UTC (permalink / raw)
  To: Noah Sanci, elfutils-devel

Hi Noah,

On Mon, 2021-07-26 at 14:56 -0400, Noah Sanci via Elfutils-devel wrote:
> 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.

This looks good. Some high level comments (because I don't actually
know much about http requests).

Why have MAXTIME default to LONG_MAX? Which is long, but different on
different arches 32/64bit. If MAXSIZE == 0 means infinite, why not make
 MAXTIME=0 the same for consistency?

An alternative of passing around the X-DEBUGINFOD-MAXSIZE header is
doing it all at the client side if we supported HEAD. See PR27277. Did
you consider that route?

The bug suggests to also check the Content-Length header on reciept (in
case the server didn't support or respect the X-DEBUGINFOD-MAXSIZE
header. Did you try to implement that?

I believe various error handling goto out1 should be goto out2 (after
your duplicate urls patch). Could you double check that?

I had to make this little tweak to make the testcase pass on my setup
(because PWD contains symlinks):

diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
index feec5ddf..8ed7743d 100755
--- a/tests/run-debuginfod-find.sh
+++ b/tests/run-debuginfod-find.sh
@@ -187,7 +187,7 @@ 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 'serving file '$(realpath ${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

Cheers,

Mark

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Bug debuginfod/27982] added DEBUGINFOD_MAXSIZE and DEBUGINFOD_MAXTIME
  2021-07-29 13:36 ` Mark Wielaard
@ 2021-07-29 13:58   ` Frank Ch. Eigler
  2021-07-29 20:29   ` Noah Sanci
  1 sibling, 0 replies; 7+ messages in thread
From: Frank Ch. Eigler @ 2021-07-29 13:58 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Noah Sanci, elfutils-devel

Hi -

> [...]  An alternative of passing around the X-DEBUGINFOD-MAXSIZE
> header is doing it all at the client side if we supported HEAD. See
> PR27277. Did you consider that route? [...]

That would require -two- http round-trips (one HEAD, one actual GET).
This way, the requested limit goes out in the initial GET request.

- FChE


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Bug debuginfod/27982] added DEBUGINFOD_MAXSIZE and DEBUGINFOD_MAXTIME
  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
  1 sibling, 1 reply; 7+ messages in thread
From: Noah Sanci @ 2021-07-29 20:29 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: elfutils-devel

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

Hello,

> Why have MAXTIME default to LONG_MAX? Which is long, but different on
> different arches 32/64bit. If MAXSIZE == 0 means infinite, why not make
>  MAXTIME=0 the same for consistency?

Fixed.

> The bug suggests to also check the Content-Length header on reciept (in
> case the server didn't support or respect the X-DEBUGINFOD-MAXSIZE
> header. Did you try to implement that?

Fixed.

> I believe various error handling goto out1 should be goto out2 (after
> your duplicate urls patch). Could you double check that?

Fixed.

> diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
> index feec5ddf..8ed7743d 100755
> --- a/tests/run-debuginfod-find.sh
> +++ b/tests/run-debuginfod-find.sh
> @@ -187,7 +187,7 @@ 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 'serving file '$(realpath ${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
Fixed.

Find all the fixes in the attached patch.

Thanks,
Noah

[-- Attachment #2: 0001-debuginfod-PR27983-ignore-duplicate-urls.patch --]
[-- Type: text/x-patch, Size: 9879 bytes --]

From b9d36857b6b7cf4f2889280119ba01628afe2420 Mon Sep 17 00:00:00 2001
From: Noah Sanci <nsanci@redhat.com>
Date: Fri, 9 Jul 2021 14:53:10 -0400
Subject: [PATCH] debuginfod: PR27983 - ignore duplicate urls

Gazing at server logs, one sees a minority of clients who appear to have
duplicate query traffic coming in: the same URL, milliseconds apart.
Chances are the user accidentally doubled her $DEBUGINFOD_URLS somehow,
and the client library is dutifully asking the servers TWICE. Bug #27863
reduces the pain on the servers' CPU, but dupe network traffic is still
being paid.  We should reject sending outright duplicate concurrent
traffic.
The urls are now simply removed upon finding a duplicate after url
construction.

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

Signed-off-by: Noah Sanci <nsanci@redhat.com>
---
 debuginfod/ChangeLog           |   9 +++
 debuginfod/debuginfod-client.c | 104 +++++++++++++++++++++++----------
 tests/ChangeLog                |   6 ++
 tests/run-debuginfod-find.sh   |  11 ++++
 4 files changed, 99 insertions(+), 31 deletions(-)

diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 7709c24d..81eb56f9 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -34,6 +34,15 @@
 	(parse_opt): Handle 'r' option by setting regex_groom to true.
 	(groom): Introduce and use reg_include and reg_exclude.
 
+2021-07-09  Noah Sanci  <nsanci@redhat.com>
+
+	PR27983
+	* debuginfod-client.c (debuginfod_query_server): As full-length
+	urls are generated with standardized formats, ignore duplicates.
+	Created out1 and changed out2 error gotos. Updated url creation print
+	statements.
+	(globals): Removed url_delim_char, as it was no longer used.
+
 2021-06-18  Mark Wielaard  <mark@klomp.org>
 
 	* debuginfod-client.c (debuginfod_begin): Don't use client if
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 26ba1891..5afefbfa 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -152,7 +152,6 @@ static const char *cache_xdg_name = "debuginfod_client";
 
 /* URLs of debuginfods, separated by url_delim. */
 static const char *url_delim =  " ";
-static const char url_delim_char = ' ';
 
 /* Timeout for debuginfods, in seconds (to get at least 100K). */
 static const long default_timeout = 90;
@@ -765,13 +764,60 @@ debuginfod_query_server (debuginfod_client *c,
       goto out0;
     }
 
+  /* Initialize the memory to zero */
+  char *strtok_saveptr;
+  char **server_url_list = NULL;
+  char *server_url = strtok_r(server_urls, url_delim, &strtok_saveptr);
   /* Count number of URLs.  */
   int num_urls = 0;
-  for (int i = 0; server_urls[i] != '\0'; i++)
-    if (server_urls[i] != url_delim_char
-        && (i == 0 || server_urls[i - 1] == url_delim_char))
-      num_urls++;
-  
+
+  while (server_url != NULL)
+    {
+      /* PR 27983: If the url is already set to be used use, skip it */
+      char *slashbuildid;
+      if (strlen(server_url) > 1 && server_url[strlen(server_url)-1] == '/')
+        slashbuildid = "buildid";
+      else
+        slashbuildid = "/buildid";
+
+      char *tmp_url;
+      if (asprintf(&tmp_url, "%s%s", server_url, slashbuildid) == -1)
+        {
+          rc = -ENOMEM;
+          goto out1;
+        }
+      int url_index;
+      for (url_index = 0; url_index < num_urls; ++url_index)
+        {
+          if(strcmp(tmp_url, server_url_list[url_index]) == 0)
+            {
+              url_index = -1;
+              break;
+            }
+        }
+      if (url_index == -1)
+        {
+          if (vfd >= 0)
+            dprintf(vfd, "duplicate url: %s, skipping\n", tmp_url);
+          free(tmp_url);
+        }
+      else
+        {
+          num_urls++;
+          char ** realloc_ptr;
+          realloc_ptr = reallocarray(server_url_list, num_urls,
+                                         sizeof(char*));
+          if (realloc_ptr == NULL)
+            {
+              rc = -ENOMEM;
+              goto out1;
+            }
+          server_url_list = realloc_ptr;
+          server_url_list[num_urls-1] = tmp_url;
+        }
+      server_url = strtok_r(NULL, url_delim, &strtok_saveptr);
+    }
+
   int retry_limit = default_retry_limit;
   const char* retry_limit_envvar = getenv(DEBUGINFOD_RETRY_LIMIT_ENV_VAR);
   if (retry_limit_envvar != NULL)
@@ -804,12 +850,11 @@ debuginfod_query_server (debuginfod_client *c,
       data[i].errbuf[0] = '\0';
     }
 
-  char *strtok_saveptr;
-  char *server_url = strtok_r(server_urls, url_delim, &strtok_saveptr);
-
   /* Initialize each handle.  */
-  for (int i = 0; i < num_urls && server_url != NULL; i++)
+  for (int i = 0; i < num_urls; i++)
     {
+      if ((server_url = server_url_list[i]) == NULL)
+        break;
       if (vfd >= 0)
 	dprintf (vfd, "init server %d %s\n", i, server_url);
 
@@ -819,18 +864,10 @@ debuginfod_query_server (debuginfod_client *c,
       if (data[i].handle == NULL)
         {
           rc = -ENETUNREACH;
-          goto out1;
+          goto out2;
         }
       data[i].client = c;
 
-      /* Build handle url. Tolerate both  http://foo:999  and
-         http://foo:999/  forms */
-      char *slashbuildid;
-      if (strlen(server_url) > 1 && server_url[strlen(server_url)-1] == '/')
-        slashbuildid = "buildid";
-      else
-        slashbuildid = "/buildid";
-
       if (filename) /* must start with / */
         {
           /* PR28034 escape characters in completed url to %hh format. */
@@ -841,14 +878,12 @@ debuginfod_query_server (debuginfod_client *c,
               rc = -ENOMEM;
               goto out1;
             }
-          snprintf(data[i].url, PATH_MAX, "%s%s/%s/%s%s", server_url,
-                   slashbuildid, build_id_bytes, type, escaped_string);
+          snprintf(data[i].url, PATH_MAX, "%s/%s/%s/%s", server_url,
+                   build_id_bytes, type, escaped_string);
           curl_free(escaped_string);
         }
       else
-        snprintf(data[i].url, PATH_MAX, "%s%s/%s/%s", server_url,
-                 slashbuildid, build_id_bytes, type);
-
+        snprintf(data[i].url, PATH_MAX, "%s/%s/%s", server_url, build_id_bytes, type);
       if (vfd >= 0)
 	dprintf (vfd, "url %d %s\n", i, data[i].url);
 
@@ -883,7 +918,6 @@ debuginfod_query_server (debuginfod_client *c,
       curl_easy_setopt(data[i].handle, CURLOPT_HTTPHEADER, c->headers);
 
       curl_multi_add_handle(curlm, data[i].handle);
-      server_url = strtok_r(NULL, url_delim, &strtok_saveptr);
     }
 
   /* Query servers in parallel.  */
@@ -927,7 +961,7 @@ debuginfod_query_server (debuginfod_client *c,
             case CURLM_OUT_OF_MEMORY: rc = -ENOMEM; break;
             default: rc = -ENETUNREACH; break;
             }
-          goto out1;
+          goto out2;
         }
 
       if (c->progressfn) /* inform/check progress callback */
@@ -1115,7 +1149,7 @@ debuginfod_query_server (debuginfod_client *c,
 	    goto query_in_parallel;
 	}
       else
-	goto out1;
+	goto out2;
     }
 
   if (vfd >= 0)
@@ -1145,7 +1179,7 @@ debuginfod_query_server (debuginfod_client *c,
   if (rc < 0)
     {
       rc = -errno;
-      goto out1;
+      goto out2;
       /* Perhaps we need not give up right away; could retry or something ... */
     }
 
@@ -1155,7 +1189,10 @@ debuginfod_query_server (debuginfod_client *c,
       curl_multi_remove_handle(curlm, data[i].handle); /* ok to repeat */
       curl_easy_cleanup (data[i].handle);
     }
-
+  
+  for (int i = 0; i < num_urls; ++i)
+    free(server_url_list[i]);
+  free(server_url_list);
   free (data);
   free (server_urls);
 
@@ -1168,7 +1205,7 @@ debuginfod_query_server (debuginfod_client *c,
   goto out;
 
 /* error exits */
- out1:
+ out2:
   /* remove all handles from multi */
   for (int i = 0; i < num_urls; i++)
     {
@@ -1181,6 +1218,11 @@ debuginfod_query_server (debuginfod_client *c,
   (void) rmdir (target_cache_dir); /* nop if not empty */
   free(data);
 
+ out1:
+  for (int i = 0; i < num_urls; ++i)
+    free(server_url_list[i]);
+  free(server_url_list);
+
  out0:
   free (server_urls);
 
@@ -1213,7 +1255,7 @@ debuginfod_query_server (debuginfod_client *c,
   free (target_cache_dir);
   free (target_cache_path);
   free (target_cache_tmppath);
-  return rc;
+    return rc;
 }
 
 
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 3be9ee48..dba750e4 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -30,6 +30,12 @@
 	* run-debuginfod-find.sh: Added test case for grooming the database
 	using regexes. 
 
+2021-07-09  Noah Sanci  <nsanci@redhat.com>
+
+	PR27983
+	* run-debuginfod-find.sh: Wrote test to ensure duplicate urls are in
+	fact not checked.
+
 2021-07-08  Mark Wielaard  <mark@klomp.org>
 
 	* Makefile.am (EXTRA_DIST): Fix typo testfile-largealign.bz2 was
diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
index 947c1261..44e16242 100755
--- a/tests/run-debuginfod-find.sh
+++ b/tests/run-debuginfod-find.sh
@@ -364,6 +364,17 @@ rm -rf extracted
 
 wait_ready $PORT1 'found_sourcerefs_total{source=".rpm archive"}' $sourcefiles
 
+########################################################################
+# PR27983 ensure no duplicate urls are used in when querying servers for files
+rm -rf $DEBUGINFOD_CACHE_PATH # clean it from previous tests
+env DEBUGINFOD_URLS="http://127.0.0.1:$PORT1 http://127.0.0.1:$PORT1 http://127.0.0.1:$PORT1 http:127.0.0.1:7999" \
+ LD_LIBRARY_PATH=$ldpath ${abs_top_builddir}/debuginfod/debuginfod-find -v executable $BUILDID2 > vlog4 2>&1 || true
+tempfiles vlog4
+if [ $( grep -c 'duplicate url: http://127.0.0.1:'$PORT1'.*' vlog4 ) -ne 2 ]; then
+  echo "Duplicated servers remain";
+  err
+fi
+########################################################################
 # Run a bank of queries against the debuginfod-rpms / debuginfod-debs test cases
 
 archive_test() {
-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Bug debuginfod/27982] added DEBUGINFOD_MAXSIZE and DEBUGINFOD_MAXTIME
  2021-07-29 20:29   ` Noah Sanci
@ 2021-07-30 11:11     ` Mark Wielaard
  2021-07-30 13:39       ` Noah Sanci
  0 siblings, 1 reply; 7+ messages in thread
From: Mark Wielaard @ 2021-07-30 11:11 UTC (permalink / raw)
  To: Noah Sanci; +Cc: elfutils-devel

Hi Noah,

On Thu, 2021-07-29 at 16:29 -0400, Noah Sanci wrote:
> Why have MAXTIME default to LONG_MAX? Which is long, but different
> > on
> > different arches 32/64bit. If MAXSIZE == 0 means infinite, why not make
> >  MAXTIME=0 the same for consistency?
> 
> Fixed.
> 
> > The bug suggests to also check the Content-Length header on reciept (in
> > case the server didn't support or respect the X-DEBUGINFOD-MAXSIZE
> > header. Did you try to implement that?
> 
> Fixed.
> 
> > I believe various error handling goto out1 should be goto out2 (after
> > your duplicate urls patch). Could you double check that?
> 
> Fixed.
> 
> > diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
> > index feec5ddf..8ed7743d 100755
> > --- a/tests/run-debuginfod-find.sh
> > +++ b/tests/run-debuginfod-find.sh
> > @@ -187,7 +187,7 @@ 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 'serving file '$(realpath ${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
> 
> Fixed.

Great. Thanks. Although you don't just have to do things the way I like
them. Feel free to push back and tell me you feel the solution you
chose is better than what I am suggesting. It really is a conversation.

> Find all the fixes in the attached patch.

Looks like you attached the wrong patch (url-duplicate)

Cheers,

Mark

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Bug debuginfod/27982] added DEBUGINFOD_MAXSIZE and DEBUGINFOD_MAXTIME
  2021-07-30 11:11     ` Mark Wielaard
@ 2021-07-30 13:39       ` Noah Sanci
  2021-08-04 13:39         ` Mark Wielaard
  0 siblings, 1 reply; 7+ messages in thread
From: Noah Sanci @ 2021-07-30 13:39 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: elfutils-devel

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

Hello,

Here is the real patch :).

Thanks,
Noah

On Fri, Jul 30, 2021 at 7:11 AM Mark Wielaard <mark@klomp.org> wrote:
>
> Hi Noah,
>
> On Thu, 2021-07-29 at 16:29 -0400, Noah Sanci wrote:
> > Why have MAXTIME default to LONG_MAX? Which is long, but different
> > > on
> > > different arches 32/64bit. If MAXSIZE == 0 means infinite, why not make
> > >  MAXTIME=0 the same for consistency?
> >
> > Fixed.
> >
> > > The bug suggests to also check the Content-Length header on reciept (in
> > > case the server didn't support or respect the X-DEBUGINFOD-MAXSIZE
> > > header. Did you try to implement that?
> >
> > Fixed.
> >
> > > I believe various error handling goto out1 should be goto out2 (after
> > > your duplicate urls patch). Could you double check that?
> >
> > Fixed.
> >
> > > diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
> > > index feec5ddf..8ed7743d 100755
> > > --- a/tests/run-debuginfod-find.sh
> > > +++ b/tests/run-debuginfod-find.sh
> > > @@ -187,7 +187,7 @@ 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 'serving file '$(realpath ${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
> >
> > Fixed.
>
> Great. Thanks. Although you don't just have to do things the way I like
> them. Feel free to push back and tell me you feel the solution you
> chose is better than what I am suggesting. It really is a conversation.
>
> > Find all the fixes in the attached patch.
>
> Looks like you attached the wrong patch (url-duplicate)
>
> Cheers,
>
> Mark
>

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

From b7e28a6a1c5e2664cdaeff7e7f5ac39aebfc12d0 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. The client side then checks to ensure this maxsize
has been respected.

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

Signed-off-by: Noah Sanci <nsanci@redhat.com>
---
 debuginfod/ChangeLog           | 12 +++++
 debuginfod/debuginfod-client.c | 99 ++++++++++++++++++++++++++++++++--
 debuginfod/debuginfod.cxx      | 13 +++++
 debuginfod/debuginfod.h.in     |  2 +
 doc/ChangeLog                  |  6 +++
 doc/debuginfod-find.1          | 15 ++++++
 tests/ChangeLog                |  7 +++
 tests/run-debuginfod-find.sh   | 26 +++++++++
 8 files changed, 177 insertions(+), 3 deletions(-)

diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 81eb56f9..9e82d78d 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,15 @@
+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.cxx (handler_cb): If the requested file exceeds
+	maxsize return code 406.
+	* 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 a70f6d79..7d4b220f 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -555,6 +555,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 = 0;
+  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 = 0;
+  const char *maxtime_envvar;
+  maxtime_envvar = getenv(DEBUGINFOD_MAXTIME_ENV_VAR);
+  if (maxtime_envvar != NULL)
+    maxtime = atol (maxtime_envvar);
+  if (maxtime && vfd >= 0)
+    dprintf(vfd, "using max time %lds\n", maxtime);
+
+  /* Maxsize is valid*/
+  if (maxsize > 0)
+    {
+      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.  */
@@ -833,7 +866,7 @@ debuginfod_query_server (debuginfod_client *c,
   if (data == NULL)
     {
       rc = -ENOMEM;
-      goto out0;
+      goto out1;
     }
 
   /* thereafter, goto out1 on error.  */
@@ -864,7 +897,7 @@ debuginfod_query_server (debuginfod_client *c,
       if (data[i].handle == NULL)
         {
           rc = -ENETUNREACH;
-          goto out2;
+          goto out1;
         }
       data[i].client = c;
 
@@ -876,7 +909,7 @@ debuginfod_query_server (debuginfod_client *c,
           if (!escaped_string)
             {
               rc = -ENOMEM;
-              goto out1;
+              goto out2;
             }
           snprintf(data[i].url, PATH_MAX, "%s/%s/%s/%s", server_url,
                    build_id_bytes, type, escaped_string);
@@ -927,8 +960,31 @@ debuginfod_query_server (debuginfod_client *c,
   long loops = 0;
   int committed_to = -1;
   bool verbose_reported = false;
+  struct timespec start_time, cur_time;
+  if ( maxtime > 0 && clock_gettime(CLOCK_MONOTONIC_RAW, &start_time) == -1)
+    {
+      rc = errno;
+      goto out2;
+    }
+  long delta = 0;
   do
     {
+      /* Check to see how long querying is taking. */
+      if (maxtime > 0)
+        {
+          if (clock_gettime(CLOCK_MONOTONIC_RAW, &cur_time) == -1)
+            {
+              rc = errno;
+              goto out2;
+            }
+          delta = cur_time.tv_sec - start_time.tv_sec;
+          if ( delta >  maxtime)
+            {
+              dprintf(vfd, "Timeout with max time=%lds and transfer time=%lds\n", maxtime, delta );
+              rc = -ETIME;
+              goto out2;
+            }
+        }
       /* Wait 1 second, the minimum DEBUGINFOD_TIMEOUT.  */
       curl_multi_wait(curlm, NULL, 0, 1000, NULL);
 
@@ -1010,6 +1066,29 @@ debuginfod_query_server (debuginfod_client *c,
           if ((*c->progressfn) (c, pa, pb))
             break;
         }
+      /* Check to see if we are downloading something which exceeds maxsize, if set.*/
+      if (maxsize > 0 && target_handle)
+        {
+          long dl_size = 0;
+#ifdef CURLINFO_SIZE_DOWNLOAD_T
+          curl_off_t download_size_t;
+          if (curl_easy_getinfo(target_handle, CURLINFO_CONTENT_LENGTH_DOWNLOAD_T,
+                                                    &download_size_t) == CURLE_OK)
+            dl_size = download_size_t >= (double)(LONG_MAX+1UL) ? LONG_MAX : (long)download_size_t;
+#else
+          double download_size;
+          if (curl_easy_getinfo(target_handle, CURLINFO_CONTENT_LENGTH_DOWNLOAD,
+                                                    &download_size) == CURLE_OK)
+            dl_size = download_size >= (double)(LONG_MAX+1UL) ? LONG_MAX : (long)download_size;
+#endif
+            if (dl_size > maxsize)
+              {
+                if (vfd >=0)
+                  dprintf(vfd, "Content-Length too large.\n");
+                rc = -EFBIG;
+                goto out2;
+              }
+        }
     } while (still_running);
 
   /* Check whether a query was successful. If so, assign its handle
@@ -1043,6 +1122,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)
                 {
@@ -1057,6 +1138,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;
                 }
             }
@@ -1129,6 +1220,8 @@ debuginfod_query_server (debuginfod_client *c,
       if (efd >= 0)
         close(efd);
     }
+  else if (rc == -EFBIG)
+    goto out2;
 
   /* 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..482a8ae7 100644
--- a/doc/debuginfod-find.1
+++ b/doc/debuginfod-find.1
@@ -147,6 +147,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
+0 (infinite time).
+
+.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 (infinite size).
+
 .SH "FILES"
 .LP
 .PD .1v
diff --git a/tests/ChangeLog b/tests/ChangeLog
index dba750e4..34666609 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 44e16242..991d1dc5 100755
--- a/tests/run-debuginfod-find.sh
+++ b/tests/run-debuginfod-find.sh
@@ -177,7 +177,33 @@ 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
+# ensure all reporting is functional
+grep 'serving file '$(realpath ${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 check"
+  err
+fi
 
+# Ensure DEBUGINFOD_MAXTIME is functional
+env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS="http://127.0.0.1:8002/" DEBUGINFOD_MAXTIME=1 \
+    ${abs_top_builddir}/debuginfod/debuginfod-find -v debuginfo F/p+r%o\$g.debug 2> find-vlog$PORT1 || true
+grep 'using max time' find-vlog$PORT1
+# Ensure p+r%o\$g.debug is NOT cached
+if [ -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID} ]; then
+  echo "File cached after maxtime check"
+  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

* Re: [Bug debuginfod/27982] added DEBUGINFOD_MAXSIZE and DEBUGINFOD_MAXTIME
  2021-07-30 13:39       ` Noah Sanci
@ 2021-08-04 13:39         ` Mark Wielaard
  0 siblings, 0 replies; 7+ messages in thread
From: Mark Wielaard @ 2021-08-04 13:39 UTC (permalink / raw)
  To: Noah Sanci; +Cc: elfutils-devel

On Fri, 2021-07-30 at 09:39 -0400, Noah Sanci wrote:
> Here is the real patch :).

Looks good. Pushed.

Thanks,

Mark

^ 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).