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

backport aws-sdk-go-v2/#2286 to v1 #4994

Merged
merged 1 commit into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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("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"),
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