[pacman-dev] [PATCH v2 1/2] libalpm: Do a sanity check before manipulating final DB URL

[Allan McRae]

On 18/04/15 01:31, David Macek wrote:
> The change in commit 9d96bed9d6b57 causes download errors for the .db.sig file
> in case the final URL for the .db file contains query strings or other
> unexpected stuff. This commit isn't intended to be a total solution, but it
> should eliminate the problem in the most obvious cases.
> ---
>  lib/libalpm/be_sync.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index 20130dc..606c4a0 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -241,9 +241,16 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
>  			unlink(sigpath);
>  			free(sigpath);
>  
> -			/* if we downloaded a DB, we want the .sig from the same server -
> -			   this information is only available from the internal downloader */
> -			if(handle->fetchcb == NULL) {
> +
> +			/* check if the final URL from internal downloader looks reasonable */
> +			if(final_db_url != NULL) {
> +				if(strlen(final_db_url) < 3 || strcmp(final_db_url + strlen(final_db_url) - 3, ".db") != 0) {
> +					final_db_url = NULL;
> +				}
> +			}
> +
> +			/* if we downloaded a DB, we want the .sig from the same server */
> +			if(final_db_url != NULL) {

I am fairly certain this is OK...   but there is a nagging feeling that
I am missing something in the change:

 -			if(handle->fetchcb == NULL) {
 +			if(final_db_url != NULL) {


@Dave: any chance you could take a very quick glance at this?


>  				/* print final_db_url into a buffer (leave space for .sig) */
>  				len = strlen(final_db_url) + 5;
>  			} else {
> @@ -254,7 +261,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
>  			/* TODO fix leak syncpath and umask unset */
>  			MALLOC(payload.fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1));
>  
> -			if(handle->fetchcb == NULL) {
> +			if(final_db_url != NULL) {
>  				snprintf(payload.fileurl, len, "%s.sig", final_db_url);
>  			} else {
>  				snprintf(payload.fileurl, len, "%s/%s.db.sig", server, db->treename);
>