From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp32.i.mail.ru (smtp32.i.mail.ru [94.100.177.92]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 22CF8430D57 for ; Tue, 29 Oct 2019 02:43:06 +0300 (MSK) Date: Tue, 29 Oct 2019 02:42:57 +0300 From: Alexander Turenko Message-ID: <20191028234257.xsz5buykfkdkufj5@tkn_work_nb> References: <9cdb220e73e142d85ef536c0b27a20180f8f7088.1572282336.git.i.kosarev@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <9cdb220e73e142d85ef536c0b27a20180f8f7088.1572282336.git.i.kosarev@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v4 4/4] http: add CURLE_BAD_CONTENT_ENCODING case for curl_request code List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ilya Kosarev Cc: tarantool-patches@freelists.org, tarantool-patches@dev.tarantool.org The code LGTM; see several comments about the description below. WBR, Alexander Turenko. > http: add CURLE_BAD_CONTENT_ENCODING case for curl_request code I would write something like that as the commit header: | httpc: handle bad Content-Encoding with curl-7.67.0+ It give a reader the reason why we doing this; also it simplify schedule the patch to proper branches; now we see that it is needed to support more broad libcurl version range. > Currently in case of unknown encoding curl returns CURLE_WRITE_ERROR > request code. Since curl/curl#4449 CURLE_BAD_CONTENT_ENCODING request code Fit a commit message body in 72 symbols. > will be returned in this case. Therefore mentioned case has to be > added into switch clause processing curl request code. How about such description? | libcurl-7.66.0 and older returns CURLE_WRITE_ERROR when a server | responds with unknown or not supported Content-Encoding (see [1] and | [2]). This was fixed in future libcurl-7.67.0 and proper | CURLE_BAD_CONTENT_ENCODING code will be returned in the case. | | We should process the code in the same way as we do for | CURLE_WRITE_ERROR. | | [1]: issue link | [2]: PR link | | Closes #4579 > > Closes #4579 > --- > src/httpc.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/httpc.c b/src/httpc.c > index b493a8498..d3516049e 100644 > --- a/src/httpc.c > +++ b/src/httpc.c > @@ -447,6 +447,7 @@ httpc_execute(struct httpc_request *req, double timeout) > case CURLE_COULDNT_RESOLVE_HOST: > case CURLE_COULDNT_CONNECT: > case CURLE_WRITE_ERROR: > + case CURLE_BAD_CONTENT_ENCODING: > /* 595 Connection Problem (AnyEvent non-standard) */ > req->status = 595; > req->reason = curl_easy_strerror(req->curl_request.code); > -- > 2.17.1 >