On 2/6/21 9:44 pm, Hung-I Wang wrote:
> `curl_retry_next_server` tries to resume a download whenever possible
> (introduced in 8bf17b2) even if its preceding HTTP request returns an
> error (with status code >= 400, e.g. 404 when a mirror is only partially
> synced).
> 
> It may result in a corrupted package if the preceding HTTP response with
> error carries a non-empty body, which is then written to a tempfile as
> if it is part of the package binary.
> 
> By activating `CURLOPT_FAILONERROR`, the tempfile won't be written
> unless the HTTP response indicates a successful status.
> 

Thanks - that option is much more simple than the approaches we had
implemented.

> Signed-off-by: Hung-I Wang <[email protected]>
> ---
>  lib/libalpm/dload.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index 2d8b4d6d..faa3d9f2 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -338,6 +338,7 @@ static void curl_set_handle_opts(CURL *curl, struct 
> dload_payload *payload)
>       curl_easy_setopt(curl, CURLOPT_TCP_KEEPINTVL, 60L);
>       curl_easy_setopt(curl, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
>       curl_easy_setopt(curl, CURLOPT_PRIVATE, (void *)payload);
> +     curl_easy_setopt(curl, CURLOPT_FAILONERROR, 1);
>  
>       _alpm_log(handle, ALPM_LOG_DEBUG, "%s: url is %s\n",
>               payload->remote_name, payload->fileurl);
> @@ -427,7 +428,8 @@ static int curl_retry_next_server(CURLM *curlm, CURL 
> *curl, struct dload_payload
>       len = strlen(server) + strlen(payload->filepath) + 2;
>       MALLOC(payload->fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1));
>       snprintf(payload->fileurl, len, "%s/%s", server, payload->filepath);
> -
> +     _alpm_log(handle, ALPM_LOG_DEBUG, "%s: url is %s\n",
> +             payload->remote_name, payload->fileurl);
>  
>       fflush(payload->localf);
>  
> @@ -496,6 +498,7 @@ static int curl_check_finished_download(CURLM *curlm, 
> CURLMsg *msg,
>       /* was it a success? */
>       switch(curlerr) {
>               case CURLE_OK:

Can break here

> +             case CURLE_HTTP_RETURNED_ERROR:
>                       /* get http/ftp response code */
>                       _alpm_log(handle, ALPM_LOG_DEBUG, "%s: response code 
> %ld\n",
>                                       payload->remote_name, 
> payload->respcode);

and here we would no longer need to check for respcode>=400

Reply via email to