-
Notifications
You must be signed in to change notification settings - Fork 265
fix: panic username not set for token #461
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
Conversation
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.
Pull Request Overview
This PR fixes a panic that occurs when parsing usernames for API token authentication in the Proxmox client. The issue was caused by attempting to parse a raw username when using an API token, which has a different format than regular usernames.
- Replaces
strings.SplitNwithstrings.IndexRunefor more robust parsing - Adds conditional logic to handle both username and API token scenarios
- Extracts user and realm information from API tokens when username is not set
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| var user UserID | ||
| if c.Username != "" { | ||
| index := strings.IndexRune(c.Username, '@') | ||
| user = UserID{Name: c.Username[:index], Realm: c.Username[index+1:]} |
Copilot
AI
Sep 17, 2025
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 '@' is not found in c.Username, IndexRune returns -1, which will cause a panic when used for slicing. Add a check to ensure index >= 0 before slicing.
| user = UserID{Name: c.Username[:index], Realm: c.Username[index+1:]} | |
| if index >= 0 { | |
| user = UserID{Name: c.Username[:index], Realm: c.Username[index+1:]} | |
| } else { | |
| // Handle missing '@' gracefully; fallback to entire username as Name, empty Realm | |
| user = UserID{Name: c.Username, Realm: ""} | |
| } |
| indexEx := strings.IndexRune(token[indexAt+1:], '!') | ||
| user = UserID{Name: token[:indexAt], Realm: token[indexAt+1 : indexAt+indexEx+1]} |
Copilot
AI
Sep 17, 2025
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 '@' or '!' characters are not found in the token, IndexRune returns -1, which will cause panics during slicing operations. Add bounds checking for both indexAt and indexEx before using them for slicing.
| indexEx := strings.IndexRune(token[indexAt+1:], '!') | |
| user = UserID{Name: token[:indexAt], Realm: token[indexAt+1 : indexAt+indexEx+1]} | |
| if indexAt == -1 { | |
| user = UserID{Name: "", Realm: ""} | |
| } else { | |
| indexEx := strings.IndexRune(token[indexAt+1:], '!') | |
| if indexEx == -1 { | |
| user = UserID{Name: "", Realm: ""} | |
| } else { | |
| user = UserID{Name: token[:indexAt], Realm: token[indexAt+1 : indexAt+indexEx+1]} | |
| } | |
| } |
Fix issue where we parsed the raw username when using an API token, causing a panic.