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

[Eli Schwartz]

On 6/14/21 8:15 AM, 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
> ---
>  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;
>  
> -		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")) {

For a .db we explicitly need it to be the last component, so we might as
well check that .db isn't a redirect to .db.tar.gz, which per IRC
discussion lazka's server appears to do.

The .pkg case is different since we have .pkg.tar plus optional and
fully arbitrary compression extensions emanating from libarchive, so I
could understand taking the simple route and not bothering there.

> +			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.
> 


-- 
Eli Schwartz
Bug Wrangler and Trusted User

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/pacman-dev/attachments/20210616/1ba9ad1d/attachment.sig>