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

cookies: Improve errorhandling for reading cookiefile #8699

Closed

Conversation

danielgustafsson
Copy link
Member

The existing programming had some issues with errorhandling for reading
the cookie file. If the file failed to open, we would silently ignore it
and continue as if there was no file (or stdin) passed. In this case, we
would also call fclose() on the NULL FILE pointer, which is undefined
behavior. Fix by ensuring that the FILE pointer is set before calling
fclose on it, and error out in case the cookiefile cannot be opened.

Closes: #xxxx

@danielgustafsson
Copy link
Member Author

Erroring out on nonexisting cookiefile is too invasive to backwards compat, so let's not go there.

lib/cookie.c Outdated Show resolved Hide resolved
lib/cookie.c Outdated Show resolved Hide resolved
lib/cookie.c Outdated Show resolved Hide resolved
The existing programming had some issues with errorhandling for reading
the cookie file. If the file failed to open, we would silently ignore it
and continue as if there was no file (or stdin) passed. In this case, we
would also call fclose() on the NULL FILE pointer, which is undefined
behavior. Fix by ensuring that the FILE pointer is set before calling
fclose on it, and issue a warning in case the file cannot be opened.
Erroring out on nonexisting file would break backwards compatibility of
very old behavior so we can't really go there.

Closes: curl#8699
Reviewed-by: Jay Satiro <raysatiro@yahoo.com>
@danielgustafsson
Copy link
Member Author

@jay thanks, fixed up with your comments.

@@ -1227,7 +1230,7 @@ struct CookieInfo *Curl_cookie_init(struct Curl_easy *data,
*/
remove_expired(c);

if(fromfile)
if(fromfile && fp)
Copy link
Member

Choose a reason for hiding this comment

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

this statement is in block if(fp) so you can skip the check here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants