diff options
author | Noah Sanci <nsanci@redhat.com> | 2021-07-16 15:16:20 -0400 |
---|---|---|
committer | Mark Wielaard <mark@klomp.org> | 2021-07-22 14:10:24 +0200 |
commit | 9ab0c139eebf4ba40ac721224a673e4b66d29cd9 (patch) | |
tree | 6c77b84cf0979d04d52b0d02d5ffe54c388173ae | |
parent | 3709516ee0bdc38f736eaf6cee440c85bf6059dd (diff) | |
download | elfutils-9ab0c139eebf4ba40ac721224a673e4b66d29cd9.tar.gz |
debuginfod: PR28034 - client-side %-escape url characters
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.
In order to ensure the server properly parses urls such as this one,
%-escape characters on the client side so that the correct url
is preserved and properly processed on the server side.
https://sourceware.org/bugzilla/show_bug.cgi?id=28034
Signed-off-by: Noah Sanci <nsanci@redhat.com>
-rw-r--r-- | debuginfod/ChangeLog | 6 | ||||
-rw-r--r-- | debuginfod/debuginfod-client.c | 15 | ||||
-rw-r--r-- | doc/debuginfod.8 | 4 | ||||
-rw-r--r-- | tests/ChangeLog | 9 | ||||
-rwxr-xr-x | tests/run-debuginfod-find.sh | 32 |
5 files changed, 48 insertions, 18 deletions
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 0de17899..7709c24d 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,9 @@ +2021-07-16 Noah Sanci <nsanci@redhat.com> + + PR28034 + * debuginfod-client.c (debuginfod_query_server): % escape filename + so the completed url is processed properly. + 2021-06-28 Noah Sanci <nsanci@redhat.com> PR25978 diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index f12f609c..26ba1891 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -832,8 +832,19 @@ debuginfod_query_server (debuginfod_client *c, slashbuildid = "/buildid"; if (filename) /* must start with / */ - snprintf(data[i].url, PATH_MAX, "%s%s/%s/%s%s", server_url, - slashbuildid, build_id_bytes, type, filename); + { + /* PR28034 escape characters in completed url to %hh format. */ + char *escaped_string; + escaped_string = curl_easy_escape(data[i].handle, filename, 0); + if (!escaped_string) + { + rc = -ENOMEM; + goto out1; + } + snprintf(data[i].url, PATH_MAX, "%s%s/%s/%s%s", server_url, + slashbuildid, 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); diff --git a/doc/debuginfod.8 b/doc/debuginfod.8 index d83c7cdb..f70af625 100644 --- a/doc/debuginfod.8 +++ b/doc/debuginfod.8 @@ -309,6 +309,10 @@ l l. \../bar/foo.c AT_comp_dir=/zoo/ /buildid/BUILDID/source/zoo//../bar/foo.c .TE +Note: the client should %-escape characters in /SOURCE/FILE that are +not shown as "unreserved" in section 2.3 of RFC3986. Some characters +that will be escaped include "+", "\\", "$", "!", the 'space' character, +and ";". RFC3986 includes a more comprehensive list of these characters. .SS /metrics This endpoint returns a Prometheus formatted text/plain dump of a diff --git a/tests/ChangeLog b/tests/ChangeLog index 51ae44eb..3be9ee48 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,12 @@ +2021-07-16 Noah Sanci <nsanci@redhat.com> + + PR28034 + * run-debuginfod-find.sh: Added a test ensuring files with % + escapable characters in their paths are accessible. The test + itself is changing the name of a binary known previously as prog to + p+r%o$g. General operations such as accessing p+r%o$g acts as the + test for %-escape checking. + 2021-07-21 Noah Sanci <nsanci@redhat.com> * run-debuginfod-find.sh: Properly kill $PID4 by waiting for it to diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh index b65f3580..947c1261 100755 --- a/tests/run-debuginfod-find.sh +++ b/tests/run-debuginfod-find.sh @@ -152,18 +152,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 @@ -174,7 +174,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 ######################################################################## @@ -216,22 +216,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 ######################################################################## @@ -688,7 +688,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 |