Skip to content
New issue

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

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

Already on GitHub? # to your account

Have krb5.init_context() raise an exception on error. #22

Merged

Conversation

pseudometric
Copy link
Contributor

krb5.init_context() currently just returns a null pointer if the C routine fails, which is unlike the norm in this module and I assume is an oversight. Typically this will fail if there's a syntax error in the libkrb5 configuration (/etc/krb5.conf, or whatever is read). A program using pykrb5 that I wrote segfaulted in this situation, because I then passed the null context to other routines. With this change, we get instead:

In [2]: ctx = krb5.init_context()
---------------------------------------------------------------------------
Krb5Error                                 Traceback (most recent call last)
...
Krb5Error: Included profile file could not be read -1429577697

init_context() currently just returns a null pointer if the C routine
fails, which is unlike the norm in this module and I assume is an
oversight. Typically this will fail if there's a syntax error in the
libkrb5 configuration (/etc/krb5.conf, or whatever is read). A program
I wrote segfaulted in this situation, because I then passed the null
context to other routines. With this change, we get instead:

In [2]: ctx = krb5.init_context()
---------------------------------------------------------------------------
Krb5Error                                 Traceback (most recent call last)
...
Krb5Error: Included profile file could not be read -1429577697
@pseudometric pseudometric changed the title Have krb5.init_context() raise an exception on error. Have krb5.init_context() raise an exception on error. Nov 16, 2022
@jborean93
Copy link
Owner

I assume is an oversight

Yea I have no idea why I didn't do this but thank you for picking this up and fixing this.

@jborean93 jborean93 merged commit 2e5284a into jborean93:main Nov 16, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants