Skip to content

Fix curl_easy_setopt() parameter type problem, again #1974

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Fix curl_easy_setopt() parameter type problem, again #1974

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
114eb9c
http: offer to cast `size_t` to `curl_off_t` safely
dscho Sep 21, 2025
fdd1327
imap-send: be more careful when casting to `curl_off_t`
dscho Sep 21, 2025
1407556
http-push: avoid new compile error
dscho Sep 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion http-push.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ static void curl_setup_http(CURL *curl, const char *url,
curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <[email protected]>
writes:

> However, there is a much better solution: To use the
> `CURLOPT_INFILESIZE_LARGE` option instead, which was added in cURL
> v7.11.0 (see https://curl.se/ch/7.11.0.html) and which Git _already_
> uses in `curl_append_msgs_to_imap()`.
>
> This fix was the motivation for renaming `xcurl_off_t()` to
> `cast_size_t_to_curl_off_t()` and making it available more broadly,
> which is the reason why it is used here, too.
>
> Signed-off-by: Johannes Schindelin <[email protected]>
> ---
>  http-push.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

As we saw in [1/3], we have been using "size large" for posting in
remote-curl.c and this brings how the size for reading the data is
handled to match that.

Looking good.  

Will queue.  Thanks.



> diff --git a/http-push.c b/http-push.c
> index 91a5465afb..7a9b96a6d0 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -208,7 +208,8 @@ static void curl_setup_http(CURL *curl, const char *url,
>  	curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
>  	curl_easy_setopt(curl, CURLOPT_URL, url);
>  	curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
> -	curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
> +	curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE,
> +			 cast_size_t_to_curl_off_t(buffer->buf.len));
>  	curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
>  	curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
>  	curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);

curl_easy_setopt(curl, CURLOPT_URL, url);
curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE,
cast_size_t_to_curl_off_t(buffer->buf.len));
curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
Expand Down
10 changes: 10 additions & 0 deletions http.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ struct packed_git;
#include <curl/curl.h>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <[email protected]>
writes:

> From: Johannes Schindelin <[email protected]>
>
> This commit moves the `xcurl_off_t()` function, which validates that a
> given value fits within the `curl_off_t` data type and then casts it, to
> a more central place so that it can be used outside of `remote-curl.c`,
> too.
>
> At the same time, this function is renamed to conform better with the
> naming convention of the helper functions that safely cast from one data
> type to another which has been well established in `git-compat-util.h`.

OK.  The code inside the renamed function is the same with an
updated message to show the value.

> With this move, the error message can unfortunately no longer be renamed
> because the `_(...)` function is not available at the time of
> definition.

It is not clear to me what change (or lack thereof?) in this patch
this paragraph refers to.  Who wants to rename what error message
and why?

> Signed-off-by: Johannes Schindelin <[email protected]>
> ---
>  http.h        | 10 ++++++++++
>  remote-curl.c | 14 +++-----------
>  2 files changed, 13 insertions(+), 11 deletions(-)

Thanks; will queue.

> +static inline curl_off_t cast_size_t_to_curl_off_t(size_t a)
> +{
> +	uintmax_t size = a;
> +	if (size > maximum_signed_value_of_type(curl_off_t))
> +		die(_("number too large to represent as curl_off_t "
> +		      "on this platform: %"PRIuMAX), (uintmax_t)a);
> +	return (curl_off_t)a;
> +}
> +
> -static curl_off_t xcurl_off_t(size_t len)
> -{
> -	uintmax_t size = len;
> -	if (size > maximum_signed_value_of_type(curl_off_t))
> -		die(_("cannot handle pushes this big"));
> -	return (curl_off_t)size;
> -}

#include <curl/easy.h>

#include "gettext.h"
#include "strbuf.h"
#include "remote.h"

Expand Down Expand Up @@ -95,6 +96,15 @@ static inline int missing__target(int code, int result)

#define missing_target(a) missing__target((a)->http_code, (a)->curl_result)

static inline curl_off_t cast_size_t_to_curl_off_t(size_t a)
{
uintmax_t size = a;
if (size > maximum_signed_value_of_type(curl_off_t))
die(_("number too large to represent as curl_off_t "
"on this platform: %"PRIuMAX), (uintmax_t)a);
return (curl_off_t)a;
}

/*
* Normalize curl results to handle CURL_FAILONERROR (or lack thereof). Failing
* http codes have their "result" converted to CURLE_HTTP_RETURNED_ERROR, and
Expand Down
2 changes: 1 addition & 1 deletion imap-send.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1721,7 +1721,7 @@ static int curl_append_msgs_to_imap(struct imap_server_conf *server,
lf_to_crlf(&msgbuf.buf);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <[email protected]>
writes:

> From: Johannes Schindelin <[email protected]>
>
> When casting a `size_t` to `curl_off_t`, there is a currently uncommon
> chance that the value can be cut off (`curl_off_t` is supposed to be
> guaranteed to be 64-bit).

"64-bit" -> "signed 64-bit", per what <curl/system.h> says,
i.e. "curl_off_t MUST be typedef'ed to a 64-bit * wide signed
integral data type.", perhaps?

msgbuf.buf is a strbuf, so its .len member is of size_t (so is
prev_len); prev_len is how big the msgbuf.buf was before the code
added some stuff taken from all_msgs, possibly wrapping the result
in html, and converting lf to crlf, all only enlarging the buffer.
We are computing how much we bloated the msgbuf.buf relative to the
original here, so this subtraction should not suffer unsigned
wraparound.  Now thanks to the previous step, we can consistently
and safely cast size_t values to curl_off_t easily, which is nice.

Looking good.  Will queue.  Thanks.

> diff --git a/imap-send.c b/imap-send.c
> index 4bd5b8aa0d..26dda7f328 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -1721,7 +1721,7 @@ static int curl_append_msgs_to_imap(struct imap_server_conf *server,
>  		lf_to_crlf(&msgbuf.buf);
>  
>  		curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE,
> -				 (curl_off_t)(msgbuf.buf.len-prev_len));
> +				 cast_size_t_to_curl_off_t(msgbuf.buf.len-prev_len));
>  
>  		res = curl_easy_perform(curl);


curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE,
(curl_off_t)(msgbuf.buf.len-prev_len));
cast_size_t_to_curl_off_t(msgbuf.buf.len-prev_len));

res = curl_easy_perform(curl);

Expand Down
14 changes: 3 additions & 11 deletions remote-curl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -894,14 +894,6 @@ static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)
return err;
}

static curl_off_t xcurl_off_t(size_t len)
{
uintmax_t size = len;
if (size > maximum_signed_value_of_type(curl_off_t))
die(_("cannot handle pushes this big"));
return (curl_off_t)size;
}

/*
* If flush_received is true, do not attempt to read any more; just use what's
* in rpc->buf.
Expand Down Expand Up @@ -999,7 +991,7 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece
* and we just need to send it.
*/
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, gzip_body);
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, xcurl_off_t(gzip_size));
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, cast_size_t_to_curl_off_t(gzip_size));

} else if (use_gzip && 1024 < rpc->len) {
/* The client backend isn't giving us compressed data so
Expand Down Expand Up @@ -1030,7 +1022,7 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece

headers = curl_slist_append(headers, "Content-Encoding: gzip");
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, gzip_body);
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, xcurl_off_t(gzip_size));
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, cast_size_t_to_curl_off_t(gzip_size));

if (options.verbosity > 1) {
fprintf(stderr, "POST %s (gzip %lu to %lu bytes)\n",
Expand All @@ -1043,7 +1035,7 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece
* more normal Content-Length approach.
*/
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, rpc->buf);
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, xcurl_off_t(rpc->len));
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, cast_size_t_to_curl_off_t(rpc->len));
if (options.verbosity > 1) {
fprintf(stderr, "POST %s (%lu bytes)\n",
rpc->service_name, (unsigned long)rpc->len);
Expand Down
Loading