Re: [PATCH 2/3] http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION

> We have to rewrite the ioctl functions into seek functions. In some ways
> they are simpler (seeking is the only operation), but in some ways more
> complex (the ioctl allowed only a full rewind, but now we can seek to
> arbitrary offsets).
> 
> Curl will only ever use SEEK_SET (per their documentation), so I didn't
> bother implementing anything else, since it would naturally be
> completely untested. This seems unlikely to change, but I added an
> assertion just in case.
> 
> Likewise, I doubt curl will ever try to seek outside of the buffer sizes
> we've told it, but I erred on the defensive side here, rather than do an
> out-of-bounds read.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  http-push.c   |  4 ++--
>  http.c        | 20 +++++++++-----------
>  http.h        |  2 +-
>  remote-curl.c | 28 +++++++++++++---------------
>  4 files changed, 25 insertions(+), 29 deletions(-)
> 
> diff --git a/http-push.c b/http-push.c
> index 1b18e775d0..7f71316456 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -203,8 +203,8 @@ static void curl_setup_http(CURL *curl, const char *url,
>  	curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
>  	curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
>  	curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
> -	curl_easy_setopt(curl, CURLOPT_IOCTLFUNCTION, ioctl_buffer);
> -	curl_easy_setopt(curl, CURLOPT_IOCTLDATA, buffer);
> +	curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
> +	curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
>  	curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);
>  	curl_easy_setopt(curl, CURLOPT_NOBODY, 0);
>  	curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);
> diff --git a/http.c b/http.c
> index 8a5ba3f477..ca0fe80ddb 100644
> --- a/http.c
> +++ b/http.c
> @@ -157,21 +157,19 @@ size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
>  	return size / eltsize;
>  }
>  
> -curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp)
> +int seek_buffer(void *clientp, curl_off_t offset, int origin)
>  {
>  	struct buffer *buffer = clientp;
>  
> -	switch (cmd) {
> -	case CURLIOCMD_NOP:
> -		return CURLIOE_OK;
> -
> -	case CURLIOCMD_RESTARTREAD:
> -		buffer->posn = 0;
> -		return CURLIOE_OK;
> -
> -	default:
> -		return CURLIOE_UNKNOWNCMD;
> +	if (origin != SEEK_SET)
> +		BUG("seek_buffer only handles SEEK_SET");

> +
> +	buffer->posn = offset;
> +	return CURL_SEEKFUNC_OK;
>  }
>  
>  size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
> diff --git a/http.h b/http.h
> index 3c94c47910..77c042706c 100644
> --- a/http.h
> +++ b/http.h
> @@ -40,7 +40,7 @@ struct buffer {
>  size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
>  size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
>  size_t fwrite_null(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
> -curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp);
> +int seek_buffer(void *clientp, curl_off_t offset, int origin);
>  
>  /* Slot lifecycle functions */
>  struct active_request_slot *get_active_slot(void);
> diff --git a/remote-curl.c b/remote-curl.c
> index 72dfb8fb86..a76b6405eb 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -717,25 +717,23 @@ static size_t rpc_out(void *ptr, size_t eltsize,
>  	return avail;
>  }
>  
> -static curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp)
> +static int rpc_seek(void *clientp, curl_off_t offset, int origin)
>  {
>  	struct rpc_state *rpc = clientp;
>  
> -	switch (cmd) {
> -	case CURLIOCMD_NOP:
> -		return CURLIOE_OK;
> +	if (origin != SEEK_SET)
> +		BUG("rpc_seek only handles SEEK_SET, not %d", origin);
>  
> -	case CURLIOCMD_RESTARTREAD:
> -		if (rpc->initial_buffer) {
> -			rpc->pos = 0;
> -			return CURLIOE_OK;
> +	if (rpc->initial_buffer) {
> +		if (offset < 0 || offset > rpc->len) {
> +			error("curl seek would be outside of rpc buffer");
> +			return CURL_SEEKFUNC_FAIL;
>  		}
> -		error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
> -		return CURLIOE_FAILRESTART;
> -
> -	default:
> -		return CURLIOE_UNKNOWNCMD;
> +		rpc->pos = offset;
> +		return CURL_SEEKFUNC_OK;
>  	}
> +	error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
> +	return CURL_SEEKFUNC_FAIL;
>  }
>  
>  struct check_pktline_state {
> @@ -959,8 +957,8 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece
>  		rpc->initial_buffer = 1;
>  		curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);
>  		curl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc);
> -		curl_easy_setopt(slot->curl, CURLOPT_IOCTLFUNCTION, rpc_ioctl);
> -		curl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, rpc);
> +		curl_easy_setopt(slot->curl, CURLOPT_SEEKFUNCTION, rpc_seek);
> +		curl_easy_setopt(slot->curl, CURLOPT_SEEKDATA, rpc);
>  		if (options.verbosity > 1) {
>  			fprintf(stderr, "POST %s (chunked)\n", rpc->service_name);
>  			fflush(stderr);