fix: construct missing-claims error before emitting event to avoid nil pointer panic - #4593
fix: construct missing-claims error before emitting event to avoid nil pointer panic#4593waterWang wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe JWT tokenizer now constructs the missing-claims bad-request error before emitting the Jsonnet failure event. It returns that same error directly instead of wrapping it again. ChangesTokenizer error handling
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This localized change prevents a nil-pointer panic and preserves the intended controlled 400 response for missing JWT claims. No actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
Related issue
Closes #4585
What
On the JWT session-tokenization path, a Jsonnet claims mapper whose output has no object-valued
claimsfield is supposed to produce a controlledErrBadRequest. Instead, the success path ofEvaluateAnonymousSnippetleaveserrnil, and the missing-claims branch passes that nil error toNewJsonnetMappingFailed→attrErrorReason→reasonForError→err.Error()panics.Fix
Construct the bad-request error before emitting the event and pass the (non-nil) error to
NewJsonnetMappingFailed, returning the same error. This keeps the intended controlled 400 path intact.Checklist
Summary by CodeRabbit