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
Add missing goto error #7825
Add missing goto error #7825
Conversation
This should be correct except that there is a bigger problem. There are a number of places in the hyper handling code where where goto error and result can be CURLE_OK. I think it should probably end like this goto cleanup; edit: on second thought, shouldn't the handshake and other things only be freed on error? does that code currently work? |
@@ -805,7 +805,7 @@ static CURLcode CONNECT(struct Curl_easy *data, | |||
if(hyper_request_set_uri(req, (uint8_t *)hostheader, | |||
strlen(hostheader))) { | |||
failf(data, "error setting path"); | |||
result = CURLE_OUT_OF_MEMORY; | |||
goto error; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should still assign result
first before the goto, right? Otherwise there's no telling what the error is!
Then those should be fixed to make sure they pass on the correct error code methinks! |
They run lots of tests fine in the CI and I've done repeated torture tests, even if they are a bit more limited when hyper is used they haven't triggered problems recently. |
IMO
but I think it's better to put one check at the label so at least there is a default error, and also that way if a result is missed we don't end up returning CURLE_OK on error |
For every 'goto error', make sure the result variable holds the error code for what went wrong. Reported-by: Rafał Mikrut Fixes #7825
I made a larger take at this problem in #7846 |
This PR adds missing
goto error
Without this, failure was not handled.
result
was set toCURLE_OUT_OF_MEMORY
, but in line 811 it was changed to somethings else, so it was uselessLooks that was copy paste bug from:
curl/lib/c-hyper.c
Lines 587 to 591 in e8c8775