[pacman-dev] [PATCH 1/2] only use effective url for urls containing .db or .pkg

Andrew Gregory andrew.gregory.8 at gmail.com
Sun Jun 20 01:24:01 UTC 2021


On 06/14/21 at 01:15pm, morganamilo wrote:
> Github and other sites redirect their downloads to a cdn. So the
> download http://foo.org/myrepo.db may redirect to something like
> https://cdn.foo.org/83749327439.
> 
> This then causes pacman to try and download the sig as
> https://cdn.foo.org/83749327439.sig which is incorrect. In this case
> pacman should append .sig to the original url.
> 
> However urls like https://archlinux.org/packages/community/x86_64/0ad/download/
> Redirect to the mirror, so .sig has to appended after the redirects and
> not before.
> 
> So we decide if we should append .sig on the original or effective url
> based on if the effective url has .db or .pkg in it.
> 
> Fixes FS#71148

At some point we need to stop chasing what sites are doing and lay out
specific requirements for mirrors because any heuristic we use is
going to break somebody's setup.  This patch might work for github's
current url scheme, but there's nothing stopping them or others from
putting .db somewhere in the middle of the url.  I'm really tempted to
just always append .sig to the original url and say that it's up to
people using redirection to make sure that's valid and all possible
destinations are properly synced.

> ---
>  lib/libalpm/dload.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index 2c14841f..72e9cfcd 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -613,11 +613,28 @@ static int curl_check_finished_download(CURLM *curlm, CURLMsg *msg,
>  	/* Let's check if client requested downloading accompanion *.sig file */
>  	if(!payload->signature && payload->download_signature && curlerr == CURLE_OK && payload->respcode < 400) {
>  		struct dload_payload *sig = NULL;
> +		char *url = payload->fileurl;
> +		char *_effective_filename;

Style rules say all variable declarations go here.

>  
> -		int len = strlen(effective_url) + 5;
> +		STRDUP(_effective_filename, effective_url, GOTO_ERR(handle, ALPM_ERR_MEMORY, cleanup));
> +		const char *effective_filename = get_filename(_effective_filename);
> +		char *query = strrchr(effective_filename, '?');
> +
> +		if(query) {
> +			query[0] = '\0';
> +		}
> +
> +		/* Only use the effective url for sig downloads if the effective_url contains .db or .pkg */
> +		if(strstr(effective_filename, ".db") || strstr(effective_filename, ".pkg")) {

Both .db and .pkg are merely conventions.  

> +			url = effective_url;
> +		}
> +
> +		free(_effective_filename);
> +
> +		int len = strlen(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));
> -		snprintf(sig->fileurl, len, "%s.sig", effective_url);
> +		snprintf(sig->fileurl, len, "%s.sig", url);
>  
>  		if(payload->trust_remote_name) {
>  			/* In this case server might provide a new name for the main payload.
> -- 
> 2.32.0


More information about the pacman-dev mailing list