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.

Sign up for GitHub

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

bugfix: fix an int parsing bug in godotenv.Marshal #235

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

bugfix: fix an int parsing bug in godotenv.Marshal #235

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8e06a02
fix an int parsing bug in godotenv.Marshal
t3mp14r3 Aug 30, 2024
de05e2d
got rid of regex, added a check for negative numbers, added a test
t3mp14r3 Sep 2, 2024
2f570db
fix '-' character trimming and expand the test
t3mp14r3 Sep 2, 2024
2099107
replace unicode.IsDigit for number checking
t3mp14r3 Sep 2, 2024
bfb9a56
add the comment to isInt, run go fmt
t3mp14r3 Sep 2, 2024
f7b3a6b
add really big numbers isInt tests
t3mp14r3 Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions godotenv.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import (
"io"
"os"
"os/exec"
"regexp"
"sort"
"strconv"
"strings"
)

Expand Down Expand Up @@ -163,9 +163,10 @@ func Write(envMap map[string]string, filename string) error {
// Each line is in the format: KEY="VALUE" where VALUE is backslash-escaped.
func Marshal(envMap map[string]string) (string, error) {
lines := make([]string, 0, len(envMap))
var isInt = regexp.MustCompile(`^[0-9]+$`).MatchString
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regexp variable should be put outside the Marshall method, so at the package level.

Parsing a regexp has a cost, and here is would be done on each call to Marshall

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, you're completely right, I will deal with the possibility of negative numbers get rid of regex.

for k, v := range envMap {
if d, err := strconv.Atoi(v); err == nil {
lines = append(lines, fmt.Sprintf(`%s=%d`, k, d))
if isInt(v) {
lines = append(lines, fmt.Sprintf(`%s=%s`, k, v))
} else {
lines = append(lines, fmt.Sprintf(`%s="%s"`, k, doubleQuoteEscape(v)))
}
Expand Down
Loading