Wrap JWKSet parsing errors in InvalidJWKValue#379
Merged
Conversation
Moved the dictionary iteration and key creation logic inside the try-except block. This ensures that any exceptions raised during the instantiation of individual JWK objects or validation checks are properly caught and safely re- raised as an InvalidJWKValue exception, rather than leaking unhandled errors. Assisted-by: Gemini <gemini@google.com> Signed-off-by: Simo Sorce <simo@redhat.com>
Collaborator
|
LGTM. I'll give some time for reporter to take a look at this before merging. |
rjeffman
approved these changes
May 5, 2026
huwcbjones
reviewed
May 6, 2026
Comment on lines
+1359
to
+1367
| if 'keys' not in jwkset: | ||
| raise ValueError("'keys' not in set") | ||
|
|
||
| for k, v in jwkset.items(): | ||
| if k == 'keys': | ||
| for jwk in v: | ||
| self['keys'].add(JWK(**jwk)) | ||
| else: | ||
| self[k] = v |
There was a problem hiding this comment.
If we're changing this around, can't we also improve it?
Something like:
Suggested change
| if 'keys' not in jwkset: | |
| raise ValueError("'keys' not in set") | |
| for k, v in jwkset.items(): | |
| if k == 'keys': | |
| for jwk in v: | |
| self['keys'].add(JWK(**jwk)) | |
| else: | |
| self[k] = v | |
| self["keys"].update(JWK(**jwk) for jwk in jwkset.pop("keys")) | |
| self.update(jwkset) |
We don't need to raise a ValueError, because we'll get a KeyError in pop("keys") that'll be caught in the except block and re-raised.
Member
Author
There was a problem hiding this comment.
I prefer to keep things readable, and I can't drop the 'keys' check as then I would miss the case when jwkset is an empty dict, which is also invalid.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Moved the dictionary iteration and key creation logic inside the try-except block. This ensures that any exceptions raised during the instantiation of individual JWK objects or validation checks are properly caught and safely re- raised as an InvalidJWKValue exception, rather than leaking unhandled errors.
Fixes: #378