If the original download redirects to to a different url then alpm would try to name the sig file after the url instead of <original_file>.sig. Instead force this naming scheme regardless of url.
Fixes FS#71274 --- lib/libalpm/dload.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 2c14841f..ca6be7b6 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -614,6 +614,7 @@ static int curl_check_finished_download(CURLM *curlm, CURLMsg *msg, if(!payload->signature && payload->download_signature && curlerr == CURLE_OK && payload->respcode < 400) { struct dload_payload *sig = NULL;
+ const char* realname = payload->destfile_name ? payload->destfile_name : payload->tempfile_name; int len = strlen(effective_url) + 5; CALLOC(sig, 1, sizeof(*sig), GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup)); MALLOC(sig->fileurl, len, FREE(sig); GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup)); @@ -623,13 +624,18 @@ static int curl_check_finished_download(CURLM *curlm, CURLMsg *msg, /* In this case server might provide a new name for the main payload. * Choose *.sig filename based on this new name. */ - const char* realname = payload->destfile_name ? payload->destfile_name : payload->tempfile_name; const char *final_file = get_filename(realname); int remote_name_len = strlen(final_file) + 5; MALLOC(sig->remote_name, remote_name_len, FREE(sig->fileurl); FREE(sig); GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup)); snprintf(sig->remote_name, remote_name_len, "%s.sig", final_file); }
+ /* force the filename to be realname + ".sig" */ + int destfile_name_len = strlen(realname) + 5; + MALLOC(sig->destfile_name, destfile_name_len, FREE(sig->remote_name); + FREE(sig->fileurl); FREE(sig); GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup)); + snprintf(sig->destfile_name, destfile_name_len, "%s.sig", realname); + sig->signature = 1; sig->handle = handle; sig->force = payload->force; @@ -762,7 +768,9 @@ static int curl_add_payload(alpm_handle_t *handle, CURLM *curlm, }
if(payload->remote_name && strlen(payload->remote_name) > 0) { - payload->destfile_name = get_fullpath(localpath, payload->remote_name, ""); + if(!payload->destfile_name) { + payload->destfile_name = get_fullpath(localpath, payload->remote_name, ""); + } payload->tempfile_name = get_fullpath(localpath, payload->remote_name, ".part"); if(!payload->destfile_name || !payload->tempfile_name) { goto cleanup;
On 17/6/21 11:14 am, morganamilo wrote:
If the original download redirects to to a different url then alpm would try to name the sig file after the url instead of <original_file>.sig. Instead force this naming scheme regardless of url.
Fixes FS#71274
This patch looks good to me.
Allan
lib/libalpm/dload.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 2c14841f..ca6be7b6 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -614,6 +614,7 @@ static int curl_check_finished_download(CURLM *curlm, CURLMsg *msg, if(!payload->signature && payload->download_signature && curlerr == CURLE_OK && payload->respcode < 400) { struct dload_payload *sig = NULL;
const char* realname = payload->destfile_name ? payload->destfile_name : payload->tempfile_name;@@ -623,13 +624,18 @@ static int curl_check_finished_download(CURLM *curlm, CURLMsg *msg, /* In this case server might provide a new name for the main payload. * Choose *.sig filename based on this new name. */
const char* realname = payload->destfile_name ? payload->destfile_name : payload->tempfile_name; const char *final_file = get_filename(realname); int remote_name_len = strlen(final_file) + 5; MALLOC(sig->remote_name, remote_name_len, FREE(sig->fileurl); FREE(sig); GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup)); snprintf(sig->remote_name, remote_name_len, "%s.sig", final_file);
/* force the filename to be realname + ".sig" */int destfile_name_len = strlen(realname) + 5;MALLOC(sig->destfile_name, destfile_name_len, FREE(sig->remote_name);FREE(sig->fileurl); FREE(sig); GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup));snprintf(sig->destfile_name, destfile_name_len, "%s.sig", realname);- sig->signature = 1; sig->handle = handle; sig->force = payload->force;
@@ -762,7 +768,9 @@ static int curl_add_payload(alpm_handle_t *handle, CURLM *curlm, }
if(payload->remote_name && strlen(payload->remote_name) > 0) {
payload->destfile_name = get_fullpath(localpath, payload->remote_name, "");
if(!payload->destfile_name) {payload->destfile_name = get_fullpath(localpath, payload->remote_name, "");}
Currently when a package is in a secondary cachedir and there is no sig file, the downloader redownloads the whole file into the primary cachedir.
So lets check all cachedirs for an up to date file.
Fixes FS#71109
---
v2: ensure .sig is downlaoded to same cachedir as the package
Note this patch now depends on "libalpm: always name signatures after original file" --- lib/libalpm/be_sync.c | 2 +- lib/libalpm/dload.c | 64 +++++++++++++++++++++++++++++-------------- lib/libalpm/dload.h | 3 +- lib/libalpm/sync.c | 2 +- 4 files changed, 48 insertions(+), 23 deletions(-)
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index d85f36ee..f5f70a10 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -205,7 +205,7 @@ int SYMEXPORT alpm_db_update(alpm_handle_t *handle, alpm_list_t *dbs, int force)
event.type = ALPM_EVENT_DB_RETRIEVE_START; EVENT(handle, &event); - ret = _alpm_download(handle, payloads, syncpath); + ret = _alpm_download(handle, payloads, syncpath, NULL); if(ret < 0) { event.type = ALPM_EVENT_DB_RETRIEVE_FAILED; EVENT(handle, &event); diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index ca6be7b6..d8b9ea0d 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -55,7 +55,7 @@ #define HOSTNAME_SIZE 256
static int curl_add_payload(alpm_handle_t *handle, CURLM *curlm, - struct dload_payload *payload, const char *localpath); + struct dload_payload *payload, const char *localpath, alpm_list_t *cachedirs); static int curl_gethost(const char *url, char *buffer, size_t buf_len);
/* number of "soft" errors required to blacklist a server, set to 0 to disable @@ -308,7 +308,7 @@ static size_t dload_parseheader_cb(void *ptr, size_t size, size_t nmemb, void *u return realsize; }
-static void curl_set_handle_opts(CURL *curl, struct dload_payload *payload) +static void curl_set_handle_opts(CURL *curl, struct dload_payload *payload, alpm_list_t *cachedirs) { alpm_handle_t *handle = payload->handle; const char *useragent = getenv("HTTP_USER_AGENT"); @@ -353,14 +353,36 @@ static void curl_set_handle_opts(CURL *curl, struct dload_payload *payload) curl_easy_setopt(curl, CURLOPT_USERAGENT, useragent); }
- if(!payload->force && payload->destfile_name && - stat(payload->destfile_name, &st) == 0) { - /* start from scratch, but only download if our local is out of date. */ - curl_easy_setopt(curl, CURLOPT_TIMECONDITION, CURL_TIMECOND_IFMODSINCE); - curl_easy_setopt(curl, CURLOPT_TIMEVALUE, (long)st.st_mtime); - _alpm_log(handle, ALPM_LOG_DEBUG, - "%s: using time condition %ld\n", - payload->remote_name, (long)st.st_mtime); + if(!payload->force && payload->destfile_name) { + if(stat(payload->destfile_name, &st) == 0) { + /* start from scratch, but only download if our local is out of date. */ + curl_easy_setopt(curl, CURLOPT_TIMECONDITION, CURL_TIMECOND_IFMODSINCE); + curl_easy_setopt(curl, CURLOPT_TIMEVALUE, (long)st.st_mtime); + _alpm_log(handle, ALPM_LOG_DEBUG, + "%s: using time condition %ld\n", + payload->remote_name, (long)st.st_mtime); + } else { + /* try and find an up to date file anywhere in cache */ + const char *name = get_filename(payload->destfile_name); + for(alpm_list_t *i = cachedirs; i; i = i->next) { + char *cachedir = i->data; + char *filepath = get_fullpath(cachedir, name, ""); + if(stat(filepath, &st) == 0) { + /* start from scratch, but only download if our local is out of date. */ + curl_easy_setopt(curl, CURLOPT_TIMECONDITION, CURL_TIMECOND_IFMODSINCE); + curl_easy_setopt(curl, CURLOPT_TIMEVALUE, (long)st.st_mtime); + _alpm_log(handle, ALPM_LOG_DEBUG, + "%s: using time condition %ld\n", + payload->remote_name, (long)st.st_mtime); + + /* use this cache dir for the payload and its .sig */ + free(payload->destfile_name); + payload->destfile_name = filepath; + break; + } + free(filepath); + } + } } else if(stat(payload->tempfile_name, &st) == 0 && payload->allow_resume) { /* a previous partial download exists, resume from end of file. */ payload->tempfile_openmode = "ab"; @@ -469,7 +491,7 @@ static int curl_retry_next_server(CURLM *curlm, CURL *curl, struct dload_payload * Returns -2 if an error happened for an optional file */ static int curl_check_finished_download(CURLM *curlm, CURLMsg *msg, - const char *localpath, int *active_downloads_num) + const char *localpath, int *active_downloads_num, alpm_list_t *cachedirs) { alpm_handle_t *handle = NULL; struct dload_payload *payload = NULL; @@ -644,7 +666,7 @@ static int curl_check_finished_download(CURLM *curlm, CURLMsg *msg, /* set hard upper limit of 16KiB */ sig->max_size = 16 * 1024;
- curl_add_payload(handle, curlm, sig, localpath); + curl_add_payload(handle, curlm, sig, localpath, cachedirs); (*active_downloads_num)++; }
@@ -727,7 +749,7 @@ cleanup: * Returns -1 if am error happened while starting a new download */ static int curl_add_payload(alpm_handle_t *handle, CURLM *curlm, - struct dload_payload *payload, const char *localpath) + struct dload_payload *payload, const char *localpath, alpm_list_t *cachedirs) { size_t len; CURL *curl = NULL; @@ -786,7 +808,7 @@ static int curl_add_payload(alpm_handle_t *handle, CURLM *curlm, } }
- curl_set_handle_opts(curl, payload); + curl_set_handle_opts(curl, payload, cachedirs);
if(payload->max_size == payload->initial_size && payload->max_size != 0) { /* .part file is complete */ @@ -831,7 +853,8 @@ cleanup: */ static int curl_download_internal(alpm_handle_t *handle, alpm_list_t *payloads /* struct dload_payload */, - const char *localpath) + const char *localpath, + alpm_list_t *cachedirs) { int active_downloads_num = 0; int err = 0; @@ -845,7 +868,7 @@ static int curl_download_internal(alpm_handle_t *handle, for(; active_downloads_num < max_streams && payloads; active_downloads_num++) { struct dload_payload *payload = payloads->data;
- if(curl_add_payload(handle, curlm, payload, localpath) == 0) { + if(curl_add_payload(handle, curlm, payload, localpath, cachedirs) == 0) { payloads = payloads->next; } else { /* The payload failed to start. Do not start any new downloads. @@ -876,7 +899,7 @@ static int curl_download_internal(alpm_handle_t *handle, } if(msg->msg == CURLMSG_DONE) { int ret = curl_check_finished_download(curlm, msg, - localpath, &active_downloads_num); + localpath, &active_downloads_num, cachedirs); if(ret == -1) { /* if current payload failed to download then stop adding new payloads but wait for the * current ones @@ -904,11 +927,12 @@ static int curl_download_internal(alpm_handle_t *handle, */ int _alpm_download(alpm_handle_t *handle, alpm_list_t *payloads /* struct dload_payload */, - const char *localpath) + const char *localpath, + alpm_list_t *cachedirs) { if(handle->fetchcb == NULL) { #ifdef HAVE_LIBCURL - return curl_download_internal(handle, payloads, localpath); + return curl_download_internal(handle, payloads, localpath, cachedirs); #else RET_ERR(handle, ALPM_ERR_EXTERNAL_DOWNLOAD, -1); #endif @@ -1003,7 +1027,7 @@ int SYMEXPORT alpm_fetch_pkgurl(alpm_handle_t *handle, const alpm_list_t *urls, event.type = ALPM_EVENT_PKG_RETRIEVE_START; event.pkg_retrieve.num = alpm_list_count(payloads); EVENT(handle, &event); - if(_alpm_download(handle, payloads, cachedir) == -1) { + if(_alpm_download(handle, payloads, cachedir, handle->cachedirs) == -1) { _alpm_log(handle, ALPM_LOG_WARNING, _("failed to retrieve some files\n")); event.type = ALPM_EVENT_PKG_RETRIEVE_FAILED; EVENT(handle, &event); diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h index 8f3d17b4..f5dabae1 100644 --- a/lib/libalpm/dload.h +++ b/lib/libalpm/dload.h @@ -61,6 +61,7 @@ void _alpm_dload_payload_reset(struct dload_payload *payload);
int _alpm_download(alpm_handle_t *handle, alpm_list_t *payloads /* struct dload_payload */, - const char *localpath); + const char *localpath, + alpm_list_t *cachedirs);
#endif /* ALPM_DLOAD_H */ diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 36ad6242..a1226a4c 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -833,7 +833,7 @@ static int download_files(alpm_handle_t *handle) payloads = alpm_list_add(payloads, payload); }
- ret = _alpm_download(handle, payloads, cachedir); + ret = _alpm_download(handle, payloads, cachedir, handle->cachedirs); if(ret == -1) { event.type = ALPM_EVENT_PKG_RETRIEVE_FAILED; EVENT(handle, &event);
On 26/6/21 3:45 am, morganamilo wrote:
Currently when a package is in a secondary cachedir and there is no sig file, the downloader redownloads the whole file into the primary cachedir.
So lets check all cachedirs for an up to date file.
Fixes FS#71109
v2: ensure .sig is downlaoded to same cachedir as the package
As far as I can see, this does not ensure that all cache directories are writable before attempting to download the signature file alongside the package file.
I am going to make a declaration that this "issue" is a transitional one. Once packages are downloaded with pacman-6.0+, they will have the signature file and this issue disappears. A "pacman -Scc" would "fix" the issue.
Perhaps repos that do not have signatures and are configured with "SigLevel = Optional" still have an issue.
In any case, I am pushing this beyond pacman-6.0.1.
Allan
pacman-dev@lists.archlinux.org