fix: regex repeat compile - #2358
Conversation
WalkthroughThis pull request refactors code for performance optimization and improved maintainability. Precompiled package-level regexes are introduced in two files to avoid per-call regex compilation. A mutation in json.go is removed by using a trimmed variable. The math/rand dependency is replaced with lo.RandomString, and the go.mod dependency for lo is bumped from v1.39.0 to v1.52.0. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
common/str.go (1)
15-19: Precompiled regex patterns improve performance.The package-level precompiled regex patterns are a good optimization. However, note that:
- The
maskURLPatternhas a permissive pattern[^\s/$.?#].[^\s]*that might match more than intended URLs.- The
maskIPPatterndoesn't validate IP octet ranges (e.g.,999.999.999.999would match), but this is acceptable for masking purposes as false positives will just result in extra masking.Consider tightening the URL pattern if you want more precise matching:
- maskURLPattern = regexp.MustCompile(`(http|https)://[^\s/$.?#].[^\s]*`) + maskURLPattern = regexp.MustCompile(`(https?://[^\s]+)`)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
common/json.go(1 hunks)common/str.go(4 hunks)go.mod(1 hunks)relay/common/override.go(2 hunks)
🔇 Additional comments (6)
relay/common/override.go (2)
14-15: LGTM! Precompiled regex improves performance.Introducing a package-level precompiled regex for negative index matching is a good optimization. The pattern
\.(-\d+)correctly matches negative indices in JSON paths.
190-218: LGTM! Correct usage of precompiled regex.The refactored
processNegativeIndexfunction correctly uses the precompilednegativeIndexRegexpinstead of compiling the regex on every call, which improves performance without changing functionality.common/json.go (1)
25-30: LGTM! Cleaner implementation with trimmed variable.The refactored code correctly stores the trimmed result in a variable and reuses it, which is cleaner and avoids potential multiple calls to
TrimSpace. The logic remains functionally equivalent.common/str.go (2)
176-239: LGTM! Correctly uses precompiled regex patterns.The
MaskSensitiveInfofunction now correctly uses the precompiled regex patterns (maskURLPattern,maskDomainPattern,maskIPPattern) instead of compiling them on each call. This is a good performance optimization with no functional changes.
28-33: Verify security implications of non-cryptographic randomness source.The refactored
GetRandomStringnow useslo.RandomString, which relies onmath/randrather than cryptographically secure randomness. While the defensive guard for non-positive lengths and code simplification are improvements, if this function is used for security-sensitive purposes (tokens, session IDs, security keys), consider:
- Reviewing call sites to determine if cryptographic randomness is required
- If security-sensitive, switch to
crypto/randdirectly instead oflo.RandomString- If for non-security purposes (e.g., temporary IDs, display strings), the current approach is acceptable
go.mod (1)
36-36: No compatibility concerns; v1.52.0 maintains backward compatibility.The
github.com/samber/lolibrary maintains SemVer guarantees with no breaking changes before v2.0.0. BothRandomStringandAlphanumericCharsetAPIs are available and stable in v1.52.0, making this a safe upgrade within the v1.x series.Likely an incorrect or invalid review comment.
…ompile fix: regex repeat compile
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.