Skip to content
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.

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

Return CURLE_WRITE_ERROR when CURLOPT_WRITEFUNCTION callback returns CURL_WRITEFUNC_ERROR #12201

Closed
enwillyado opened this issue Oct 26, 2023 · 8 comments

Comments

@enwillyado
Copy link

enwillyado commented Oct 26, 2023

I did this

Make a CURLOPT_WRITEFUNCTION callback that returns CURL_WRITEFUNC_ERROR in order to abort the curl session.

For instance, if a large download need to be aborted without errors by user.

I expected the following

imagen

The cURL session must return CURLE_ABORTED_BY_CALLBACK.

curl/libcurl version

lib environment, last version

operating system

Unix ARM.

@icing
Copy link
Contributor

icing commented Oct 26, 2023

So, what did you see instead? What is the failure you'd like to report?

@enwillyado
Copy link
Author

enwillyado commented Oct 26, 2023

See the chop_write function:

if(wrote != chunklen) {

When a CURLOPT_WRITEFUNCTION callback called, the return size was stored in wrote variable. This function checks if the returned value was CURL_WRITEFUNC_PAUSE. If the special CURL_WRITEFUNC_ERROR return negative value is returned, a generic CURLE_WRITE_ERROR. Get the specific CURLE_ABORTED_BY_CALLBACK error is better in order to distinguish a non-error-condition status from a generic-error status when aborting curl session.

@enwillyado
Copy link
Author

Probably this report should be a suggestion to have a CURL_WRITEFUNC_ABORT special returned value, right?

@bagder
Copy link
Member

bagder commented Oct 26, 2023

I still don't understand. Your callback returns CURL_WRITEFUNC_ERROR.

What are you saying is returned and what are you saying you think should have been returned?

@enwillyado
Copy link
Author

Yes, literally if the CURLOPT_WRITEFUNCTION callback needs to return a abort, now only CURL_WRITEFUNC_ERROR can used. Then, if this special value was returned, the curl sessions ends with CURLE_WRITE_ERROR final code. This is the same final code if callbacks get an error (not memory or something like this)... in this case, by returning a amount that differs from the amount passed by curl, is enough.

So, my suggestion is: return CURLE_ABORTED_BY_CALLBACK when callback returns CURL_WRITEFUNC_ERROR, or better, make a new CURL_WRITEFUNC_ABORT special return value to use in callbacks in order to get the curl session final code CURLE_ABORTED_BY_CALLBACK.

@bagder
Copy link
Member

bagder commented Oct 26, 2023

Yes, maybe CURLE_ABORTED_BY_CALLBACK would be slightly more logical to use, but I am reluctant to change this now that it has been in use like this since 7.87.0. Applications and users have already adapted to this return code and I fear that changing it now might cause more problems than simply accepting defeat and better documenting the current state.

bagder added a commit that referenced this issue Oct 26, 2023
…UNC_ERROR

It returns CURLE_WRITE_ERROR. It was not previously stated clearly.

Reported-by: enWILLYado on github
Fixes #12201
@enwillyado
Copy link
Author

Thanks @bagder , please, consider to add a new CURL_WRITEFUNC_ABORT special return value for Callbacks in order to get a CURLE_ABORTED_BY_CALLBACK response code from curl session!

@bagder
Copy link
Member

bagder commented Oct 26, 2023

consider to add a new CURL_WRITEFUNC_ABORT special return value for Callbacks in order to get a CURLE_ABORTED_BY_CALLBACK response code from curl session!

I like this idea!

@bagder bagder closed this as completed in 910f740 Oct 26, 2023
bagder added a commit that referenced this issue Oct 26, 2023
... which makes libcurl return CURLE_ABORTED_BY_CALLBACK as error
instead of CURLE_WRITE_ERROR.

Ref: #12201
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants