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

azd env set un-escapes special characters in values #4542

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
41 changes: 39 additions & 2 deletions cli/azd/pkg/environment/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
Copy link
Contributor

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.

Copy link
Member Author

@Menghua1 Menghua1 Nov 14, 2024

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.

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
}
42 changes: 42 additions & 0 deletions cli/azd/pkg/environment/environment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"encoding/json"
"os"
"path/filepath"
"strings"
"testing"

"github.com/azure/azure-dev/cli/azd/pkg/config"
Expand Down Expand Up @@ -184,6 +185,47 @@ func TestCleanName(t *testing.T) {
require.Equal(t, "was-CLEANED-with--bad--things-(123)", CleanName("was CLEANED with *bad* things (123)"))
}

func TestMarshal_NoEscapeForSpecialChars(t *testing.T) {
envMap := map[string]string{
"EXAMPLE1": "Hello!World",
"EXAMPLE2": "Price=100$",
"EXAMPLE3": "Text`WithBacktick",
"EXAMPLE4": "NormalText",
}

result, err := Marshal(envMap)
if err != nil {
t.Fatalf("Marshal failed: %v", err)
}

tests := []struct {
key string
expected string
}{
{"EXAMPLE1", `EXAMPLE1="Hello!World"`},
{"EXAMPLE2", `EXAMPLE2="Price=100$"`},
{"EXAMPLE3", `EXAMPLE3="Text` + "`" + `WithBacktick"`},
{"EXAMPLE4", `EXAMPLE4="NormalText"`},
}

for _, tt := range tests {
t.Run(tt.key, func(t *testing.T) {
if !containsLine(result, tt.expected) {
t.Errorf("Expected line %q to be in result, but got %q", tt.expected, result)
}
})
}
}

func containsLine(result, expected string) bool {
for _, line := range strings.Split(result, "\n") {
if line == expected {
return true
}
}
return false
}

func TestRoundTripNumberWithLeadingZeros(t *testing.T) {
mockContext := mocks.NewMockContext(context.Background())
envManager, _ := createEnvManager(mockContext, t.TempDir())
Expand Down
Loading