Skip to content
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

pkg/utils: GetBBProject should hoist out the static string built regex as a global and invoke panics not os.Exit(1) #96

Open
odeke-em opened this issue Dec 4, 2023 · 0 comments
Labels
good first issue Good for newcomers

Comments

@odeke-em
Copy link

odeke-em commented Dec 4, 2023

From Orijtech Cyber's audits of earlybird, we noticed that this code could be hoisted out and made a global given that the regular expression is constructed from a constant/static string per

func GetBBProject(bbURL string) (project string) {
re := regexp.MustCompile(`(?:projects/)([^/]*)`)

to be made

diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go
index 18eb9ad..87dc4b3 100644
--- a/pkg/utils/utils.go
+++ b/pkg/utils/utils.go
@@ -158,10 +158,11 @@ func GetGitRepo(gitURL string) (repository string) {
 	return repository
 }
 
+var reProject = regexp.MustCompile(`(?:projects/)([^/]*)`)
+
 // GetBBProject Parse project name from bitbucket URL
 func GetBBProject(bbURL string) (project string) {
-	re := regexp.MustCompile(`(?:projects/)([^/]*)`)
-	results := re.FindStringSubmatch(bbURL) // Match second capture group, 1 = project/XXX, 2 = XXX
+	results := reProject.FindStringSubmatch(bbURL) // Match second capture group, 1 = project/XXX, 2 = XXX
 	if len(results) < 1 {
 		log.Println("Failed To Get BB Project from URL:", bbURL)
 		os.Exit(1)

Benefits

Please read more at https://cyber.orijtech.com/scsec/cosmos-go-coding-guide#construct-regexps-once and see a real-world benchmark result

$ benchstat before.txt after.txt 
name              old time/op    new time/op    delta
RegexpCompiled-8    3.00µs ±20%    1.84µs ± 1%  -38.75%  (p=0.000 n=10+9)

name              old alloc/op   new alloc/op   delta
RegexpCompiled-8    1.13kB ± 0%    0.25kB ± 0%  -77.94%  (p=0.000 n=10+10)

name              old allocs/op  new allocs/op  delta
RegexpCompiled-8      16.0 ± 0%       6.0 ± 0%  -62.50%  (p=0.000 n=10+10)

Future proofing

Please install into your CI/CD workflows, our tool staticmajor https://github.com/marketplace/actions/staticmajor-analyzer which will report such issues beforehand.

/cc @elias-orijtech @madflojo

@grinish21 grinish21 added the good first issue Good for newcomers label Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants