From patchwork Thu Jul 29 13:36:00 2021
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Mark Wielaard <mark@klomp.org>
X-Patchwork-Id: 45842
From: mark@klomp.org (Mark Wielaard)
Date: Thu, 29 Jul 2021 15:36:00 +0200
Subject: [Bug debuginfod/27982] added DEBUGINFOD_MAXSIZE and
 DEBUGINFOD_MAXTIME
In-Reply-To: 
 <CAJXA7qjTnL8dwzTZ9p6=+wfNS6mbASngb1jkKK3G3A+k_y__Ng@mail.gmail.com>
References: 
 <CAJXA7qjTnL8dwzTZ9p6=+wfNS6mbASngb1jkKK3G3A+k_y__Ng@mail.gmail.com>
Message-ID: <34693753bef32fe1583538ca8ca7abe42f7d49a1.camel@klomp.org>

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):


Cheers,

Mark

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