diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 8a1927a39ca..02e2fecc7e2 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -3,3 +3,5 @@ ### SDK Enhancements ### SDK Bugs +* `aws/session`: Removed typed literal parsing for config, everything is now treated as a string until a numeric value is needed. + * This resolves an issue where the contents of a profile would silently be dropped with certain values. diff --git a/aws/session/shared_config.go b/aws/session/shared_config.go index ea3ac0d0316..8f1388f9fbf 100644 --- a/aws/session/shared_config.go +++ b/aws/session/shared_config.go @@ -389,8 +389,15 @@ func (cfg *sharedConfig) setFromIniFile(profile string, file sharedConfigFile, e updateString(&cfg.Region, section, regionKey) updateString(&cfg.CustomCABundle, section, customCABundleKey) + // we're retaining a behavioral quirk with this field that existed before + // the removal of literal parsing for (aws-sdk-go-v2/#2276): + // - if the key is missing, the config field will not be set + // - if the key is set to a non-numeric, the config field will be set to 0 if section.Has(roleDurationSecondsKey) { - d := time.Duration(section.Int(roleDurationSecondsKey)) * time.Second + var d time.Duration + if v, ok := section.Int(roleDurationSecondsKey); ok { + d = time.Duration(v) * time.Second + } cfg.AssumeRoleDuration = &d } @@ -668,7 +675,10 @@ func updateBool(dst *bool, section ini.Section, key string) { if !section.Has(key) { return } - *dst = section.Bool(key) + + // retains pre-(aws-sdk-go-v2#2276) behavior where non-bool value would resolve to false + v, _ := section.Bool(key) + *dst = v } // updateBoolPtr will only update the dst with the value in the section key, @@ -677,8 +687,11 @@ func updateBoolPtr(dst **bool, section ini.Section, key string) { if !section.Has(key) { return } + + // retains pre-(aws-sdk-go-v2#2276) behavior where non-bool value would resolve to false + v, _ := section.Bool(key) *dst = new(bool) - **dst = section.Bool(key) + **dst = v } // SharedConfigLoadError is an error for the shared config file failed to load. @@ -805,7 +818,8 @@ func updateUseDualStackEndpoint(dst *endpoints.DualStackEndpointState, section i return } - if section.Bool(key) { + // retains pre-(aws-sdk-go-v2/#2276) behavior where non-bool value would resolve to false + if v, _ := section.Bool(key); v { *dst = endpoints.DualStackEndpointStateEnabled } else { *dst = endpoints.DualStackEndpointStateDisabled @@ -821,7 +835,8 @@ func updateUseFIPSEndpoint(dst *endpoints.FIPSEndpointState, section ini.Section return } - if section.Bool(key) { + // retains pre-(aws-sdk-go-v2/#2276) behavior where non-bool value would resolve to false + if v, _ := section.Bool(key); v { *dst = endpoints.FIPSEndpointStateEnabled } else { *dst = endpoints.FIPSEndpointStateDisabled diff --git a/internal/ini/ini_lexer_test.go b/internal/ini/ini_lexer_test.go index 6cf3e74852d..8259dd21b62 100644 --- a/internal/ini/ini_lexer_test.go +++ b/internal/ini/ini_lexer_test.go @@ -11,8 +11,6 @@ import ( ) func TestTokenize(t *testing.T) { - numberToken := newToken(TokenLit, []rune("123"), IntegerType) - numberToken.base = 10 cases := []struct { r io.Reader expectedTokens []Token @@ -25,7 +23,7 @@ func TestTokenize(t *testing.T) { newToken(TokenWS, []rune(" "), NoneType), newToken(TokenOp, []rune("="), NoneType), newToken(TokenWS, []rune(" "), NoneType), - numberToken, + newToken(TokenLit, []rune("123"), StringType), }, }, { diff --git a/internal/ini/literal_tokens.go b/internal/ini/literal_tokens.go index 34a481afbd4..b1b686086a9 100644 --- a/internal/ini/literal_tokens.go +++ b/internal/ini/literal_tokens.go @@ -154,11 +154,11 @@ func (v ValueType) String() string { // ValueType enums const ( NoneType = ValueType(iota) - DecimalType - IntegerType + DecimalType // deprecated + IntegerType // deprecated StringType QuotedStringType - BoolType + BoolType // deprecated ) // Value is a union container @@ -166,9 +166,9 @@ type Value struct { Type ValueType raw []rune - integer int64 - decimal float64 - boolean bool + integer int64 // deprecated + decimal float64 // deprecated + boolean bool // deprecated str string } @@ -253,24 +253,6 @@ func newLitToken(b []rune) (Token, int, error) { } token = newToken(TokenLit, b[:n], QuotedStringType) - } else if isNumberValue(b) { - var base int - base, n, err = getNumericalValue(b) - if err != nil { - return token, 0, err - } - - value := b[:n] - vType := IntegerType - if contains(value, '.') || hasExponent(value) { - vType = DecimalType - } - token = newToken(TokenLit, value, vType) - token.base = base - } else if isBoolValue(b) { - n, err = getBoolValue(b) - - token = newToken(TokenLit, b[:n], BoolType) } else { n, err = getValue(b) token = newToken(TokenLit, b[:n], StringType) @@ -280,18 +262,33 @@ func newLitToken(b []rune) (Token, int, error) { } // IntValue returns an integer value -func (v Value) IntValue() int64 { - return v.integer +func (v Value) IntValue() (int64, bool) { + i, err := strconv.ParseInt(string(v.raw), 0, 64) + if err != nil { + return 0, false + } + return i, true } // FloatValue returns a float value -func (v Value) FloatValue() float64 { - return v.decimal +func (v Value) FloatValue() (float64, bool) { + f, err := strconv.ParseFloat(string(v.raw), 64) + if err != nil { + return 0, false + } + return f, true } // BoolValue returns a bool value -func (v Value) BoolValue() bool { - return v.boolean +func (v Value) BoolValue() (bool, bool) { + // we don't use ParseBool as it recognizes more than what we've + // historically supported + if isCaselessLitValue(runesTrue, v.raw) { + return true, true + } else if isCaselessLitValue(runesFalse, v.raw) { + return false, true + } + return false, false } func isTrimmable(r rune) bool { diff --git a/internal/ini/literal_tokens_test.go b/internal/ini/literal_tokens_test.go index cbbdfd69c3a..bf6609ea6d6 100644 --- a/internal/ini/literal_tokens_test.go +++ b/internal/ini/literal_tokens_test.go @@ -95,7 +95,7 @@ func TestNewLiteralToken(t *testing.T) { expectedRead: 3, expectedToken: newToken(TokenLit, []rune("123"), - IntegerType, + StringType, ), }, { @@ -104,7 +104,7 @@ func TestNewLiteralToken(t *testing.T) { expectedRead: 7, expectedToken: newToken(TokenLit, []rune("123.456"), - DecimalType, + StringType, ), }, { @@ -113,7 +113,7 @@ func TestNewLiteralToken(t *testing.T) { expectedRead: 3, expectedToken: newToken(TokenLit, []rune("123"), - IntegerType, + StringType, ), }, { @@ -122,7 +122,7 @@ func TestNewLiteralToken(t *testing.T) { expectedRead: 3, expectedToken: newToken(TokenLit, []rune("123"), - IntegerType, + StringType, ), }, { @@ -149,7 +149,7 @@ func TestNewLiteralToken(t *testing.T) { expectedRead: 4, expectedToken: newToken(TokenLit, []rune("true"), - BoolType, + StringType, ), }, { @@ -158,21 +158,21 @@ func TestNewLiteralToken(t *testing.T) { expectedRead: 5, expectedToken: newToken(TokenLit, []rune("false"), - BoolType, + StringType, ), }, { - name: "utf8 whitespace", - b: []rune("0 0"), - expectedRead: 1, + name: "utf8 whitespace", + b: []rune("0 0"), + expectedRead: 3, expectedToken: newToken(TokenLit, []rune("0"), - IntegerType, + StringType, ), }, { - name: "utf8 whitespace expr", - b: []rune("0=0 0"), + name: "utf8 whitespace expr", + b: []rune("0=0 0"), expectedRead: 1, expectedToken: newToken(TokenLit, []rune("0"), diff --git a/internal/ini/testdata/valid/array_profile_expected b/internal/ini/testdata/valid/array_profile_expected index 95d9f6b1f65..3a5761d44a3 100644 --- a/internal/ini/testdata/valid/array_profile_expected +++ b/internal/ini/testdata/valid/array_profile_expected @@ -1,6 +1,6 @@ { "foo": { - "baz": 123, + "baz": "123", "zed": "zee" } } diff --git a/internal/ini/testdata/valid/base_numbers_profile b/internal/ini/testdata/valid/base_numbers_profile index c7ad6338e7b..b77831bff88 100644 --- a/internal/ini/testdata/valid/base_numbers_profile +++ b/internal/ini/testdata/valid/base_numbers_profile @@ -4,3 +4,4 @@ octal=0o107 ten=12 hex=0xAFB1 hex2=0xafb1 +color=970b00 diff --git a/internal/ini/testdata/valid/base_numbers_profile_expected b/internal/ini/testdata/valid/base_numbers_profile_expected index b84ac0425dd..04f92daabd9 100644 --- a/internal/ini/testdata/valid/base_numbers_profile_expected +++ b/internal/ini/testdata/valid/base_numbers_profile_expected @@ -1,9 +1,10 @@ { "default": { - "binary": 9, - "octal": 71, - "ten": 12, - "hex": 44977, - "hex2": 44977 + "binary": "0b1001", + "octal": "0o107", + "ten": "12", + "hex": "0xAFB1", + "hex2": "0xafb1", + "color": "970b00" } } diff --git a/internal/ini/testdata/valid/commented_profile b/internal/ini/testdata/valid/commented_profile index 85e7792170a..f40a405042e 100644 --- a/internal/ini/testdata/valid/commented_profile +++ b/internal/ini/testdata/valid/commented_profile @@ -2,3 +2,4 @@ [ default ] region = "foo-region" # another comment output = json # comment again +bar = 123 ; comment with semi-colon diff --git a/internal/ini/testdata/valid/commented_profile_expected b/internal/ini/testdata/valid/commented_profile_expected index dbf5571cf6f..de7b26c6d7e 100644 --- a/internal/ini/testdata/valid/commented_profile_expected +++ b/internal/ini/testdata/valid/commented_profile_expected @@ -1,6 +1,7 @@ { "default": { "region": "foo-region", - "output": "json" + "output": "json", + "bar": "123" } } diff --git a/internal/ini/testdata/valid/exponent_profile_expected b/internal/ini/testdata/valid/exponent_profile_expected index bc913318a81..cf0f9c030c6 100644 --- a/internal/ini/testdata/valid/exponent_profile_expected +++ b/internal/ini/testdata/valid/exponent_profile_expected @@ -1,7 +1,7 @@ { "default": { - "exponent": 10000, - "exponent_2": 0.0001, + "exponent": "1e4", + "exponent_2": "1E-4", "exponent_should_be_string": "0x1ob" } } diff --git a/internal/ini/testdata/valid/nested_fields_expected b/internal/ini/testdata/valid/nested_fields_expected index 362bb9c8415..0f1ae73acfc 100644 --- a/internal/ini/testdata/valid/nested_fields_expected +++ b/internal/ini/testdata/valid/nested_fields_expected @@ -12,4 +12,4 @@ "aws_secret_access_key": "valid", "aws_session_token": "valid" } -} +} \ No newline at end of file diff --git a/internal/ini/testdata/valid/number_lhs_expr_expected b/internal/ini/testdata/valid/number_lhs_expr_expected index 065736be491..edd2865e2d6 100644 --- a/internal/ini/testdata/valid/number_lhs_expr_expected +++ b/internal/ini/testdata/valid/number_lhs_expr_expected @@ -1,5 +1,5 @@ { "default": { - "123": 456.456 + "123": "456.456" } } diff --git a/internal/ini/visitor.go b/internal/ini/visitor.go index 081cf433424..1d08e138aba 100644 --- a/internal/ini/visitor.go +++ b/internal/ini/visitor.go @@ -145,17 +145,17 @@ func (t Section) ValueType(k string) (ValueType, bool) { } // Bool returns a bool value at k -func (t Section) Bool(k string) bool { +func (t Section) Bool(k string) (bool, bool) { return t.values[k].BoolValue() } // Int returns an integer value at k -func (t Section) Int(k string) int64 { +func (t Section) Int(k string) (int64, bool) { return t.values[k].IntValue() } // Float64 returns a float value at k -func (t Section) Float64(k string) float64 { +func (t Section) Float64(k string) (float64, bool) { return t.values[k].FloatValue() } diff --git a/internal/ini/walker_test.go b/internal/ini/walker_test.go index f48628dd792..cd2598f0a23 100644 --- a/internal/ini/walker_test.go +++ b/internal/ini/walker_test.go @@ -71,24 +71,6 @@ func TestValidDataFiles(t *testing.T) { if e != a { t.Errorf("%s: expected %v, but received %v for profile %v", path, e, a, profile) } - case int: - a := p.Int(k) - if int64(e) != a { - t.Errorf("%s: expected %v, but received %v", path, e, a) - } - case float64: - v := p.values[k] - if v.Type == IntegerType { - a := p.Int(k) - if int64(e) != a { - t.Errorf("%s: expected %v, but received %v", path, e, a) - } - } else { - a := p.Float64(k) - if e != a { - t.Errorf("%s: expected %v, but received %v", path, e, a) - } - } default: t.Errorf("unexpected type: %T", e) }