[pacman-dev] [PATCH 1/4] be_sync: use _alpm_db_get_sigverify_level()
Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/be_sync.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index 11e2807..d484185 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -84,6 +84,7 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) size_t len; int ret; mode_t oldmask; + pgp_verify_t check_sig; ALPM_LOG_FUNC; @@ -136,8 +137,10 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) goto cleanup; } + check_sig = _alpm_db_get_sigverify_level(db); + /* Download and check the signature of the database if needed */ - if(db->pgp_verify != PM_PGP_VERIFY_NEVER) { + if(check_sig != PM_PGP_VERIFY_NEVER) { char *sigfile, *sigfilepath; int sigret; @@ -155,7 +158,7 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) sigret = _alpm_download_single_file(sigfile, db->servers, syncpath, 0); free(sigfile); - if(sigret == -1 && db->pgp_verify == PM_PGP_VERIFY_ALWAYS) { + if(sigret == -1 && check_sig == PM_PGP_VERIFY_ALWAYS) { _alpm_log(PM_LOG_ERROR, _("Failed to download signature for db: %s\n"), alpm_strerrorlast()); pm_errno = PM_ERR_SIG_INVALID; @@ -164,8 +167,8 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) } sigret = alpm_db_check_pgp_signature(db); - if((db->pgp_verify == PM_PGP_VERIFY_ALWAYS && sigret != 0) || - (db->pgp_verify == PM_PGP_VERIFY_OPTIONAL && sigret == 1)) { + if((check_sig == PM_PGP_VERIFY_ALWAYS && sigret != 0) || + (check_sig == PM_PGP_VERIFY_OPTIONAL && sigret == 1)) { /* pm_errno was set by the checking code */ /* TODO: should we just leave the unverified database */ ret = -1; -- 1.7.4.4
Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/dload.c | 11 ++++++----- lib/libalpm/dload.h | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 40099b6..f756828 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -46,7 +46,7 @@ static double prevprogress; /* last download amount */ #endif -static char *get_filename(const char *url) +static const char *get_filename(const char *url) { char *filename = strrchr(url, '/'); if(filename != NULL) { @@ -71,9 +71,10 @@ static char *get_fullpath(const char *path, const char *filename, #define check_stop() if(dload_interrupted) { ret = -1; goto cleanup; } enum sighandlers { OLD = 0, NEW = 1 }; -int dload_interrupted; +static int dload_interrupted; static void inthandler(int signum) { + (void)signum; dload_interrupted = 1; } @@ -119,7 +120,7 @@ static int curl_progress(void *file, double dltotal, double dlnow, static int curl_gethost(const char *url, char *buffer) { - int hostlen; + size_t hostlen; char *p; if(strncmp(url, "file://", 7) == 0) { @@ -382,8 +383,8 @@ int _alpm_download_files(alpm_list_t *files, /** Fetch a remote pkg. */ char SYMEXPORT *alpm_fetch_pkgurl(const char *url) { - char *filename, *filepath; - const char *cachedir; + char *filepath; + const char *filename, *cachedir; int ret; ALPM_LOG_FUNC; diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h index 5ce44b8..324bd11 100644 --- a/lib/libalpm/dload.h +++ b/lib/libalpm/dload.h @@ -27,7 +27,7 @@ /* internal structure for communicating with curl progress callback */ struct fileinfo { - char *filename; + const char *filename; double initial_size; }; -- 1.7.4.4
The allow_resume is the start of the fix to the "don't ever resume database downloads" problem, as well as being useful for '.sig' downloads as well. For now, we say "always allow resume", but this will eventually get pushed down as necessary. Error checks are reworked in order to correctly error out when a file is not found on the remote end and reports 0 bytes downloaded. In addition, the two error messages printed are now different as one reports a more specific error message provided via the cURL error buffer. Some example output from an -Sy run with [testing], [community], [community2], [eee], and [nonexistant] defined as repos. [community2] and [nonexistant] are both invalid, one using FTP and one using HTTP. :: Synchronizing package databases... testing is up to date community is up to date error: failed retrieving file 'community2.db' from ftp.archlinux.org : Given file does not exist error: failed to update community2 (FTP: couldn't retrieve (RETR failed) the specified file) eee is up to date error: failed retrieving file 'nonexistant.db' from code.toofishes.net : The requested URL returned error: 404 error: failed to update nonexistant (HTTP response code said error) Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/dload.c | 40 +++++++++++++++++++++------------------- 1 files changed, 21 insertions(+), 19 deletions(-) diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index f756828..45fabaa 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -156,16 +156,18 @@ static int utimes_long(const char *path, long time) static int curl_download_internal(const char *url, const char *localpath, - int force) + int force, int allow_resume) { int ret = -1; FILE *localf = NULL; const char *useragent; const char *open_mode = "wb"; char *destfile, *tempfile; - char hostname[256]; /* RFC1123 states applications should support this length */ + /* RFC1123 states applications should support this length */ + char hostname[256]; + char error_buffer[CURL_ERROR_SIZE]; struct stat st; - long httpresp, timecond, remote_time; + long timecond, remote_time; double remote_size, bytes_dl; struct sigaction sig_pipe[2], sig_int[2]; struct fileinfo dlfile; @@ -188,6 +190,7 @@ static int curl_download_internal(const char *url, const char *localpath, curl_easy_reset(handle->curl); curl_easy_setopt(handle->curl, CURLOPT_URL, url); curl_easy_setopt(handle->curl, CURLOPT_FAILONERROR, 1L); + curl_easy_setopt(handle->curl, CURLOPT_ERRORBUFFER, error_buffer); curl_easy_setopt(handle->curl, CURLOPT_CONNECTTIMEOUT, 10L); curl_easy_setopt(handle->curl, CURLOPT_FILETIME, 1L); curl_easy_setopt(handle->curl, CURLOPT_NOPROGRESS, 0L); @@ -206,9 +209,8 @@ static int curl_download_internal(const char *url, const char *localpath, * our local is out of date. */ curl_easy_setopt(handle->curl, CURLOPT_TIMECONDITION, CURL_TIMECOND_IFMODSINCE); curl_easy_setopt(handle->curl, CURLOPT_TIMEVALUE, (long)st.st_mtime); - } else if(stat(tempfile, &st) == 0 && st.st_size > 0) { - /* assume its a partial package download. we do not support resuming of - * transfers on partially downloaded sync DBs. */ + } else if(stat(tempfile, &st) == 0 && allow_resume) { + /* a previous partial download exists, resume from end of file. */ open_mode = "ab"; curl_easy_setopt(handle->curl, CURLOPT_RESUME_FROM, (long)st.st_size); _alpm_log(PM_LOG_DEBUG, "tempfile found, attempting continuation"); @@ -243,8 +245,18 @@ static int curl_download_internal(const char *url, const char *localpath, /* perform transfer */ handle->curlerr = curl_easy_perform(handle->curl); + /* was it a success? */ + if(handle->curlerr == CURLE_ABORTED_BY_CALLBACK) { + goto cleanup; + } else if(handle->curlerr != CURLE_OK) { + pm_errno = PM_ERR_LIBCURL; + _alpm_log(PM_LOG_ERROR, _("failed retrieving file '%s' from %s : %s\n"), + dlfile.filename, hostname, error_buffer); + unlink(tempfile); + goto cleanup; + } + /* retrieve info about the state of the transfer */ - curl_easy_getinfo(handle->curl, CURLINFO_RESPONSE_CODE, &httpresp); curl_easy_getinfo(handle->curl, CURLINFO_FILETIME, &remote_time); curl_easy_getinfo(handle->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &remote_size); curl_easy_getinfo(handle->curl, CURLINFO_SIZE_DOWNLOAD, &bytes_dl); @@ -252,22 +264,12 @@ static int curl_download_internal(const char *url, const char *localpath, /* time condition was met and we didn't download anything. we need to * clean up the 0 byte .part file that's left behind. */ - if(DOUBLE_EQ(bytes_dl, 0) && timecond == 1) { + if(timecond == 1 && DOUBLE_EQ(bytes_dl, 0)) { ret = 1; unlink(tempfile); goto cleanup; } - if(handle->curlerr == CURLE_ABORTED_BY_CALLBACK) { - goto cleanup; - } else if(handle->curlerr != CURLE_OK) { - pm_errno = PM_ERR_LIBCURL; - _alpm_log(PM_LOG_ERROR, _("failed retrieving file '%s' from %s : %s\n"), - dlfile.filename, hostname, curl_easy_strerror(handle->curlerr)); - unlink(tempfile); - goto cleanup; - } - /* remote_size isn't necessarily the full size of the file, just what the * server reported as remaining to download. compare it to what curl reported * as actually being transferred during curl_easy_perform() */ @@ -313,7 +315,7 @@ static int download(const char *url, const char *localpath, { if(handle->fetchcb == NULL) { #ifdef HAVE_LIBCURL - return curl_download_internal(url, localpath, force); + return curl_download_internal(url, localpath, force, 1); #else RET_ERR(PM_ERR_EXTERNAL_DOWNLOAD, -1); #endif -- 1.7.4.4
There's a lot of related moving parts here: * Iteration through mirrors is moved back to the calling functions. This allows removal of _alpm_download_single_file and _alpm_download_files. * The download function gets a few more arguments to influence behavior. This allows several different scenarios to customize behavior: - database - database signature (req'd and optional) - package - package via direct URL - package signature via direct URL (req'd and optional) * For databases, we need signatures from the same mirror, so structure the code accordingly. Some-inspiration-from: Dave Reisner <d@falconindy.com> Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/be_sync.c | 88 +++++++++++++++++------------------------- lib/libalpm/dload.c | 101 +++++++++++++++++------------------------------- lib/libalpm/dload.h | 8 +--- lib/libalpm/sync.c | 25 +++++++++++- 4 files changed, 95 insertions(+), 127 deletions(-) diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index d484185..3dfea14 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -78,11 +78,12 @@ */ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) { - char *dbfile, *syncpath; + char *syncpath; const char *dbpath; + alpm_list_t *i; struct stat buf; size_t len; - int ret; + int ret = -1; mode_t oldmask; pgp_verify_t check_sig; @@ -91,14 +92,7 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) /* Sanity checks */ ASSERT(handle != NULL, RET_ERR(PM_ERR_HANDLE_NULL, -1)); ASSERT(db != NULL && db != handle->db_local, RET_ERR(PM_ERR_WRONG_ARGS, -1)); - - if(!alpm_list_find_ptr(handle->dbs_sync, db)) { - RET_ERR(PM_ERR_DB_NOT_FOUND, -1); - } - - len = strlen(db->treename) + 4; - MALLOC(dbfile, len, RET_ERR(PM_ERR_MEMORY, -1)); - sprintf(dbfile, "%s.db", db->treename); + ASSERT(db->servers != NULL, RET_ERR(PM_ERR_SERVER_NONE, -1)); dbpath = alpm_option_get_dbpath(); len = strlen(dbpath) + 6; @@ -112,20 +106,48 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) _alpm_log(PM_LOG_DEBUG, "database dir '%s' does not exist, creating it\n", syncpath); if(_alpm_makepath(syncpath) != 0) { - free(dbfile); free(syncpath); RET_ERR(PM_ERR_SYSTEM, -1); } } else if(!S_ISDIR(buf.st_mode)) { _alpm_log(PM_LOG_WARNING, _("removing invalid file: %s\n"), syncpath); if(unlink(syncpath) != 0 || _alpm_makepath(syncpath) != 0) { - free(dbfile); free(syncpath); RET_ERR(PM_ERR_SYSTEM, -1); } } - ret = _alpm_download_single_file(dbfile, db->servers, syncpath, force); + check_sig = _alpm_db_get_sigverify_level(db); + + for(i = db->servers; i; i = i->next) { + const char *server = i->data; + char *fileurl; + size_t len; + int sig_ret = 0; + + /* print server + filename into a buffer (leave space for .sig) */ + len = strlen(server) + strlen(db->treename) + 9; + CALLOC(fileurl, len, sizeof(char), RET_ERR(PM_ERR_MEMORY, -1)); + snprintf(fileurl, len, "%s/%s.db", server, db->treename); + + ret = _alpm_download(fileurl, syncpath, force, 0, 0); + + if(ret == 0 && (check_sig == PM_PGP_VERIFY_ALWAYS || + check_sig == PM_PGP_VERIFY_OPTIONAL)) { + int errors_ok = (check_sig == PM_PGP_VERIFY_OPTIONAL); + /* if we downloaded a DB, we want the .sig from the same server */ + snprintf(fileurl, len, "%s/%s.db.sig", server, db->treename); + + sig_ret = _alpm_download(fileurl, syncpath, 1, 0, errors_ok); + /* errors_ok suppresses error messages, but not the return code */ + sig_ret = errors_ok ? 0 : sig_ret; + } + + FREE(fileurl); + if(ret != -1 && sig_ret != -1) { + break; + } + } if(ret == 1) { /* files match, do nothing */ @@ -137,51 +159,11 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) goto cleanup; } - check_sig = _alpm_db_get_sigverify_level(db); - - /* Download and check the signature of the database if needed */ - if(check_sig != PM_PGP_VERIFY_NEVER) { - char *sigfile, *sigfilepath; - int sigret; - - len = strlen(dbfile) + 5; - MALLOC(sigfile, len, RET_ERR(PM_ERR_MEMORY, -1)); - sprintf(sigfile, "%s.sig", dbfile); - - /* prevent old signature being used if the following download fails */ - len = strlen(syncpath) + strlen(sigfile) + 1; - MALLOC(sigfilepath, len, RET_ERR(PM_ERR_MEMORY, -1)); - sprintf(sigfilepath, "%s%s", syncpath, sigfile); - _alpm_rmrf(sigfilepath); - free(sigfilepath); - - sigret = _alpm_download_single_file(sigfile, db->servers, syncpath, 0); - free(sigfile); - - if(sigret == -1 && check_sig == PM_PGP_VERIFY_ALWAYS) { - _alpm_log(PM_LOG_ERROR, _("Failed to download signature for db: %s\n"), - alpm_strerrorlast()); - pm_errno = PM_ERR_SIG_INVALID; - ret = -1; - goto cleanup; - } - - sigret = alpm_db_check_pgp_signature(db); - if((check_sig == PM_PGP_VERIFY_ALWAYS && sigret != 0) || - (check_sig == PM_PGP_VERIFY_OPTIONAL && sigret == 1)) { - /* pm_errno was set by the checking code */ - /* TODO: should we just leave the unverified database */ - ret = -1; - goto cleanup; - } - } - /* Cache needs to be rebuilt */ _alpm_db_free_pkgcache(db); cleanup: - free(dbfile); free(syncpath); umask(oldmask); return ret; diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 45fabaa..c27854c 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -156,7 +156,7 @@ static int utimes_long(const char *path, long time) static int curl_download_internal(const char *url, const char *localpath, - int force, int allow_resume) + int force, int allow_resume, int errors_ok) { int ret = -1; FILE *localf = NULL; @@ -249,9 +249,14 @@ static int curl_download_internal(const char *url, const char *localpath, if(handle->curlerr == CURLE_ABORTED_BY_CALLBACK) { goto cleanup; } else if(handle->curlerr != CURLE_OK) { - pm_errno = PM_ERR_LIBCURL; - _alpm_log(PM_LOG_ERROR, _("failed retrieving file '%s' from %s : %s\n"), - dlfile.filename, hostname, error_buffer); + if(!errors_ok) { + pm_errno = PM_ERR_LIBCURL; + _alpm_log(PM_LOG_ERROR, _("failed retrieving file '%s' from %s : %s\n"), + dlfile.filename, hostname, error_buffer); + } else { + _alpm_log(PM_LOG_DEBUG, "failed retrieving file '%s' from %s : %s\n", + dlfile.filename, hostname, error_buffer); + } unlink(tempfile); goto cleanup; } @@ -289,8 +294,6 @@ cleanup: utimes_long(tempfile, remote_time); } - /* TODO: A signature download will need to return success here as well before - * we're willing to rotate the new file into place. */ if(ret == 0) { rename(tempfile, destfile); } @@ -310,78 +313,24 @@ cleanup: } #endif -static int download(const char *url, const char *localpath, - int force) +int _alpm_download(const char *url, const char *localpath, + int force, int allow_resume, int errors_ok) { if(handle->fetchcb == NULL) { #ifdef HAVE_LIBCURL - return curl_download_internal(url, localpath, force, 1); + return curl_download_internal(url, localpath, force, allow_resume, errors_ok); #else RET_ERR(PM_ERR_EXTERNAL_DOWNLOAD, -1); #endif } else { int ret = handle->fetchcb(url, localpath, force); - if(ret == -1) { + if(ret == -1 && !errors_ok) { RET_ERR(PM_ERR_EXTERNAL_DOWNLOAD, -1); } return ret; } } -/* - * Download a single file - * - servers must be a list of urls WITHOUT trailing slashes. - * - * RETURN: 0 for successful download - * 1 if the files are identical - * -1 on error - */ -int _alpm_download_single_file(const char *filename, - alpm_list_t *servers, const char *localpath, - int force) -{ - alpm_list_t *i; - int ret = -1; - - ASSERT(servers != NULL, RET_ERR(PM_ERR_SERVER_NONE, -1)); - - for(i = servers; i; i = i->next) { - const char *server = i->data; - char *fileurl = NULL; - size_t len; - - /* print server + filename into a buffer */ - len = strlen(server) + strlen(filename) + 2; - CALLOC(fileurl, len, sizeof(char), RET_ERR(PM_ERR_MEMORY, -1)); - snprintf(fileurl, len, "%s/%s", server, filename); - - ret = download(fileurl, localpath, force); - FREE(fileurl); - if(ret != -1) { - break; - } - } - - return ret; -} - -int _alpm_download_files(alpm_list_t *files, - alpm_list_t *servers, const char *localpath) -{ - int ret = 0; - alpm_list_t *lp; - - for(lp = files; lp; lp = lp->next) { - char *filename = lp->data; - if(_alpm_download_single_file(filename, servers, - localpath, 0) == -1) { - ret++; - } - } - - return ret; -} - /** Fetch a remote pkg. */ char SYMEXPORT *alpm_fetch_pkgurl(const char *url) { @@ -397,13 +346,35 @@ char SYMEXPORT *alpm_fetch_pkgurl(const char *url) cachedir = _alpm_filecache_setup(); /* download the file */ - ret = download(url, cachedir, 0); + ret = _alpm_download(url, cachedir, 0, 1, 0); if(ret == -1) { _alpm_log(PM_LOG_WARNING, _("failed to download %s\n"), url); return NULL; } _alpm_log(PM_LOG_DEBUG, "successfully downloaded %s\n", url); + /* attempt to download the signature */ + if(ret == 0 && (handle->sigverify == PM_PGP_VERIFY_ALWAYS || + handle->sigverify == PM_PGP_VERIFY_OPTIONAL)) { + char *sig_url; + size_t len; + int errors_ok = (handle->sigverify == PM_PGP_VERIFY_OPTIONAL); + + len = strlen(url) + 5; + CALLOC(sig_url, len, sizeof(char), RET_ERR(PM_ERR_MEMORY, NULL)); + snprintf(sig_url, len, "%s.sig", url); + + ret = _alpm_download(sig_url, cachedir, 1, 0, errors_ok); + if(ret == -1 && !errors_ok) { + _alpm_log(PM_LOG_WARNING, _("failed to download %s\n"), sig_url); + /* Warn now, but don't return NULL. We will fail later during package + * load time. */ + } else if(ret == 0) { + _alpm_log(PM_LOG_DEBUG, "successfully downloaded %s\n", sig_url); + } + FREE(sig_url); + } + /* we should be able to find the file the second time around */ filepath = _alpm_filecache_find(filename); return filepath; diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h index 324bd11..f4fd14c 100644 --- a/lib/libalpm/dload.h +++ b/lib/libalpm/dload.h @@ -31,12 +31,8 @@ struct fileinfo { double initial_size; }; -int _alpm_download_single_file(const char *filename, - alpm_list_t *servers, const char *localpath, - int force); - -int _alpm_download_files(alpm_list_t *files, - alpm_list_t *servers, const char *localpath); +int _alpm_download(const char *url, const char *localpath, + int force, int allow_resume, int errors_ok); #endif /* _ALPM_DLOAD_H */ diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 63ea2b2..4ce62e6 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -689,8 +689,8 @@ static int test_md5sum(pmtrans_t *trans, const char *filepath, int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data) { - alpm_list_t *i, *j, *files = NULL; - alpm_list_t *deltas = NULL; + alpm_list_t *i, *j, *k; + alpm_list_t *files = NULL, *deltas = NULL; size_t numtargs, current = 0, replaces = 0; int errors = 0; const char *cachedir = NULL; @@ -758,7 +758,26 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data) if(files) { EVENT(trans, PM_TRANS_EVT_RETRIEVE_START, current->treename, NULL); - errors = _alpm_download_files(files, current->servers, cachedir); + for(j = files; j; j = j->next) { + const char *filename = j->data; + for(k = current->servers; k; k = k->next) { + const char *server = k->data; + char *fileurl; + size_t len; + + /* print server + filename into a buffer */ + len = strlen(server) + strlen(filename) + 2; + CALLOC(fileurl, len, sizeof(char), RET_ERR(PM_ERR_MEMORY, -1)); + snprintf(fileurl, len, "%s/%s", server, filename); + + ret = _alpm_download(fileurl, cachedir, 0, 1, 0); + FREE(fileurl); + if(ret != -1) { + break; + } + errors++; + } + } if(errors) { _alpm_log(PM_LOG_WARNING, _("failed to retrieve some files from %s\n"), -- 1.7.4.4
On 22/04/11 09:36, Dan McGee wrote:
There's a lot of related moving parts here: * Iteration through mirrors is moved back to the calling functions. This allows removal of _alpm_download_single_file and _alpm_download_files. * The download function gets a few more arguments to influence behavior. This allows several different scenarios to customize behavior: - database - database signature (req'd and optional) - package - package via direct URL - package signature via direct URL (req'd and optional) * For databases, we need signatures from the same mirror, so structure the code accordingly.
Some-inspiration-from: Dave Reisner<d@falconindy.com> Signed-off-by: Dan McGee<dan@archlinux.org>
Signed-off-by: Allan Extends to PATCH 3/4 in this series too. Both patches look good to me and I have been unable to break them in the last couple of days. Allan
participants (2)
-
Allan McRae
-
Dan McGee