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

Prevent GSSError/_display_status() infinite recursion #112

Merged

Conversation

frozencemetery
Copy link
Member

I was unable to reproduce the problem, but this should prevent the issue (which is #111).

@frozencemetery
Copy link
Member Author

(Pulled some of the prep commits out of #109)

@tiran
Copy link
Contributor

tiran commented Mar 27, 2017

I'm not a fan of assert() for flow control. Some flags (e.g. -O) remove assert statements.

@frozencemetery
Copy link
Member Author

frozencemetery commented Mar 27, 2017 via email

@tiran
Copy link
Contributor

tiran commented Apr 4, 2017

Any other exception will do. I like ValueError for an invalid value. You can subclass ValueError if you prefer to catch just that specific exception.

@frozencemetery
Copy link
Member Author

Switched to ValueError. (Commit order changed too, but that's not important.)

Copy link
Contributor

@simo5 simo5 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@DirectXMan12 DirectXMan12 left a comment

Choose a reason for hiding this comment

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

small change, but otherwise looks good

# but obviates infinite recursion into stack exhaustion. The
# exception raised here is handled by get_all_statuses(), which prints
# the code.
raise ValueError("gss_display_status call returned failure.")
Copy link
Member

Choose a reason for hiding this comment

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

please attach the error codes from gss_display_status into this message

I was unable to reproduce the problem, but this should prevent the issue.

Resolves: pythongssapi#111
@frozencemetery
Copy link
Member Author

Updated to include gss_display_status errors (for future debugging) as per request.

@frozencemetery frozencemetery merged commit b7e6c6c into pythongssapi:master Apr 4, 2017
@frozencemetery frozencemetery deleted the bug/display_forever branch April 11, 2017 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants