refactor(recovery): extract Authorization header masking into maskAuthorization func #4143
Conversation
|
please resolve conflicts. |
61986d6 to
8e9ba2d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4143 +/- ##
==========================================
- Coverage 99.21% 98.92% -0.30%
==========================================
Files 42 44 +2
Lines 3182 3434 +252
==========================================
+ Hits 3157 3397 +240
- Misses 17 26 +9
- Partials 8 11 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
Extract the inline Authorization header masking logic into a dedicated maskAuthorization helper for clarity and reuse.
- Introduced a
maskAuthorizationfunction with a doc comment. - Replaced the inline loop in
CustomRecoveryWithWriterwith the new helper. - Added
TestMaskAuthorizationto validate the masking behavior.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| recovery.go | Extracted header-masking loop into maskAuthorization and added documentation |
| recovery_test.go | Added TestMaskAuthorization to ensure Authorization tokens are hidden |
Comments suppressed due to low confidence (1)
recovery_test.go:93
- Consider adding a test case with a lowercase
authorization: ...header to verify thatmaskAuthorizationhandles header names case-insensitively.
headers := []string{
recovery.go
Outdated
| func maskAuthorization(headers *[]string) { | ||
| for idx, header := range *headers { | ||
| key, _, _ := strings.Cut(header, ":") | ||
| if key == "Authorization" { | ||
| (*headers)[idx] = key + ": *" |
There was a problem hiding this comment.
[nitpick] Avoid using a pointer to a slice since slices are reference types in Go; consider using func maskAuthorization(headers []string) []string or modify the slice in-place without a pointer for clearer intent.
| func maskAuthorization(headers *[]string) { | |
| for idx, header := range *headers { | |
| key, _, _ := strings.Cut(header, ":") | |
| if key == "Authorization" { | |
| (*headers)[idx] = key + ": *" | |
| func maskAuthorization(headers []string) { | |
| for idx, header := range headers { | |
| key, _, _ := strings.Cut(header, ":") | |
| if key == "Authorization" { | |
| headers[idx] = key + ": *" |
Copilot uses AI. Check for mistakes.
recovery.go
Outdated
| func maskAuthorization(headers *[]string) { | ||
| for idx, header := range *headers { | ||
| key, _, _ := strings.Cut(header, ":") | ||
| if key == "Authorization" { |
There was a problem hiding this comment.
Comparing header keys with a case-sensitive key == "Authorization" may miss variations like authorization:; use strings.EqualFold(key, "Authorization") for case-insensitive matching.
| if key == "Authorization" { | |
| if strings.EqualFold(key, "Authorization") { |
Copilot uses AI. Check for mistakes.
8e9ba2d to
32b3414
Compare
Hi :)
Extract Authorization header masking into maskAuthorization func for better readability.