INI Parsing Failures silently ignored

[daemonl]

Describe the bug

Config INI files with certain strings which look like numbers silently cause the config file to be entirely skipped.

E.g.

color = 970b00

Expected Behavior

Either an error should be thrown for corrupted ini files, or in this case the string should be parsed correctly.

Current Behavior

• The config file is skipped entirely, with no warning or error
• Meaning the profile specified in the $AWS_PROFILE variable isn't found
• Which also silently fails
• The SDK falls back to attempting to load EC2 IMDS credentials

Reproduction Steps

Add the color line to ~/.aws/config under any profile

[profile foobar]
color = 970b00
... existing creds ...

export AWS_PROFILE=foobar

and attempt to use that profile through config.LoadDefaultConfig

cfg, err := config.LoadDefaultConfig(ctx)
if err != nil {
return fmt.Errorf("failed to load configuration: %w", err)
}

cfClient := cloudformation.NewFromConfig(cfg)
res, err := d.CFClient.DescribeStacks(ctx, &cloudformation.DescribeStacksInput{
StackName: aws.String(stackName),
})

Possible Solution

Error is thrown here
Ignored here

func countTokens(runes []rune) int {
...
		if err != nil {
			return 0
		}

Leading to the tokenize function simply not iterating over any tokens.

Returning an error from countTokens passes it up the chain:

failed to load configuration: failed to load shared config file, [redacted], incorrect base format found b at 3 index

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

ithub.com/google/go-cmp@v0.5.8
github.com/aws/aws-sdk-go-v2/service/sso@v1.13.6 github.com/aws/aws-sdk-go-v2@v1.21.0
github.com/aws/aws-sdk-go-v2/service/sso@v1.13.6 github.com/aws/aws-sdk-go-v2/internal/configsources@v1.1.41
github.com/aws/aws-sdk-go-v2/service/sso@v1.13.6 github.com/aws/aws-sdk-go-v2/internal/endpoints/v2@v2.4.35
github.com/aws/aws-sdk-go-v2/service/sso@v1.13.6 github.com/aws/smithy-go@v1.14.2
github.com/aws/aws-sdk-go-v2/service/sso@v1.13.6 github.com/google/go-cmp@v0.5.8
github.com/aws/aws-sdk-go-v2/service/ssooidc@v1.15.6 github.com/aws/aws-sdk-go-v2@v1.21.0
github.com/aws/aws-sdk-go-v2/service/ssooidc@v1.15.6 github.com/aws/aws-sdk-go-v2/internal/configsources@v1.1.41
github.com/aws/aws-sdk-go-v2/service/ssooidc@v1.15.6 github.com/aws/aws-sdk-go-v2/internal/endpoints/v2@v2.4.35
github.com/aws/aws-sdk-go-v2/service/ssooidc@v1.15.6 github.com/aws/smithy-go@v1.14.2
github.com/aws/aws-sdk-go-v2/service/ssooidc@v1.15.6 github.com/google/go-cmp@v0.5.8
github.com/aws/aws-sdk-go-v2/service/sts@v1.21.5 github.com/aws/aws-sdk-go-v2@v1.21.0
github.com/aws/aws-sdk-go-v2/service/sts@v1.21.5 github.com/aws/aws-sdk-go-v2/internal/configsources@v1.1.41
github.com/aws/aws-sdk-go-v2/service/sts@v1.21.5 github.com/aws/aws-sdk-go-v2/internal/endpoints/v2@v2.4.35
github.com/aws/aws-sdk-go-v2/service/sts@v1.21.5 github.com/aws/aws-sdk-go-v2/service/internal/presigned-url@v1.9.35
github.com/aws/aws-sdk-go-v2/service/sts@v1.21.5 github.com/aws/smithy-go@v1.14.2
github.com/aws/aws-sdk-go-v2/service/sts@v1.21.5 github.com/google/go-cmp@v0.5.8
github.com/aws/smithy-go@v1.14.2 github.com/google/go-cmp@v0.5.8
github.com/aws/smithy-go@v1.14.2 github.com/jmespath/go-jmespath@v0.4.0

Compiler and Version used

go version go1.20.5 darwin/arm64

Operating System and version

OSX 13.5.2 (22G91)

[lucix-aws]

This is a good find- after taking a closer look, frankly I'm going to say the tokenization behavior we have today is fundamentally wrong.

Basically, in the section where we handle "literals", we first decide whether something is a number, string, or bool, and then parse that type accordingly. There are two issues with this:

• The code that decides whether some token is "numeric" is clearly wrong - it can decide that something is a number that the number parser can't actually handle
• This whole notion of parsing typed literals isn't right to begin with. They should all be treated as strings at this stage. The format of literals doesn't mean anything to the parser - it should only matter for the actual consumer of the values.

Ultimately point (1) above is moot - we need to get rid of typed literal parsing in the first place. The actual config structure we have that consumes the parsed config is responsible for determining whether the config values are actually valid.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.