elfutils 0.179 release end of the week?
Hi, It has been almost 4 months since 0.178. There have been more than 60 commits. Various important bug fixes and some nice new features for debuginfod. We seem to be in good shape. So I propose we'll do a new release by the end of the week. If there are specific issues/bugs you want to work on/get in before we do a release please let me know. I think it would be good if we could figure out whether this is possible to support: https://sourceware.org/bugzilla/show_bug.cgi?id=25548 also support canonicalized source-file name lookups in webapi The problem is that it isn't entirely clear we can support "canonicalization" in a way that doesn't rely on the file system. But maybe we can just say that we support the most basic form of "resolving" '/./', 'foobar/../' and '//' as in the original description . The issue with the missing CURLOPT_PATH_AS_IS from comment #4 probably should be resolved also. Are there any other "urgent" bug fixes pending? Cheers, Mark
Re: elfutils 0.179 release end of the week?
Hi - > [...] > I think it would be good if we could figure out whether this is > possible to support: > https://sourceware.org/bugzilla/show_bug.cgi?id=25548 > also support canonicalized source-file name lookups in webapi Yes, posted a draft patch to a git branch last night, as per irc: elfutils: fche elfutils.git:fche/pr25548 * elfutils-0.178-59-g0db03a05 / debuginfod/debuginfod-client.c debuginfod/debuginfod.cxx tests/run-debuginfod-find.sh: snap > Are there any other "urgent" bug fixes pending? Yeah, a few more debuginfod bits please. I'll work all out today and tomorrow on as many as possible of PR25548, PR25367, PR25366, PR25448, PR25583. - FChE
Re: elfutils 0.179 release end of the week?
On 24/03/20 12:37, Mark Wielaard wrote: > Hi, > > It has been almost 4 months since 0.178. There have been more than 60 > commits. Various important bug fixes and some nice new features for > debuginfod. We seem to be in good shape. So I propose we'll do a new > release by the end of the week. If there are specific issues/bugs you > want to work on/get in before we do a release please let me know. > > I think it would be good if we could figure out whether this is > possible to support: > > https://sourceware.org/bugzilla/show_bug.cgi?id=25548 > also support canonicalized source-file name lookups in webapi > > The problem is that it isn't entirely clear we can support > "canonicalization" in a way that doesn't rely on the file system. But > maybe we can just say that we support the most basic form of > "resolving" '/./', 'foobar/../' and '//' as in the original description > . The issue with the missing CURLOPT_PATH_AS_IS from comment #4 > probably should be resolved also. > > Are there any other "urgent" bug fixes pending? > > Cheers, > > Mark Has there been any movement on PR21002 recently? Last comment is from 2 years ago (2018-03-11). Gentoo Linux has bugs https://bugs.gentoo.org/602126 and https://bugs.gentoo.org/701478 , and I don't doubt other distributions that use musl-libc (eg. Void linux, Alpine, Adelie) will also be patching this for their use. Best regards, Michael Everitt. veremitz / Freenode, OFTC. signature.asc Description: OpenPGP digital signature
Re: PR25369 rfc slice 2: debuginfod get_url
Hi - > > Sorry I just hate to spam people with more email, when the information > > strikes me as such small value beyond the contents of the git repo. Are > > there folks who read only the mailing list but don't git update? > > Personally I like more email. Especially if it shows which code got > pushed and why. Then I also know whether I can expect some patch to > show up when I do a git pull and whether it is (subtly) different from > what was posted to the list earlier. OK, will do. - FChE
obv patch: debuginfod-find
Hi - Committing as obvious. diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 2410430c2361..00e7ec63232e 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,8 @@ +2020-03-24 Frank Ch. Eigler + + * debuginfod-find.c (main): Correct /source full-pathness check for + "debuginfod-find -v source deadbeef /pathname" case. + 2020-03-22 Frank Ch. Eigler * debuginfod-client.c (struct debuginfod_client): Add url field. diff --git a/debuginfod/debuginfod-find.c b/debuginfod/debuginfod-find.c index 787779c76a47..400c268224fb 100644 --- a/debuginfod/debuginfod-find.c +++ b/debuginfod/debuginfod-find.c @@ -1,6 +1,6 @@ /* Command-line frontend for retrieving ELF / DWARF / source files from the debuginfod. - Copyright (C) 2019 Red Hat, Inc. + Copyright (C) 2019-2020 Red Hat, Inc. This file is part of elfutils. This file is free software; you can redistribute it and/or modify @@ -121,7 +121,7 @@ main(int argc, char** argv) &cache_name); else if (strcmp(argv[remaining], "source") == 0) { - if (remaining+2 == argc || argv[3][0] != '/') + if (remaining+2 == argc || argv[remaining+2][0] != '/') { fprintf(stderr, "If FILETYPE is \"source\" then absolute /FILENAME must be given\n"); return 1;
patch PR25548: debuginfod canonicalized source paths
Hi - The following patch generalizes the webapi slightly, which allows debuggers like lldb to operate against packages/binaries with source files that include posix pathname noise. commit 8ac3883f36c3c3d48bc90891aad6718a798bdd7a (HEAD -> fche/pr25548) Author: Frank Ch. Eigler Date: Mon Mar 23 15:33:56 2020 -0400 PR25548: support canonicalized source-path names in debuginfod webapi Programs are sometimes compiled with source path names containing noise like /./ or // or /foo/../, and these survive into DWARF. This code allows either raw or canonicalized pathnames in the webapi, by letting the client pass things verbatim, and letting the server store/accept both raw and canonicalized path names for source files. Tests included & docs updated. Signed-off-by: Frank Ch. Eigler diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 2410430c2361..4d687e75bdf8 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,14 @@ +2020-03-24 Frank Ch. Eigler + + * debuginfod-client.c (debuginfod_query_server): Set + CURLOPT_PATH_AS_IS, to propagate file names verbatim. + * debuginfod.cxx (canon_pathname): Implement RFC3986 + style pathname canonicalization. + (handle_buildid): Canonicalize incoming webapi source + paths, accept either one. + (scan_source_file, archive_classify): Store both + original and canonicalized dwarf-source file names. + 2020-03-22 Frank Ch. Eigler * debuginfod-client.c (struct debuginfod_client): Add url field. diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index 58a04b9a734b..3d6f645d56f2 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -681,6 +681,7 @@ debuginfod_query_server (debuginfod_client *c, curl_easy_setopt(data[i].handle, CURLOPT_FOLLOWLOCATION, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_FAILONERROR, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_NOSIGNAL, (long) 1); + curl_easy_setopt(data[i].handle, CURLOPT_PATH_AS_IS, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_AUTOREFERER, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_ACCEPT_ENCODING, ""); add_extra_headers(data[i].handle); diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx index 7c7e85eb6d14..dc62eb088c02 100644 --- a/debuginfod/debuginfod.cxx +++ b/debuginfod/debuginfod.cxx @@ -944,6 +944,82 @@ shell_escape(const string& str) } +// PR25548: Perform POSIX / RFC3986 style path canonicalization on the input string. +// +// Namely: +//// -> / +///foo/../ -> / +///./-> / +// +// This mapping is done on dwarf-side source path names, which may +// include these constructs, so we can deal with debuginfod clients +// that accidentally canonicalize the paths. +// +// realpath(3) is close but not quite right, because it also resolves +// symbolic links. Symlinks at the debuginfod server have nothing to +// do with the build-time symlinks, thus they must not be considered. +// +// see also curl Curl_dedotdotify() aka RFC3986, which we mostly follow here +// see also libc __realpath() +// see also llvm llvm::sys::path::remove_dots() +static string +canon_pathname (const string& input) +{ + string i = input; // 5.2.4 (1) + string o; + + while (i.size() != 0) +{ + // 5.2.4 (2) A + if (i.substr(0,3) == "../") +i = i.substr(3); + else if(i.substr(0,2) == "./") +i = i.substr(2); + + // 5.2.4 (2) B + else if (i.substr(0,3) == "/./") +i = i.substr(2); + else if (i == "/.") +i = ""; // no need to handle "/." complete-path-segment case; we're dealing with file names + + // 5.2.4 (2) C + else if (i.substr(0,4) == "/../") { +i = i.substr(3); +string::size_type sl = o.rfind("/"); +if (sl != string::npos) + o = o.substr(0, sl); +else + o = ""; + } else if (i == "/..") +i = ""; // no need to handle "/.." complete-path-segment case; we're dealing with file names + + // 5.2.4 (2) D + // no need to handle these cases; we're dealing with file names + else if (i == ".") +i = ""; + else if (i == "..") +i = ""; + + // POSIX special: map // to / + else if (i.substr(0,2) == "//") +i = i.substr(1); + + // 5.2.4 (2) E + else { +string::size_type next_slash = i.find("/", (i[0]=='/' ? 1 : 0)); // skip first slash +o += i.substr(0, next_slash); +if (next_slash == string::npos) + i = ""; +else + i = i.substr(next_slash); + } +} + + return o; +} + + + // A map-like class that owns a cache of file descriptors (indexed by // file / content names). // @@ -1393,12 +1469,17 @@ static struct MHD_Response* handle_buildid (const string& buildid /* unsafe */, } else if (atyp
patch obv: debuginfod-client default_progressfn message
Hi - Correcting a thinko from earlier, which became evident with (forthcoming) server-side diagnostic improvements. Merging as obvious. The patch is more than one line long only because a function body was moved upward, to avoid having to do a forward declaration. Author: Frank Ch. Eigler Date: Tue Mar 24 21:30:02 2020 -0400 debuginfod-client thinko: non-default progressfn extra output A previous commit changed the default_progressfn output format to \rFOOBAR, to be terminated by an \n when the download finished. The \n terminator was conditional on the wrong thing (env var setting, rather than actual progressfn setting), so the \n could be printed even if an app overrode the default. diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 00e7ec63232e..b34b4d2938dd 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,9 @@ +2020-03-24 Frank Ch. Eigler + + * debuginfod-client.c (debuginfod_query_server): Print the + default_progressfn terminating \n message only if that progressfn + is actually set. + 2020-03-24 Frank Ch. Eigler * debuginfod-find.c (main): Correct /source full-pathness check for diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index 58a04b9a734b..ea2d16249a31 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -401,6 +401,41 @@ add_extra_headers(CURL *handle) } \ } while (0) + +/* Offer a basic form of progress tracing */ +static int +default_progressfn (debuginfod_client *c, long a, long b) +{ + const char* url = debuginfod_get_url (c); + int len = 0; + + /* We prefer to print the host part of the URL to keep the + message short. */ + if (url != NULL) +{ + const char* buildid = strstr(url, "buildid/"); + if (buildid != NULL) +len = (buildid - url); + else +len = strlen(url); +} + + if (b == 0 || url==NULL) /* early stage */ +dprintf(STDERR_FILENO, +"\rDownloading %c", "-/|\\"[a % 4]); + else if (b < 0) /* download in progress but unknown total length */ +dprintf(STDERR_FILENO, +"\rDownloading from %.*s %ld", +len, url, a); + else /* download in progress, and known total length */ +dprintf(STDERR_FILENO, +"\rDownloading from %.*s %ld/%ld", +len, url, a, b); + + return 0; +} + + /* Query each of the server URLs found in $DEBUGINFOD_URLS for the file with the specified build-id, type (debuginfo, executable or source) and filename. filename may be NULL. If found, return a file @@ -885,7 +920,7 @@ debuginfod_query_server (debuginfod_client *c, /* general purpose exit */ out: /* Conclude the last \r status line */ - if (getenv(DEBUGINFOD_PROGRESS_ENV_VAR)) + if (c->progressfn == & default_progressfn) dprintf(STDERR_FILENO, "\n"); free (cache_path); @@ -898,39 +933,6 @@ debuginfod_query_server (debuginfod_client *c, } -/* Activate a basic form of progress tracing */ -static int -default_progressfn (debuginfod_client *c, long a, long b) -{ - const char* url = debuginfod_get_url (c); - int len = 0; - - /* We prefer to print the host part of the URL to keep the - message short. */ - if (url != NULL) -{ - const char* buildid = strstr(url, "buildid/"); - if (buildid != NULL) -len = (buildid - url); - else -len = strlen(url); -} - - if (b == 0 || url==NULL) /* early stage */ -dprintf(STDERR_FILENO, -"\rDownloading %c", "-/|\\"[a % 4]); - else if (b < 0) /* download in progress but unknown total length */ -dprintf(STDERR_FILENO, -"\rDownloading from %.*s %ld", -len, url, a); - else /* download in progress, and known total length */ -dprintf(STDERR_FILENO, -"\rDownloading from %.*s %ld/%ld", -len, url, a, b); - - return 0; -} - /* See debuginfod.h */ debuginfod_client *
[Bug debuginfod/25367] web request status logging improvements
https://sourceware.org/bugzilla/show_bug.cgi?id=25367 --- Comment #1 from Frank Ch. Eigler --- https://sourceware.org/pipermail/elfutils-devel/2020q1/002524.html -- You are receiving this mail because: You are on the CC list for the bug.
PR25267: debuginfod status logging improvements
Hi - This makes debuginfod logs more useful to admins. Author: Frank Ch. Eigler Date: Tue Mar 24 21:57:59 2020 -0400 PR25367: improve debuginfod webapi logging Improve debuginfod logging to show webapi query results including http status, sizes, and processing times. Signed-off-by: Frank Ch. Eigler diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index b34b4d2938dd..9eb06719ec0d 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,12 @@ +2020-03-24 Frank Ch. Eigler + + * debuginfod.cxx (handle_buildid): In case of federated fallback + queries, handle errors analogously to local ENOENT/404. + (handle_metrics): Return a size-of-response value. + (handler_cb): Add code to time entire application-side processing + stage + response sizes + http codes, so as to emit a complete + httpd-flavoured log line for each webapi request. + 2020-03-24 Frank Ch. Eigler * debuginfod-client.c (debuginfod_query_server): Print the diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx index 7c7e85eb6d14..490169a40ded 100644 --- a/debuginfod/debuginfod.cxx +++ b/debuginfod/debuginfod.cxx @@ -853,6 +853,7 @@ conninfo (struct MHD_Connection * conn) + static void add_mhd_last_modified (struct MHD_Response *resp, time_t mtime) { @@ -1473,8 +1474,16 @@ static struct MHD_Response* handle_buildid (const string& buildid /* unsafe */, } close (fd); } - else if (fd != -ENOSYS) // no DEBUGINFOD_URLS configured -throw libc_exception(-fd, "upstream debuginfod query failed"); + else +switch(fd) + { + case -ENOSYS: +break; + case -ENOENT: +break; + default: // some more tricky error +throw libc_exception(-fd, "upstream debuginfod query failed"); + } throw reportable_exception(MHD_HTTP_NOT_FOUND, "not found"); } @@ -1564,7 +1573,7 @@ add_metric(const string& metric, static struct MHD_Response* -handle_metrics () +handle_metrics (off_t* size) { stringstream o; { @@ -1576,6 +1585,7 @@ handle_metrics () MHD_Response* r = MHD_create_response_from_buffer (os.size(), (void*) os.c_str(), MHD_RESPMEM_MUST_COPY); + *size = os.size(); MHD_add_response_header (r, "Content-Type", "text/plain"); return r; } @@ -1598,8 +1608,11 @@ handler_cb (void * /*cls*/, struct MHD_Response *r = NULL; string url_copy = url; - if (verbose) -obatched(clog) << conninfo(connection) << " " << method << " " << url << endl; + int rc = MHD_NO; // mhd + int http_code = 500; + off_t http_size = -1; + struct timeval tv_start, tv_end; + gettimeofday (&tv_start, NULL); try { @@ -1632,12 +1645,22 @@ handler_cb (void * /*cls*/, } inc_metric("http_requests_total", "type", artifacttype); - r = handle_buildid(buildid, artifacttype, suffix, 0); // NB: don't care about result-fd + + // get the resulting fd so we can report its size + int fd; + r = handle_buildid(buildid, artifacttype, suffix, &fd); + if (r) +{ + struct stat fs; + if (fstat(fd, &fs) == 0) +http_size = fs.st_size; + // libmicrohttpd will close (fd); +} } else if (url1 == "/metrics") { inc_metric("http_requests_total", "type", "metrics"); - r = handle_metrics(); + r = handle_metrics(& http_size); } else throw reportable_exception("webapi error, unrecognized /operation"); @@ -1645,16 +1668,27 @@ handler_cb (void * /*cls*/, if (r == 0) throw reportable_exception("internal error, missing response"); - int rc = MHD_queue_response (connection, MHD_HTTP_OK, r); + rc = MHD_queue_response (connection, MHD_HTTP_OK, r); + http_code = MHD_HTTP_OK; MHD_destroy_response (r); - return rc; } catch (const reportable_exception& e) { inc_metric("http_responses_total","result","error"); e.report(clog); - return e.mhd_send_response (connection); + http_code = e.code; + rc = e.mhd_send_response (connection); } + + gettimeofday (&tv_end, NULL); + double deltas = (tv_end.tv_sec - tv_start.tv_sec) + (tv_end.tv_usec - tv_start.tv_usec)*0.01; + obatched(clog) << conninfo(connection) + << ' ' << method << ' ' << url + << ' ' << http_code << ' ' << http_size + << ' ' << (int)(deltas*1000) << "ms" + << endl; + + return rc; } diff --git a/tests/ChangeLog b/tests/ChangeLog index d0d32a87315a..7c1c307a532f 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,8 @@ +2020-03-24 Frank Ch. Eigler + + * run-
[Bug debuginfod/25548] also support canonicalized source-file name lookups in webapi
https://sourceware.org/bugzilla/show_bug.cgi?id=25548 --- Comment #5 from Frank Ch. Eigler --- https://sourceware.org/pipermail/elfutils-devel/2020q1/002522.html -- You are receiving this mail because: You are on the CC list for the bug.
PR25369 slice 3/3: debuginfod header relay
Hi - This is the last of three bits for the month-ago PR25369 patchset. Author: Frank Ch. Eigler Date: Tue Mar 24 23:46:30 2020 -0400 debuginfod: User-Agent and X-Forwarded-For header relay Extend the debuginfod client API with a function to stuff outgoing headers into libcurl. Use this from debuginfod so federated trees of debuginfod/httpd can trace back to to the originating client for administrative purposes. Docs & test included. Signed-off-by: Frank Ch. Eigler diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 00e7ec63232e..44c1349bfd80 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,15 @@ +2020-03-24 Frank Ch. Eigler + + * debuginfod.h, libdebuginfod.map: New functions for _add_url_header. + * debuginfod-client.c (struct debuginfod_client): Add headers fields. + (debuginfod_add_http_header): New client api to add outgoing headers. + (add_default_headers): Renamed from add_extra_headers, skip if flag. + (debuginfod_query_server): Pass accumulated headers to libcurl. + (debuginfod_end): Clean accumulated headers. + (debuginfod_find_*): Add default headers at this point. + * debuginfod.cxx (handle_buildid): Add conn pointer. Use it to relay + incoming UA and XFF headers to federated upstream debuginfods. + 2020-03-24 Frank Ch. Eigler * debuginfod-find.c (main): Correct /source full-pathness check for diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index 58a04b9a734b..6517cb72432f 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -85,6 +85,10 @@ struct debuginfod_client /* Stores current/last url, if any. */ char* url; + /* Accumulates outgoing http header names/values. */ + int user_agent_set_p; /* affects add_default_headers */ + struct curl_slist *headers; + /* Can contain all other context, like cache_path, server_urls, timeout or other info gotten from environment variables, the handle data, etc. So those don't have to be reparsed and @@ -311,8 +315,11 @@ debuginfod_clean_cache(debuginfod_client *c, static void -add_extra_headers(CURL *handle) +add_default_headers(debuginfod_client *client) { + if (client->user_agent_set_p) +return; + /* Compute a User-Agent: string to send. The more accurately this describes this host, the likelier that the debuginfod servers might be able to locate debuginfo for us. */ @@ -372,7 +379,7 @@ add_extra_headers(CURL *handle) } char *ua = NULL; - rc = asprintf(& ua, "%s/%s,%s,%s/%s", + rc = asprintf(& ua, "User-Agent: %s/%s,%s,%s/%s", PACKAGE_NAME, PACKAGE_VERSION, utspart ?: "", id ?: "", @@ -381,7 +388,7 @@ add_extra_headers(CURL *handle) ua = NULL; if (ua) -curl_easy_setopt(handle, CURLOPT_USERAGENT, (void*) ua); /* implicit strdup */ +(void) debuginfod_add_http_header (client, ua); free (ua); free (id); @@ -683,7 +690,7 @@ debuginfod_query_server (debuginfod_client *c, curl_easy_setopt(data[i].handle, CURLOPT_NOSIGNAL, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_AUTOREFERER, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_ACCEPT_ENCODING, ""); - add_extra_headers(data[i].handle); + 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); @@ -969,6 +976,8 @@ debuginfod_get_url(debuginfod_client *client) void debuginfod_end (debuginfod_client *client) { + if (client) /* make it safe to be invoked with NULL */ +curl_slist_free_all (client->headers); free (client->url); free (client); } @@ -978,6 +987,7 @@ debuginfod_find_debuginfo (debuginfod_client *client, const unsigned char *build_id, int build_id_len, char **path) { + add_default_headers(client); return debuginfod_query_server(client, build_id, build_id_len, "debuginfo", NULL, path); } @@ -989,6 +999,7 @@ debuginfod_find_executable(debuginfod_client *client, const unsigned char *build_id, int build_id_len, char **path) { + add_default_headers(client); return debuginfod_query_server(client, build_id, build_id_len, "executable", NULL, path); } @@ -998,11 +1009,29 @@ int debuginfod_find_source(debuginfod_client *client, const unsigned char *build_id, int build_id_len, const char *filename, char **path) { + add_default_headers(client); return debuginfod_query_server(client, build_id, build_id_len, "source", filename, path); } +/* Add an outgoing HTTP header. */ +int debuginfod_add_http_header (debugi