-
Notifications
You must be signed in to change notification settings - Fork 45
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
Handle GSS_C_NO_OID_SET when creating sets #150
Conversation
cc @simo5 @frozencemetery can one of you test this, and or figure out a good way to write a gated unit test for it? |
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.
>>> import gssapi
>>> ntlm_mech = gssapi.OID.from_int_seq("1.3.6.1.4.1.311.2.2.10")
>>> gssapi.raw.inquire_attrs_for_mech(ntlm_mech)
InquireAttrsResult(mech_attrs=set([]), known_mech_attrs=set([]))
>>>
3323ab1
to
1e1a318
Compare
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.
One nit but otherwise LGTM
# but returning None would make the API harder to work with, | ||
# without much value) | ||
return set() | ||
|
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.
If we ever need the distinction we may need to make an incompatible API change and start returning None, is that ok ?
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.
yes, but we'll have to rev the major version. I'm curious as to what that situation might be -- you're not using this to detect errors (we have errors for that), so fundamentally, what's the difference here between "a set with no items", and "no set" from the user perspective.
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.
ok
1e1a318
to
899acc4
Compare
@DirectXMan12 are you happy with the way this is being tested, or do you want something different? |
Yeah, I'm not certain there's much more we can do, unless there's a dummy OID that we know will never have any attrs. This should be good to merge. |
899acc4
to
5e58d2e
Compare
Some methods can return GSS_C_NO_OID_SET on success, so we should handle
that in our set converter by returning the empty set.
Fixes #148