Skip to content

Commit

Permalink
Merge pull request #4994 from aws/fix-parseconfig
Browse files Browse the repository at this point in the history
backport aws-sdk-go-v2/#2286 to v1
  • Loading branch information
lucix-aws authored Sep 21, 2023
2 parents 9401136 + 0508d4b commit 70cfeb8
Show file tree
Hide file tree
Showing 15 changed files with 80 additions and 82 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG_PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
25 changes: 20 additions & 5 deletions aws/session/shared_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 1 addition & 3 deletions internal/ini/ini_lexer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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),
},
},
{
Expand Down
57 changes: 27 additions & 30 deletions internal/ini/literal_tokens.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,21 +154,21 @@ 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
type Value struct {
Type ValueType
raw []rune

integer int64
decimal float64
boolean bool
integer int64 // deprecated
decimal float64 // deprecated
boolean bool // deprecated
str string
}

Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
24 changes: 12 additions & 12 deletions internal/ini/literal_tokens_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func TestNewLiteralToken(t *testing.T) {
expectedRead: 3,
expectedToken: newToken(TokenLit,
[]rune("123"),
IntegerType,
StringType,
),
},
{
Expand All @@ -104,7 +104,7 @@ func TestNewLiteralToken(t *testing.T) {
expectedRead: 7,
expectedToken: newToken(TokenLit,
[]rune("123.456"),
DecimalType,
StringType,
),
},
{
Expand All @@ -113,7 +113,7 @@ func TestNewLiteralToken(t *testing.T) {
expectedRead: 3,
expectedToken: newToken(TokenLit,
[]rune("123"),
IntegerType,
StringType,
),
},
{
Expand All @@ -122,7 +122,7 @@ func TestNewLiteralToken(t *testing.T) {
expectedRead: 3,
expectedToken: newToken(TokenLit,
[]rune("123"),
IntegerType,
StringType,
),
},
{
Expand All @@ -149,7 +149,7 @@ func TestNewLiteralToken(t *testing.T) {
expectedRead: 4,
expectedToken: newToken(TokenLit,
[]rune("true"),
BoolType,
StringType,
),
},
{
Expand All @@ -158,21 +158,21 @@ func TestNewLiteralToken(t *testing.T) {
expectedRead: 5,
expectedToken: newToken(TokenLit,
[]rune("false"),
BoolType,
StringType,
),
},
{
name: "utf8 whitespace",
b: []rune("00"),
expectedRead: 1,
name: "utf8 whitespace",
b: []rune("00"),
expectedRead: 3,
expectedToken: newToken(TokenLit,
[]rune("0"),
IntegerType,
StringType,
),
},
{
name: "utf8 whitespace expr",
b: []rune("0=00"),
name: "utf8 whitespace expr",
b: []rune("0=00"),
expectedRead: 1,
expectedToken: newToken(TokenLit,
[]rune("0"),
Expand Down
2 changes: 1 addition & 1 deletion internal/ini/testdata/valid/array_profile_expected
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"foo": {
"baz": 123,
"baz": "123",
"zed": "zee"
}
}
1 change: 1 addition & 0 deletions internal/ini/testdata/valid/base_numbers_profile
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ octal=0o107
ten=12
hex=0xAFB1
hex2=0xafb1
color=970b00
11 changes: 6 additions & 5 deletions internal/ini/testdata/valid/base_numbers_profile_expected
Original file line number Diff line number Diff line change
@@ -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"
}
}
1 change: 1 addition & 0 deletions internal/ini/testdata/valid/commented_profile
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
[ default ]
region = "foo-region" # another comment
output = json # comment again
bar = 123 ; comment with semi-colon
3 changes: 2 additions & 1 deletion internal/ini/testdata/valid/commented_profile_expected
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"default": {
"region": "foo-region",
"output": "json"
"output": "json",
"bar": "123"
}
}
4 changes: 2 additions & 2 deletions internal/ini/testdata/valid/exponent_profile_expected
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"default": {
"exponent": 10000,
"exponent_2": 0.0001,
"exponent": "1e4",
"exponent_2": "1E-4",
"exponent_should_be_string": "0x1ob"
}
}
2 changes: 1 addition & 1 deletion internal/ini/testdata/valid/nested_fields_expected
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@
"aws_secret_access_key": "valid",
"aws_session_token": "valid"
}
}
}
2 changes: 1 addition & 1 deletion internal/ini/testdata/valid/number_lhs_expr_expected
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"default": {
"123": 456.456
"123": "456.456"
}
}
6 changes: 3 additions & 3 deletions internal/ini/visitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
18 changes: 0 additions & 18 deletions internal/ini/walker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit 70cfeb8

Please sign in to comment.