-
Notifications
You must be signed in to change notification settings - Fork 214
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
azd env set
un-escapes special characters in values
#4542
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,10 +14,15 @@ import ( | |
|
||
"maps" | ||
|
||
"sort" | ||
"strconv" | ||
|
||
"github.com/azure/azure-dev/cli/azd/pkg/config" | ||
"github.com/joho/godotenv" | ||
) | ||
|
||
// DoubleQuoteSpecialChars defines the characters that need to be escaped. | ||
const doubleQuoteSpecialChars = "\\\n\r\"" | ||
|
||
// EnvNameEnvVarName is the name of the key used to store the envname property in the environment. | ||
const EnvNameEnvVarName = "AZURE_ENV_NAME" | ||
|
||
|
@@ -281,10 +286,42 @@ func fixupUnquotedDotenv(values map[string]string, dotenv string) string { | |
// Instead of calling `godotenv.Write` directly, we need to save the file ourselves, so we can fixup any numeric values | ||
// that were incorrectly unquoted. | ||
func marshallDotEnv(env *Environment) (string, error) { | ||
marshalled, err := godotenv.Marshal(env.dotenv) | ||
marshalled, err := Marshal(env.dotenv) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New added function is the same function code of latest stable version of godotenv: https://github.com/joho/godotenv/blob/v1.5.1/godotenv.go#L164. May I ask about the difference? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hemarina The original As of now, we haven't found a replace function that doesn't escape these special characters. So we overwrote the |
||
if err != nil { | ||
return "", fmt.Errorf("marshalling .env: %w", err) | ||
} | ||
|
||
return fixupUnquotedDotenv(env.dotenv, marshalled), nil | ||
} | ||
|
||
// Marshal outputs the given environment as a dotenv-formatted environment file. | ||
// 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)) | ||
for k, v := range envMap { | ||
if d, err := strconv.Atoi(v); err == nil { | ||
lines = append(lines, fmt.Sprintf(`%s=%d`, k, d)) | ||
} else { | ||
lines = append(lines, fmt.Sprintf(`%s="%s"`, k, doubleQuoteEscape(v))) | ||
} | ||
} | ||
sort.Strings(lines) | ||
return strings.Join(lines, "\n"), nil | ||
} | ||
|
||
// DoubleQuoteEscape escapes special characters in the string to ensure it is safely quoted | ||
// for a dotenv file. It escapes characters like newlines, carriage returns, | ||
// and any other special characters defined in `doubleQuoteSpecialChars` | ||
func doubleQuoteEscape(line string) string { | ||
for _, c := range doubleQuoteSpecialChars { | ||
toReplace := "\\" + string(c) | ||
if c == '\n' { | ||
toReplace = `\n` | ||
} | ||
if c == '\r' { | ||
toReplace = `\r` | ||
} | ||
line = strings.Replace(line, string(c), toReplace, -1) | ||
} | ||
return line | ||
} |
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.
Please add unit tests to verify this functionality to check for well known uses cases as well as cases that fall into this special treatment.
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.
@wbreza Unit tests have been added, please check.
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.
@wbreza We have updated by your comments, please re-review. Thanks.