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

#3726 vauth: return CURLE_OUT_OF_MEMORY only in case of out of memory #3848

Closed
wants to merge 1 commit into from

Conversation

dhoelzl
Copy link
Contributor

@dhoelzl dhoelzl commented May 6, 2019

#3726 vauth: return CURLE_OUT_OF_MEMORY only in case of out of memory; return CURLE_RECV_ERROR in case of other errors

…mory; return CURLE_RECV_ERROR in case of other errors
@kdudka
Copy link
Contributor

kdudka commented May 6, 2019

CURLE_OUT_OF_MEMORY indeed does not make any sense on certain places in the code. On the other hand, CURLE_RECV_ERROR is not always precise either. Would not CURLE_LOGIN_DENIED be closer to the nature of the failures?

@dhoelzl
Copy link
Contributor Author

dhoelzl commented May 6, 2019

You are correct. I have not checked where to use CURLE_LOGIN_DENIED instead of CURLE_RECV_ERROR.
What about CURLE_SEND_ERROR in case of processing a response succeeds, but generating the next request fails?
@kdudka could you please check this? Thank you.

@kdudka
Copy link
Contributor

kdudka commented May 6, 2019

The problem is that the failure might not be network-related at all. I think that the most common case, which #3726 is about, is just missing credential cache and/or missing Kerberos configuration, which are both local failure.

@dhoelzl
Copy link
Contributor Author

dhoelzl commented May 6, 2019

There are a lot of return codes from gssapi and sspi. Maybe there is a need for a mapping of those error codes to cURL error codes?

@jay
Copy link
Member

jay commented May 9, 2019

Maybe it would be better to have a generic CURLE_AUTH_ERROR or something? We seem to have a lot of these and recv error is not entirely correct, though I'm also of the opinion that it's better than an out of memory error.

@kdudka
Copy link
Contributor

kdudka commented May 9, 2019

@jay I agree. libcurl currently does not have any suitable error code for authentication failures triggered by local environment.

@dhoelzl dhoelzl closed this May 9, 2019
@dhoelzl
Copy link
Contributor Author

dhoelzl commented May 9, 2019

I close this PR, as the based issue has been resolved, and I currently have not the time to investigate deeper into the different error situations / error codes to distinct between CURLE_LOGIN_DENIED/CURLE_RECV_ERROR/CURLE_SEND_ERROR/CURLE_AUTH_ERROR etc. as all the cases probably need to be tested in detail on all platforms.

jay added a commit to jay/curl that referenced this pull request May 11, 2019
- Add new error code CURLE_AUTH_ERROR.

Prior to this change auth function errors were signaled by
CURLE_OUT_OF_MEMORY and CURLE_RECV_ERROR, and neither one was
technically correct.

Ref: curl#3848

Co-authored-by: Dominik Hölzl

Closes #xxxx
@lock lock bot locked as resolved and limited conversation to collaborators Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

4 participants