[pacman-dev] [PATCH 1/1] fix db filename handling

[Dave Reisner]

On Wed, Sep 04, 2013 at 05:35:51PM +0200, Christian Hesse wrote:
> From: Christian Hesse <mail at eworm.de>
> 
> If the server redirects from ${repo}.db to ${repo}.db.tar.gz pacman gets
> this wrong: It saves to new filename and fails when accessing
> ${repo}.db.
> 
> This introduces a new field 'deny_rename' to payload. If set pacman
> downloads to the filename specified in payload.remote_name.
> ---
>  lib/libalpm/be_sync.c |  2 ++
>  lib/libalpm/dload.c   | 36 ++++++++++++++++++++----------------
>  lib/libalpm/dload.h   |  1 +
>  3 files changed, 23 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index 123d953..622153d 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -223,6 +223,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
>  		payload.handle = handle;
>  		payload.force = force;
>  		payload.unlink_on_fail = 1;
> +		payload.deny_rename = 1;
>  
>  		ret = _alpm_download(&payload, syncpath, NULL, NULL);
>  		_alpm_dload_payload_reset(&payload);
> @@ -245,6 +246,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
>  			snprintf(payload.fileurl, len, "%s/%s.db.sig", server, db->treename);
>  			payload.handle = handle;
>  			payload.force = 1;
> +			payload.deny_rename = 1;
>  			payload.errors_ok = (level & ALPM_SIG_DATABASE_OPTIONAL);
>  
>  			/* set hard upper limit of 16KiB */
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index 8537b3d..381e548 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -547,23 +547,27 @@ static int curl_download_internal(struct dload_payload *payload,
>  		goto cleanup;
>  	}
>  
> -	if(payload->content_disp_name) {
> -		/* content-disposition header has a better name for our file */
> -		free(payload->destfile_name);
> -		payload->destfile_name = get_fullpath(localpath, payload->content_disp_name, "");
> +	if(payload->deny_rename) {
> +		payload->destfile_name = get_fullpath(localpath, payload->remote_name, "");

This is actually a leak. payload->destfile_name should already be set
with this exact value earlier in the function. You only need to worry
about the negative case where we allow renaming.

I've fixed this up on my own branch.

>  	} else {
> -		const char *effective_filename = strrchr(effective_url, '/');
> -		if(effective_filename && strlen(effective_filename) > 2) {
> -			effective_filename++;
> -
> -			/* if destfile was never set, we wrote to a tempfile. even if destfile is
> -			 * set, we may have followed some redirects and the effective url may
> -			 * have a better suggestion as to what to name our file. in either case,
> -			 * refactor destfile to this newly derived name. */
> -			if(!payload->destfile_name || strcmp(effective_filename,
> -						strrchr(payload->destfile_name, '/') + 1) != 0) {
> -				free(payload->destfile_name);
> -				payload->destfile_name = get_fullpath(localpath, effective_filename, "");
> +		if(payload->content_disp_name) {
> +			/* content-disposition header has a better name for our file */
> +			free(payload->destfile_name);
> +			payload->destfile_name = get_fullpath(localpath, payload->content_disp_name, "");
> +		} else {
> +			const char *effective_filename = strrchr(effective_url, '/');
> +			if(effective_filename && strlen(effective_filename) > 2) {
> +				effective_filename++;
> +
> +				/* if destfile was never set, we wrote to a tempfile. even if destfile is
> +				 * set, we may have followed some redirects and the effective url may
> +				 * have a better suggestion as to what to name our file. in either case,
> +				 * refactor destfile to this newly derived name. */
> +				if(!payload->destfile_name || strcmp(effective_filename,
> +							strrchr(payload->destfile_name, '/') + 1) != 0) {
> +					free(payload->destfile_name);
> +					payload->destfile_name = get_fullpath(localpath, effective_filename, "");
> +				}
>  			}
>  		}
>  	}
> diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h
> index 95bb91a..bd01d8b 100644
> --- a/lib/libalpm/dload.h
> +++ b/lib/libalpm/dload.h
> @@ -38,6 +38,7 @@ struct dload_payload {
>  	int allow_resume;
>  	int errors_ok;
>  	int unlink_on_fail;
> +	int deny_rename;
>  	alpm_list_t *servers;
>  #ifdef HAVE_LIBCURL
>  	CURLcode curlerr;       /* last error produced by curl */
> -- 
> 1.8.4
> 
>