From 0734afd41f4202411e7b9a0120a0d06c954d00ab Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Tue, 19 Sep 2017 11:59:35 +0100 Subject: [PATCH] lib: Fix guestfs_int_download_to_tmp. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit 65cfecb0f5344ec92d3f0d3c2ec0538b6b2726e2, ‘guestfs_int_download_to_tmp’ was buggy because it did not deal with the case where a guest had multiple roots. It cached the downloaded file under a single name which was not distinguished by which root we were looking at. As a result, if you inspected (eg.) the icon on such a guest, then in some circumstances the same icon could be returned for all roots (incorrectly). This changes the function in a few ways: - Don't cache downloaded files. It wasn't a useful feature of the function in most scenarios. Instead a new name is generated for every call. - Use guestfs_int_make_temp_path. - Allow an extension to be specified. --- lib/guestfs-internal.h | 2 +- lib/inspect-apps.c | 12 +++++------- lib/inspect-icon.c | 36 +++++++++++++++--------------------- 3 files changed, 21 insertions(+), 29 deletions(-) diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h index f3379da7d..66e03c3b8 100644 --- a/lib/guestfs-internal.h +++ b/lib/guestfs-internal.h @@ -713,7 +713,7 @@ extern int guestfs_int_set_backend (guestfs_h *g, const char *method); } while (0) /* inspect.c */ -extern char *guestfs_int_download_to_tmp (guestfs_h *g, const char *filename, const char *basename, uint64_t max_size); +extern char *guestfs_int_download_to_tmp (guestfs_h *g, const char *filename, const char *extension, uint64_t max_size); /* dbdump.c */ typedef int (*guestfs_int_db_dump_callback) (guestfs_h *g, const unsigned char *key, size_t keylen, const unsigned char *value, size_t valuelen, void *opaque); diff --git a/lib/inspect-apps.c b/lib/inspect-apps.c index b721595da..f0cf16b38 100644 --- a/lib/inspect-apps.c +++ b/lib/inspect-apps.c @@ -370,14 +370,12 @@ list_applications_rpm (guestfs_h *g, const char *root) struct guestfs_application2_list *apps = NULL; struct read_package_data data; - Name = guestfs_int_download_to_tmp (g, - "/var/lib/rpm/Name", "rpm_Name", + Name = guestfs_int_download_to_tmp (g, "/var/lib/rpm/Name", NULL, MAX_PKG_DB_SIZE); if (Name == NULL) goto error; - Packages = guestfs_int_download_to_tmp (g, - "/var/lib/rpm/Packages", "rpm_Packages", + Packages = guestfs_int_download_to_tmp (g, "/var/lib/rpm/Packages", NULL, MAX_PKG_DB_SIZE); if (Packages == NULL) goto error; @@ -429,7 +427,7 @@ list_applications_deb (guestfs_h *g, const char *root) char **continuation_field = NULL; size_t continuation_field_len = 0; - status = guestfs_int_download_to_tmp (g, "/var/lib/dpkg/status", "status", + status = guestfs_int_download_to_tmp (g, "/var/lib/dpkg/status", NULL, MAX_PKG_DB_SIZE); if (status == NULL) return NULL; @@ -605,7 +603,7 @@ list_applications_pacman (guestfs_h *g, const char *root) fname = safe_malloc (g, strlen (curr->name) + path_len + 1); sprintf (fname, "/var/lib/pacman/local/%s/desc", curr->name); free (desc_file); - desc_file = guestfs_int_download_to_tmp (g, fname, curr->name, 8192); + desc_file = guestfs_int_download_to_tmp (g, fname, NULL, 8192); /* The desc files are small (4K). If the desc file does not exist or is * larger than the 8K limit we've used, the database is probably corrupted, @@ -714,7 +712,7 @@ list_applications_apk (guestfs_h *g, const char *root) *url = NULL, *description = NULL; installed = guestfs_int_download_to_tmp (g, "/lib/apk/db/installed", - "installed", MAX_PKG_DB_SIZE); + NULL, MAX_PKG_DB_SIZE); if (installed == NULL) return NULL; diff --git a/lib/inspect-icon.c b/lib/inspect-icon.c index c45761041..3e44b23eb 100644 --- a/lib/inspect-icon.c +++ b/lib/inspect-icon.c @@ -234,7 +234,7 @@ get_png (guestfs_h *g, const char *filename, size_t *size_r, size_t max_size) if (max_size == 0) max_size = 4 * w * h; - local = guestfs_int_download_to_tmp (g, real, "icon", max_size); + local = guestfs_int_download_to_tmp (g, real, "png", max_size); if (!local) return NOT_FOUND; @@ -385,7 +385,7 @@ icon_cirros (guestfs_h *g, size_t *size_r) if (!STRPREFIX (type, "ASCII text")) return NOT_FOUND; - local = guestfs_int_download_to_tmp (g, CIRROS_LOGO, "icon", 1024); + local = guestfs_int_download_to_tmp (g, CIRROS_LOGO, "png", 1024); if (!local) return NOT_FOUND; @@ -469,8 +469,7 @@ icon_windows_xp (guestfs_h *g, const char *systemroot, size_t *size_r) if (r == 0) return NOT_FOUND; - filename_downloaded = guestfs_int_download_to_tmp (g, filename_case, - "explorer.exe", + filename_downloaded = guestfs_int_download_to_tmp (g, filename_case, "exe", MAX_WINDOWS_EXPLORER_SIZE); if (filename_downloaded == NULL) return NOT_FOUND; @@ -538,8 +537,7 @@ icon_windows_7 (guestfs_h *g, const char *systemroot, size_t *size_r) if (win7_explorer[i] == NULL) return NOT_FOUND; - filename_downloaded = guestfs_int_download_to_tmp (g, filename_case, - "explorer.exe", + filename_downloaded = guestfs_int_download_to_tmp (g, filename_case, "exe", MAX_WINDOWS_EXPLORER_SIZE); if (filename_downloaded == NULL) return NOT_FOUND; @@ -592,8 +590,8 @@ icon_windows_8 (guestfs_h *g, size_t *size_r) if (r == 0) return NOT_FOUND; - filename_downloaded = guestfs_int_download_to_tmp (g, filename_case, - "wlive48x48.png", 8192); + filename_downloaded = guestfs_int_download_to_tmp (g, filename_case, "png", + 8192); if (filename_downloaded == NULL) return NOT_FOUND; @@ -649,35 +647,31 @@ case_sensitive_path_silently (guestfs_h *g, const char *path) } /** - * Download a guest file to a local temporary file. The file is - * cached in the temporary directory using C, and is not - * downloaded again. + * Download a guest file to a local temporary file. * * The name of the temporary (downloaded) file is returned. The * caller must free the pointer, but does I need to delete the * temporary file. It will be deleted when the handle is closed. * + * The name of the temporary file is randomly generated, but an + * extension can be specified using C (or pass C for none). + * * Refuse to download the guest file if it is larger than C. * On this and other errors, C is returned. - * - * XXX Prior to commit 65cfecb0f5344ec92d3f0d3c2ec0538b6b2726e2 this - * function used different basenames for each inspection root. After - * this commit icons probably won't work properly. Needs fixing. */ char * -guestfs_int_download_to_tmp (guestfs_h *g, - const char *filename, - const char *basename, uint64_t max_size) +guestfs_int_download_to_tmp (guestfs_h *g, const char *filename, + const char *extension, + uint64_t max_size) { char *r; int fd; char devfd[32]; int64_t size; - if (asprintf (&r, "%s/%s", g->tmpdir, basename) == -1) { - perrorf (g, "asprintf"); + r = guestfs_int_make_temp_path (g, "download", extension); + if (r == NULL) return NULL; - } /* Check size of remote file. */ size = guestfs_filesize (g, filename);