[Bug debuginfod/28034] %-escape url characters

[Noah Sanci]

Hello,

> On Mon, Jul 19, 2021 at 10:53:17AM -0400, Noah Sanci via Elfutils-devel wrote:
> > When requesting some source files, some URL-inconvenient chars
> > sometimes pop up.  Example from f33 libstdc++:
> > /buildid/44d8485cb75512c2ca5c8f70afbd475cae30af4f/source/usr/src/debug/
> > gcc-10.3.1-1.fc33.x86_64/obj-x86_64-redhat-linux/x86_64-redhat-linux/
> > libstdc++-v3/src/c++11/../../../../../libstdc++-v3/src/c++11/
> > condition_variable.cc
> > As this URL is passed into debuginfod's handler_cb, it appears that the
> > + signs are helpfully unescaped to spaces by libmicrohttpd, which
> > 'course breaks everything.  We need to suppress this HTTP URL
> > processing step.  Also worth checking that %HEX decoding is also
> > suppressed.
>
> I find this description slightly confusing. It ends with "Also worth
> checking..." but that is actually what is done in this patch. The part
> before that about what debuginfod/microhttpd does on the server side
> is interesting, but not really what this patch is about (just why this
> patch is necessary, but it seems necessary on the client side always
> whatever server is used).

Thank you for bringing this to my attention. Fixed

> curl_easy_escape () can return NULL on failure.
>

Fixed.

> > +Note: the client should %-escape characters in /SOURCE/FILE that are not shown as "unreserved" in section 2.3 of RFC3986.

Please read the new note just to ensure that it's reasonable, as my
updated note specifies some characters that will be percent escaped.
Otherwise fixed.

> > @@ -149,18 +150,18 @@ ps -q $PID1 -e -L -o '%p %c %a' | grep traverse
> >  # Compile a simple program, strip its debuginfo and save the build-id.
> >  # Also move the debuginfo into another directory so that elfutils
> >  # cannot find it without debuginfod.
> > -echo "int main() { return 0; }" > ${PWD}/prog.c
> > -tempfiles prog.c
> > +echo "int main() { return 0; }" > ${PWD}/p+r%o\$g.c
> > +tempfiles p+r%o\$g.c
> >  # Create a subdirectory to confound source path names
> >  mkdir foobar
> > -gcc -Wl,--build-id -g -o prog ${PWD}/foobar///./../prog.c
> > -testrun ${abs_top_builddir}/src/strip -g -f prog.debug ${PWD}/prog
> > +gcc -Wl,--build-id -g -o p+r%o\$g ${PWD}/foobar///./../p+r%o\$g.c
> > +testrun ${abs_top_builddir}/src/strip -g -f p+r%o\$g.debug ${PWD}/p+r%o\$g
> >  BUILDID=`env LD_LIBRARY_PATH=$ldpath ${abs_builddir}/../src/readelf \
> > -          -a prog | grep 'Build ID' | cut -d ' ' -f 7`
> > +          -a p+r%o\\$g | grep 'Build ID' | cut -d ' ' -f 7`
> >
> >  wait_ready $PORT1 'thread_work_total{role="traverse"}' 1
> > -mv prog F
> > -mv prog.debug F
> > +mv p+r%o\$g F
> > +mv p+r%o\$g.debug F
> >  kill -USR1 $PID1
> >  # Wait till both files are in the index.
> >  wait_ready $PORT1 'thread_work_total{role="traverse"}' 2
> > @@ -171,7 +172,7 @@ wait_ready $PORT1 'thread_busy{role="scan"}' 0
> >
> >  # Test whether elfutils, via the debuginfod client library dlopen hooks,
> >  # is able to fetch debuginfo from the local debuginfod.
> > -testrun ${abs_builddir}/debuginfod_build_id_find -e F/prog 1
> > +testrun ${abs_builddir}/debuginfod_build_id_find -e F/p+r%o\$g 1
> >
> >  ########################################################################
> >
> > @@ -213,22 +214,22 @@ fi
> >  # Test whether debuginfod-find is able to fetch those files.
> >  rm -rf $DEBUGINFOD_CACHE_PATH # clean it from previous tests
> >  filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID`
> > -cmp $filename F/prog.debug
> > +cmp $filename F/p+r%o\$g.debug
> >  if [ -w $filename ]; then
> >      echo "cache file writable, boo"
> >      err
> >  fi
> >
> > -filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find executable F/prog`
> > -cmp $filename F/prog
> > +filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find executable F/p+r%o\\$g`
> > +cmp $filename F/p+r%o\$g
> >
> >  # raw source filename
> > -filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find source $BUILDID ${PWD}/foobar///./../prog.c`
> > -cmp $filename  ${PWD}/prog.c
> > +filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find source $BUILDID ${PWD}/foobar///./../p+r%o\\$g.c`
> > +cmp $filename  ${PWD}/p+r%o\$g.c
> >
> >  # and also the canonicalized one
> > -filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find source $BUILDID ${PWD}/prog.c`
> > -cmp $filename  ${PWD}/prog.c
> > +filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find source $BUILDID ${PWD}/p+r%o\\$g.c`
> > +cmp $filename  ${PWD}/p+r%o\$g.c
> >
> >
> >  ########################################################################
> > @@ -672,7 +673,7 @@ touch -d '1970-01-01' $DEBUGINFOD_CACHE_PATH/00000000 # old enough to guarantee
> >  echo 0 > $DEBUGINFOD_CACHE_PATH/cache_clean_interval_s
> >  echo 0 > $DEBUGINFOD_CACHE_PATH/max_unused_age_s
> >
> > -testrun ${abs_builddir}/debuginfod_build_id_find -e F/prog 1
> > +testrun ${abs_builddir}/debuginfod_build_id_find -e F/p+r%o\$g 1
> >
> >  testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID2 && false || true
> >
> > @@ -725,6 +726,7 @@ PID4=$!
> >  wait_ready $PORT3 'ready' 1
> >  tempfiles vlog$PORT3
> >  errfiles vlog$PORT3
> > +tempfiles $DB.backup*
>
> ???
>

Hopefully the updated changelog message clarifies these changes.

> >  kill -USR2 $PID4
> >  wait_ready $PORT3 'thread_work_total{role="groom"}' 1
> > @@ -738,6 +740,7 @@ wait_ready $PORT3 'groom{statistic="files scanned (#)"}' 0
> >  wait_ready $PORT3 'groom{statistic="files scanned (mb)"}' 0
> >
> >  kill $PID4
> > +PID4=0
>
> And this?
> Might need to be
>
>   wait $PID
>   PID4=0

The small test suite fixes have been removed from the script, however
I would like to update the test suite ASAP. Can I just commit a new
branch titled something like 'elfutils-test-fixes' without a specific
pr?

Noah
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-debuginfod-PR28034-client-side-escape-url-characters.patch
Type: text/x-patch
Size: 7532 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/elfutils-devel/attachments/20210721/6ed717a9/attachment.bin>