From 096ad68e8b2e3d96fec4f4eb058a5e12c078259b Mon Sep 17 00:00:00 2001 From: Luc Talatinian Date: Tue, 19 Sep 2023 13:42:36 -0400 Subject: [PATCH 1/2] fix: defer config value type assertion to load stage instead of parse --- .../87dcac43e51040c38f8e72fb9b874042.json | 9 + config/shared_config.go | 68 +++--- internal/ini/ini_lexer_test.go | 4 +- internal/ini/literal_tokens.go | 186 +++-------------- internal/ini/literal_tokens_test.go | 105 +--------- internal/ini/number_helper.go | 152 -------------- internal/ini/number_helper_test.go | 194 ------------------ .../ini/testdata/valid/array_profile_expected | 2 +- .../ini/testdata/valid/base_numbers_profile | 1 + .../valid/base_numbers_profile_expected | 11 +- .../testdata/valid/exponent_profile_expected | 4 +- .../testdata/valid/mixed_case_keys_expected | 4 +- .../testdata/valid/number_lhs_expr_expected | 2 +- internal/ini/value_util.go | 161 --------------- internal/ini/value_util_test.go | 123 ----------- internal/ini/visitor.go | 6 +- internal/ini/walker_test.go | 18 -- 17 files changed, 87 insertions(+), 963 deletions(-) create mode 100644 .changelog/87dcac43e51040c38f8e72fb9b874042.json delete mode 100644 internal/ini/number_helper.go delete mode 100644 internal/ini/number_helper_test.go diff --git a/.changelog/87dcac43e51040c38f8e72fb9b874042.json b/.changelog/87dcac43e51040c38f8e72fb9b874042.json new file mode 100644 index 00000000000..40ba40bdb82 --- /dev/null +++ b/.changelog/87dcac43e51040c38f8e72fb9b874042.json @@ -0,0 +1,9 @@ +{ + "id": "87dcac43-e510-40c3-8f8e-72fb9b874042", + "type": "bugfix", + "description": "Move type assertion of config values out of the parsing stage, which resolves an issue where the contents of a profile would silently be dropped with certain numeric formats.", + "modules": [ + "config", + "internal/ini" + ] +} \ No newline at end of file diff --git a/config/shared_config.go b/config/shared_config.go index e699194d376..ae5ba76584e 100644 --- a/config/shared_config.go +++ b/config/shared_config.go @@ -740,6 +740,8 @@ func mergeSections(dst *ini.Sections, src ini.Sections) error { defaultsModeKey, retryModeKey, caBundleKey, + roleDurationSecondsKey, + retryMaxAttemptsKey, ssoSessionNameKey, ssoAccountIDKey, @@ -753,16 +755,6 @@ func mergeSections(dst *ini.Sections, src ini.Sections) error { } } - intKeys := []string{ - roleDurationSecondsKey, - retryMaxAttemptsKey, - } - for i := range intKeys { - if err := mergeIntKey(&srcSection, &dstSection, sectionName, intKeys[i]); err != nil { - return err - } - } - // set srcSection on dst srcSection *dst = dst.SetSection(sectionName, dstSection) } @@ -789,26 +781,6 @@ func mergeStringKey(srcSection *ini.Section, dstSection *ini.Section, sectionNam return nil } -func mergeIntKey(srcSection *ini.Section, dstSection *ini.Section, sectionName, key string) error { - if srcSection.Has(key) { - srcValue := srcSection.Int(key) - v, err := ini.NewIntValue(srcValue) - if err != nil { - return fmt.Errorf("error merging %s, %w", key, err) - } - - if dstSection.Has(key) { - dstSection.Logs = append(dstSection.Logs, newMergeKeyLogMessage(sectionName, key, - dstSection.SourceFile[key], srcSection.SourceFile[key])) - - } - - dstSection.UpdateValue(key, v) - dstSection.UpdateSourceFile(key, srcSection.SourceFile[key]) - } - return nil -} - func newMergeKeyLogMessage(sectionName, key, dstSourceFile, srcSourceFile string) string { return fmt.Sprintf("For profile: %v, overriding %v value, defined in %v "+ "with a %v value found in a duplicate profile defined at file %v. \n", @@ -962,9 +934,16 @@ func (c *SharedConfig) setFromIniSection(profile string, section ini.Section) er updateString(&c.SSOAccountID, section, ssoAccountIDKey) updateString(&c.SSORoleName, section, ssoRoleNameKey) + // we're retaining a behavioral quirk with this field that existed before + // the removal of literal parsing for #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 - c.RoleDurationSeconds = &d + if v, ok := section.Int(roleDurationSecondsKey); ok { + c.RoleDurationSeconds = aws.Duration(time.Duration(v) * time.Second) + } else { + c.RoleDurationSeconds = aws.Duration(time.Duration(0)) + } } updateString(&c.CredentialProcess, section, credentialProcessKey) @@ -1314,12 +1293,13 @@ func updateInt(dst *int, section ini.Section, key string) error { if !section.Has(key) { return nil } - if vt, _ := section.ValueType(key); vt != ini.IntegerType { - return fmt.Errorf("invalid value %s=%s, expect integer", - key, section.String(key)) + v, ok := section.Int(key) + if !ok { + return fmt.Errorf("invalid value %s=%s, expect integer", key, section.String(key)) } - *dst = int(section.Int(key)) + + *dst = int(v) return nil } @@ -1329,7 +1309,10 @@ func updateBool(dst *bool, section ini.Section, key string) { if !section.Has(key) { return } - *dst = section.Bool(key) + + // retains pre-#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, @@ -1338,8 +1321,11 @@ func updateBoolPtr(dst **bool, section ini.Section, key string) { if !section.Has(key) { return } + + // retains pre-#2276 behavior where non-bool value would resolve to false + v, _ := section.Bool(key) *dst = new(bool) - **dst = section.Bool(key) + **dst = v } // updateEndpointDiscoveryType will only update the dst with the value in the section, if @@ -1371,7 +1357,8 @@ func updateUseDualStackEndpoint(dst *aws.DualStackEndpointState, section ini.Sec return } - if section.Bool(key) { + // retains pre-#2276 behavior where non-bool value would resolve to false + if v, _ := section.Bool(key); v { *dst = aws.DualStackEndpointStateEnabled } else { *dst = aws.DualStackEndpointStateDisabled @@ -1387,7 +1374,8 @@ func updateUseFIPSEndpoint(dst *aws.FIPSEndpointState, section ini.Section, key return } - if section.Bool(key) { + // retains pre-#2276 behavior where non-bool value would resolve to false + if v, _ := section.Bool(key); v { *dst = aws.FIPSEndpointStateEnabled } else { *dst = aws.FIPSEndpointStateDisabled diff --git a/internal/ini/ini_lexer_test.go b/internal/ini/ini_lexer_test.go index 8a9f99529aa..b6868f2ed3e 100644 --- a/internal/ini/ini_lexer_test.go +++ b/internal/ini/ini_lexer_test.go @@ -8,8 +8,6 @@ import ( ) func TestTokenize(t *testing.T) { - numberToken := newToken(TokenLit, []rune("123"), IntegerType) - numberToken.base = 10 cases := []struct { r io.Reader expectedTokens []Token @@ -22,7 +20,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 eca42d1b293..efcd2e6c7da 100644 --- a/internal/ini/literal_tokens.go +++ b/internal/ini/literal_tokens.go @@ -12,34 +12,6 @@ var ( runesFalse = []rune("false") ) -var literalValues = [][]rune{ - runesTrue, - runesFalse, -} - -func isBoolValue(b []rune) bool { - for _, lv := range literalValues { - if isCaselessLitValue(lv, b) { - return true - } - } - return false -} - -func isLitValue(want, have []rune) bool { - if len(have) < len(want) { - return false - } - - for i := 0; i < len(want); i++ { - if want[i] != have[i] { - return false - } - } - - return true -} - // isCaselessLitValue is a caseless value comparison, assumes want is already lower-cased for efficiency. func isCaselessLitValue(want, have []rune) bool { if len(have) < len(want) { @@ -55,68 +27,6 @@ func isCaselessLitValue(want, have []rune) bool { return true } -// isNumberValue will return whether not the leading characters in -// a byte slice is a number. A number is delimited by whitespace or -// the newline token. -// -// A number is defined to be in a binary, octal, decimal (int | float), hex format, -// or in scientific notation. -func isNumberValue(b []rune) bool { - negativeIndex := 0 - helper := numberHelper{} - needDigit := false - - for i := 0; i < len(b); i++ { - negativeIndex++ - - switch b[i] { - case '-': - if helper.IsNegative() || negativeIndex != 1 { - return false - } - helper.Determine(b[i]) - needDigit = true - continue - case 'e', 'E': - if err := helper.Determine(b[i]); err != nil { - return false - } - negativeIndex = 0 - needDigit = true - continue - case 'b': - if helper.numberFormat == hex { - break - } - fallthrough - case 'o', 'x': - needDigit = true - if i == 0 { - return false - } - - fallthrough - case '.': - if err := helper.Determine(b[i]); err != nil { - return false - } - needDigit = true - continue - } - - if i > 0 && (isNewline(b[i:]) || isWhitespace(b[i])) { - return !needDigit - } - - if !helper.CorrectByte(b[i]) { - return false - } - needDigit = false - } - - return !needDigit -} - func isValid(b []rune) (bool, int, error) { if len(b) == 0 { // TODO: should probably return an error @@ -138,14 +48,8 @@ func (v ValueType) String() string { switch v { case NoneType: return "NONE" - case DecimalType: - return "FLOAT" - case IntegerType: - return "INT" case StringType: return "STRING" - case BoolType: - return "BOOL" } return "" @@ -154,11 +58,9 @@ func (v ValueType) String() string { // ValueType enums const ( NoneType = ValueType(iota) - DecimalType - IntegerType StringType QuotedStringType - BoolType + // FUTURE(2226) MapType ) // Value is a union container @@ -166,10 +68,8 @@ type Value struct { Type ValueType raw []rune - integer int64 - decimal float64 - boolean bool - str string + str string + // FUTURE(2226) mp map[string]string } func newValue(t ValueType, base int, raw []rune) (Value, error) { @@ -177,36 +77,15 @@ func newValue(t ValueType, base int, raw []rune) (Value, error) { Type: t, raw: raw, } - var err error switch t { - case DecimalType: - v.decimal, err = strconv.ParseFloat(string(raw), 64) - case IntegerType: - if base != 10 { - raw = raw[2:] - } - - v.integer, err = strconv.ParseInt(string(raw), base, 64) case StringType: v.str = string(raw) case QuotedStringType: v.str = string(raw[1 : len(raw)-1]) - case BoolType: - v.boolean = isCaselessLitValue(runesTrue, v.raw) - } - - // issue 2253 - // - // if the value trying to be parsed is too large, then we will use - // the 'StringType' and raw value instead. - if nerr, ok := err.(*strconv.NumError); ok && nerr.Err == strconv.ErrRange { - v.Type = StringType - v.str = string(raw) - err = nil } - return v, err + return v, nil } // NewStringValue returns a Value type generated using a string input. @@ -214,24 +93,12 @@ func NewStringValue(str string) (Value, error) { return newValue(StringType, 10, []rune(str)) } -// NewIntValue returns a Value type generated using an int64 input. -func NewIntValue(i int64) (Value, error) { - v := strconv.FormatInt(i, 10) - return newValue(IntegerType, 10, []rune(v)) -} - func (v Value) String() string { switch v.Type { - case DecimalType: - return fmt.Sprintf("decimal: %f", v.decimal) - case IntegerType: - return fmt.Sprintf("integer: %d", v.integer) case StringType: return fmt.Sprintf("string: %s", string(v.raw)) case QuotedStringType: return fmt.Sprintf("quoted string: %s", string(v.raw)) - case BoolType: - return fmt.Sprintf("bool: %t", v.boolean) default: return "union not set" } @@ -249,24 +116,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) @@ -276,18 +125,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 489155a454b..928eac82b8f 100644 --- a/internal/ini/literal_tokens_test.go +++ b/internal/ini/literal_tokens_test.go @@ -5,78 +5,6 @@ import ( "testing" ) -func TestIsNumberValue(t *testing.T) { - cases := []struct { - name string - b []rune - expected bool - }{ - { - "integer", - []rune("123"), - true, - }, - { - "negative integer", - []rune("-123"), - true, - }, - { - "decimal", - []rune("123.456"), - true, - }, - { - "small e exponent", - []rune("1e234"), - true, - }, - { - "big E exponent", - []rune("1E234"), - true, - }, - { - "error case exponent base 16", - []rune("1ea4"), - false, - }, - { - "error case negative", - []rune("1-23"), - false, - }, - { - "error case multiple negative", - []rune("-1-23"), - false, - }, - { - "error case end negative", - []rune("123-"), - false, - }, - { - "error case non-number", - []rune("a"), - false, - }, - { - "utf8 whitespace", - []rune("0 0"), - true, - }, - } - - for i, c := range cases { - t.Run(c.name, func(t *testing.T) { - if e, a := c.expected, isNumberValue(c.b); e != a { - t.Errorf("%d: expected %t, but received %t", i+1, e, a) - } - }) - } -} - // TODO: test errors func TestNewLiteralToken(t *testing.T) { cases := []struct { @@ -92,7 +20,7 @@ func TestNewLiteralToken(t *testing.T) { expectedRead: 3, expectedToken: newToken(TokenLit, []rune("123"), - IntegerType, + StringType, ), }, { @@ -101,7 +29,7 @@ func TestNewLiteralToken(t *testing.T) { expectedRead: 7, expectedToken: newToken(TokenLit, []rune("123.456"), - DecimalType, + StringType, ), }, { @@ -110,7 +38,7 @@ func TestNewLiteralToken(t *testing.T) { expectedRead: 3, expectedToken: newToken(TokenLit, []rune("123"), - IntegerType, + StringType, ), }, { @@ -119,7 +47,7 @@ func TestNewLiteralToken(t *testing.T) { expectedRead: 3, expectedToken: newToken(TokenLit, []rune("123"), - IntegerType, + StringType, ), }, { @@ -146,7 +74,7 @@ func TestNewLiteralToken(t *testing.T) { expectedRead: 4, expectedToken: newToken(TokenLit, []rune("true"), - BoolType, + StringType, ), }, { @@ -155,16 +83,16 @@ func TestNewLiteralToken(t *testing.T) { expectedRead: 5, expectedToken: newToken(TokenLit, []rune("false"), - BoolType, + StringType, ), }, { name: "utf8 whitespace", b: []rune("0 0"), - expectedRead: 1, + expectedRead: 3, expectedToken: newToken(TokenLit, []rune("0"), - IntegerType, + StringType, ), }, { @@ -212,20 +140,3 @@ func TestNewStringValue(t *testing.T) { t.Errorf("expect %v string got %v", e, a) } } - -func TestNewIntValue(t *testing.T) { - const expect int64 = 1234 - - actual, err := NewIntValue(expect) - if err != nil { - t.Fatalf("expect no error, %v", err) - } - - if e, a := IntegerType, actual.Type; e != a { - t.Errorf("expect %v type got %v", e, a) - } - if e, a := expect, actual.integer; e != a { - t.Errorf("expect %v integer got %v", e, a) - } - -} diff --git a/internal/ini/number_helper.go b/internal/ini/number_helper.go deleted file mode 100644 index a45c0bc5662..00000000000 --- a/internal/ini/number_helper.go +++ /dev/null @@ -1,152 +0,0 @@ -package ini - -import ( - "bytes" - "fmt" - "strconv" -) - -const ( - none = numberFormat(iota) - binary - octal - decimal - hex - exponent -) - -type numberFormat int - -// numberHelper is used to dictate what format a number is in -// and what to do for negative values. Since -1e-4 is a valid -// number, we cannot just simply check for duplicate negatives. -type numberHelper struct { - numberFormat numberFormat - - negative bool - negativeExponent bool -} - -func (b numberHelper) Exists() bool { - return b.numberFormat != none -} - -func (b numberHelper) IsNegative() bool { - return b.negative || b.negativeExponent -} - -func (b *numberHelper) Determine(c rune) error { - if b.Exists() { - return NewParseError(fmt.Sprintf("multiple number formats: 0%v", string(c))) - } - - switch c { - case 'b': - b.numberFormat = binary - case 'o': - b.numberFormat = octal - case 'x': - b.numberFormat = hex - case 'e', 'E': - b.numberFormat = exponent - case '-': - if b.numberFormat != exponent { - b.negative = true - } else { - b.negativeExponent = true - } - case '.': - b.numberFormat = decimal - default: - return NewParseError(fmt.Sprintf("invalid number character: %v", string(c))) - } - - return nil -} - -func (b numberHelper) CorrectByte(c rune) bool { - switch { - case b.numberFormat == binary: - if !isBinaryByte(c) { - return false - } - case b.numberFormat == octal: - if !isOctalByte(c) { - return false - } - case b.numberFormat == hex: - if !isHexByte(c) { - return false - } - case b.numberFormat == decimal: - if !isDigit(c) { - return false - } - case b.numberFormat == exponent: - if !isDigit(c) { - return false - } - case b.negativeExponent: - if !isDigit(c) { - return false - } - case b.negative: - if !isDigit(c) { - return false - } - default: - if !isDigit(c) { - return false - } - } - - return true -} - -func (b numberHelper) Base() int { - switch b.numberFormat { - case binary: - return 2 - case octal: - return 8 - case hex: - return 16 - default: - return 10 - } -} - -func (b numberHelper) String() string { - buf := bytes.Buffer{} - i := 0 - - switch b.numberFormat { - case binary: - i++ - buf.WriteString(strconv.Itoa(i) + ": binary format\n") - case octal: - i++ - buf.WriteString(strconv.Itoa(i) + ": octal format\n") - case hex: - i++ - buf.WriteString(strconv.Itoa(i) + ": hex format\n") - case exponent: - i++ - buf.WriteString(strconv.Itoa(i) + ": exponent format\n") - default: - i++ - buf.WriteString(strconv.Itoa(i) + ": integer format\n") - } - - if b.negative { - i++ - buf.WriteString(strconv.Itoa(i) + ": negative format\n") - } - - if b.negativeExponent { - i++ - buf.WriteString(strconv.Itoa(i) + ": negative exponent format\n") - } - - return buf.String() -} diff --git a/internal/ini/number_helper_test.go b/internal/ini/number_helper_test.go deleted file mode 100644 index 04b3d109341..00000000000 --- a/internal/ini/number_helper_test.go +++ /dev/null @@ -1,194 +0,0 @@ -package ini - -import ( - "testing" -) - -func TestNumberHelper(t *testing.T) { - cases := []struct { - b []rune - determineIndex int - - expectedExists []bool - expectedErrors []bool - expectedCorrects []bool - expectedNegative bool - expectedBase int - }{ - { - b: []rune("-10"), - determineIndex: 0, - expectedExists: []bool{ - false, - false, - false, - }, - expectedErrors: []bool{ - false, - false, - false, - }, - expectedCorrects: []bool{ - true, - true, - true, - }, - expectedNegative: true, - expectedBase: 10, - }, - { - b: []rune("0x10"), - determineIndex: 1, - expectedExists: []bool{ - false, - false, - true, - true, - }, - expectedErrors: []bool{ - false, - false, - false, - false, - }, - expectedCorrects: []bool{ - true, - true, - true, - true, - }, - expectedNegative: false, - expectedBase: 16, - }, - { - b: []rune("0b101"), - determineIndex: 1, - expectedExists: []bool{ - false, - false, - true, - true, - true, - }, - expectedErrors: []bool{ - false, - false, - false, - false, - false, - }, - expectedCorrects: []bool{ - true, - true, - true, - true, - true, - }, - expectedNegative: false, - expectedBase: 2, - }, - { - b: []rune("0o271"), - determineIndex: 1, - expectedExists: []bool{ - false, - false, - true, - true, - true, - }, - expectedErrors: []bool{ - false, - false, - false, - false, - false, - }, - expectedCorrects: []bool{ - true, - true, - true, - true, - true, - }, - expectedNegative: false, - expectedBase: 8, - }, - { - b: []rune("99"), - determineIndex: -1, - expectedExists: []bool{ - false, - false, - }, - expectedErrors: []bool{ - false, - false, - }, - expectedCorrects: []bool{ - true, - true, - }, - expectedNegative: false, - expectedBase: 10, - }, - { - b: []rune("0o2o71"), - determineIndex: 1, - expectedExists: []bool{ - false, - false, - true, - true, - true, - true, - }, - expectedErrors: []bool{ - false, - false, - false, - false, - false, - true, - }, - expectedCorrects: []bool{ - true, - true, - true, - false, - true, - true, - }, - expectedNegative: false, - expectedBase: 8, - }, - } - - for _, c := range cases { - helper := numberHelper{} - - for i := 0; i < len(c.b); i++ { - if e, a := c.expectedExists[i], helper.Exists(); e != a { - t.Errorf("expected %t, but received %t", e, a) - } - - if i == c.determineIndex { - if e, a := c.expectedErrors[i], helper.Determine(c.b[i]) != nil; e != a { - t.Errorf("expected %t, but received %t", e, a) - } - } else { - if e, a := c.expectedCorrects[i], helper.CorrectByte(c.b[i]); e != a { - t.Errorf("expected %t, but received %t", e, a) - } - } - } - - if e, a := c.expectedNegative, helper.IsNegative(); e != a { - t.Errorf("expected %t, but received %t", e, a) - } - - if e, a := c.expectedBase, helper.Base(); e != a { - t.Errorf("expected %d, but received %d", e, a) - } - } -} 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/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/mixed_case_keys_expected b/internal/ini/testdata/valid/mixed_case_keys_expected index ec363ca7f4d..be3e13872ce 100644 --- a/internal/ini/testdata/valid/mixed_case_keys_expected +++ b/internal/ini/testdata/valid/mixed_case_keys_expected @@ -1,7 +1,7 @@ { "with_mixed_case_keys": { - "int_value": 60, + "int_value": "60", "string_value": "secret", - "float_value": 12.3 + "float_value": "12.3" } } 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/value_util.go b/internal/ini/value_util.go index b5480fdeb35..d38a9cd4857 100644 --- a/internal/ini/value_util.go +++ b/internal/ini/value_util.go @@ -41,149 +41,6 @@ func getStringValue(b []rune) (int, error) { return i + 1, nil } -// getBoolValue will return a boolean and the amount -// of bytes read -// -// an error will be returned if the boolean is not of a correct -// value -func getBoolValue(b []rune) (int, error) { - if len(b) < 4 { - return 0, NewParseError("invalid boolean value") - } - - n := 0 - for _, lv := range literalValues { - if len(lv) > len(b) { - continue - } - - if isCaselessLitValue(lv, b) { - n = len(lv) - } - } - - if n == 0 { - return 0, NewParseError("invalid boolean value") - } - - return n, nil -} - -// getNumericalValue will return a numerical string, the amount -// of bytes read, and the base of the number -// -// an error will be returned if the number is not of a correct -// value -func getNumericalValue(b []rune) (int, int, error) { - if !isDigit(b[0]) { - return 0, 0, NewParseError("invalid digit value") - } - - i := 0 - helper := numberHelper{} - -loop: - for negativeIndex := 0; i < len(b); i++ { - negativeIndex++ - - if !isDigit(b[i]) { - switch b[i] { - case '-': - if helper.IsNegative() || negativeIndex != 1 { - return 0, 0, NewParseError("parse error '-'") - } - - n := getNegativeNumber(b[i:]) - i += (n - 1) - helper.Determine(b[i]) - continue - case '.': - if err := helper.Determine(b[i]); err != nil { - return 0, 0, err - } - case 'e', 'E': - if err := helper.Determine(b[i]); err != nil { - return 0, 0, err - } - - negativeIndex = 0 - case 'b': - if helper.numberFormat == hex { - break - } - fallthrough - case 'o', 'x': - if i == 0 && b[i] != '0' { - return 0, 0, NewParseError("incorrect base format, expected leading '0'") - } - - if i != 1 { - return 0, 0, NewParseError(fmt.Sprintf("incorrect base format found %s at %d index", string(b[i]), i)) - } - - if err := helper.Determine(b[i]); err != nil { - return 0, 0, err - } - default: - if isWhitespace(b[i]) { - break loop - } - - if isNewline(b[i:]) { - break loop - } - - if !(helper.numberFormat == hex && isHexByte(b[i])) { - if i+2 < len(b) && !isNewline(b[i:i+2]) { - return 0, 0, NewParseError("invalid numerical character") - } else if !isNewline([]rune{b[i]}) { - return 0, 0, NewParseError("invalid numerical character") - } - - break loop - } - } - } - } - - return helper.Base(), i, nil -} - -// isDigit will return whether or not something is an integer -func isDigit(b rune) bool { - return b >= '0' && b <= '9' -} - -func hasExponent(v []rune) bool { - return contains(v, 'e') || contains(v, 'E') -} - -func isBinaryByte(b rune) bool { - switch b { - case '0', '1': - return true - default: - return false - } -} - -func isOctalByte(b rune) bool { - switch b { - case '0', '1', '2', '3', '4', '5', '6', '7': - return true - default: - return false - } -} - -func isHexByte(b rune) bool { - if isDigit(b) { - return true - } - return (b >= 'A' && b <= 'F') || - (b >= 'a' && b <= 'f') -} - func getValue(b []rune) (int, error) { i := 0 @@ -211,24 +68,6 @@ func getValue(b []rune) (int, error) { return i, nil } -// getNegativeNumber will return a negative number from a -// byte slice. This will iterate through all characters until -// a non-digit has been found. -func getNegativeNumber(b []rune) int { - if b[0] != '-' { - return 0 - } - - i := 1 - for ; i < len(b); i++ { - if !isDigit(b[i]) { - return i - } - } - - return i -} - // isEscaped will return whether or not the character is an escaped // character. func isEscaped(value []rune, b rune) bool { diff --git a/internal/ini/value_util_test.go b/internal/ini/value_util_test.go index ff6e2d87af0..b24aee67618 100644 --- a/internal/ini/value_util_test.go +++ b/internal/ini/value_util_test.go @@ -47,126 +47,3 @@ func TestStringValue(t *testing.T) { } } } - -func TestBoolValue(t *testing.T) { - cases := []struct { - b []rune - expectedRead int - expectedError bool - expectedValue string - }{ - { - b: []rune("true"), - expectedRead: 4, - expectedValue: "true", - }, - { - b: []rune("false"), - expectedRead: 5, - expectedValue: "false", - }, - { - b: []rune(`"false"`), - expectedError: true, - }, - { - b: []rune(`True`), - expectedRead: 4, - expectedValue: "True", - }, - { - b: []rune(`False`), - expectedRead: 5, - expectedValue: "False", - }, - } - - for _, c := range cases { - n, err := getBoolValue(c.b) - - if e, a := c.expectedValue, string(c.b[:n]); e != a { - t.Errorf("expected %v, but received %v", e, a) - } - - if e, a := c.expectedRead, n; e != a { - t.Errorf("expected %v, but received %v", e, a) - } - - if e, a := c.expectedError, err != nil; e != a { - t.Errorf("expected %v, but received %v", e, a) - } - } -} - -func TestNumericalValue(t *testing.T) { - cases := []struct { - b []rune - expectedRead int - expectedError bool - expectedValue string - expectedBase int - }{ - { - b: []rune("1.2"), - expectedRead: 3, - expectedValue: "1.2", - expectedBase: 10, - }, - { - b: []rune("123"), - expectedRead: 3, - expectedValue: "123", - expectedBase: 10, - }, - { - b: []rune("0x123A"), - expectedRead: 6, - expectedValue: "0x123A", - expectedBase: 16, - }, - { - b: []rune("0b101"), - expectedRead: 5, - expectedValue: "0b101", - expectedBase: 2, - }, - { - b: []rune("0o7"), - expectedRead: 3, - expectedValue: "0o7", - expectedBase: 8, - }, - { - b: []rune(`"123"`), - expectedError: true, - }, - { - b: []rune("0xo123"), - expectedError: true, - }, - { - b: []rune("123A"), - expectedError: true, - }, - } - - for i, c := range cases { - base, n, err := getNumericalValue(c.b) - - if e, a := c.expectedValue, string(c.b[:n]); e != a { - t.Errorf("%d: expected %v, but received %v", i+1, e, a) - } - - if e, a := c.expectedRead, n; e != a { - t.Errorf("%d: expected %v, but received %v", i+1, e, a) - } - - if e, a := c.expectedError, err != nil; e != a { - t.Errorf("%d: expected %v, but received %v", i+1, e, a) - } - - if e, a := c.expectedBase, base; e != a { - t.Errorf("%d: expected %d, but received %d", i+1, e, a) - } - } -} diff --git a/internal/ini/visitor.go b/internal/ini/visitor.go index a07a6373897..97fb3d7f3a2 100644 --- a/internal/ini/visitor.go +++ b/internal/ini/visitor.go @@ -245,17 +245,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 a59e6dc1537..02946be782b 100644 --- a/internal/ini/walker_test.go +++ b/internal/ini/walker_test.go @@ -75,24 +75,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 for profile %v", path, e, a, profile) - } - 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 for profile %v", path, e, a, profile) - } - } else { - a := p.Float64(k) - if e != a { - t.Errorf("%s: expected %v, but received %v for profile %v", path, e, a, profile) - } - } default: t.Errorf("unexpected type: %T", e) } From db4a6ac2c352da904ab8360734976f5fc5413ee4 Mon Sep 17 00:00:00 2001 From: Luc Talatinian Date: Wed, 20 Sep 2023 12:03:22 -0400 Subject: [PATCH 2/2] changelog for #2287 --- .changelog/8e58ee8fce61452eb60126004c893c2d.json | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changelog/8e58ee8fce61452eb60126004c893c2d.json diff --git a/.changelog/8e58ee8fce61452eb60126004c893c2d.json b/.changelog/8e58ee8fce61452eb60126004c893c2d.json new file mode 100644 index 00000000000..32e86f6ec90 --- /dev/null +++ b/.changelog/8e58ee8fce61452eb60126004c893c2d.json @@ -0,0 +1,9 @@ +{ + "id": "8e58ee8f-ce61-452e-b601-26004c893c2d", + "type": "bugfix", + "description": "Fixed a bug where merging `max_attempts` or `duration_seconds` fields across shared config files with invalid values would silently default them to 0.", + "modules": [ + "config", + "internal/ini" + ] +} \ No newline at end of file