aws / aws-sdk-ruby

The official AWS SDK for Ruby.
https://aws.amazon.com/sdk-for-ruby/
Apache License 2.0
3.56k stars 1.22k forks source link

Fix INI parsing of whitespace preceding config key #3058

Closed jordanrmerrick closed 2 months ago

jordanrmerrick commented 3 months ago

Describe the bug

Whitespace before a key, specifically a key after a profile in the config file, causes the ini parser to fail. Per the TOML spec, whitespace around (before and after) keys and values should be ignores.

This profile works correctly:

[profile my-profile]
output = json
region = us-west-2
...

This profile fails with aws-sdk-core-3.197.0/lib/aws-sdk-core/ini_parser.rb:28:inblock in ini_parse': undefined method []' for nil:NilClass (NoMethodError):

[profile my-profile]
    output = json
region = us-west-2

Expected Behavior

A profile with whitespace before a key should be parsed correctly.

[profile my-profile]
    output = json
region = us-west-2

Should be parsed into

profile=my-profile
output=json
region=us-west-2

Current Behavior

A profile with whitespace before a key is not parsed correctly.

 [profile my-profile]
 output = json
 region = us-west-2

Results in the error

aws-sdk-core-3.197.0/lib/aws-sdk-core/ini_parser.rb:28:in `block in ini_parse': undefined method `[]' for nil:NilClass (NoMethodError)

Reproduction Steps

  1. Add a profile to your config file, typically found at $HOME/.aws/config. It should contain whitespace before the first key, e.g.

    [profile my-profile]
        output = json
    region = us-west-2
  2. Try to call IniParser::ini_parse with the file's contents

    IniParser::ini_parse(File.read("<your-config-path>"))

Possible Solution

The item and prefix regex patterns can be modified to accommodate whitespace before keys.

item = line.match(/^(.+?)\s*=\s*(.+?)\s*$/)
prefix = line.match(/^(.+?)\s*=\s*$/)

Can be modified to

item = line.match(/^\s*(.+?)\s*=\s*(.+?)\s*$/)
prefix = line.match(/^\s*(.+?)\s*=\s*$/)

Additional Information/Context

No response

Gem name ('aws-sdk', 'aws-sdk-resources' or service gems like 'aws-sdk-s3') and its version

aws-sdk-core 3.197

Environment details (Version of Ruby, OS environment)

Ruby 2.7, MacOS

jterapin commented 3 months ago

Thank you for submitting the ticket. We will be taking a look.

jordanrmerrick commented 3 months ago

I don't actually think this is so simple, I didn't consider nested configurations (i.e., services). I'm curious what you expect is the right behavior here. In this scenario, there is no nested configuration. Are top level properties permitted to be indented?

Example:

[profile my-profile]
    region=us-east-1 # This fails right now
s3 =
  region = us-west-2
jterapin commented 3 months ago

I'm currently under the impression that the whitespaces on the top level properties should be ignored based on some guidelines set across AWS SDKs. We are still investigating so stay tuned.

jterapin commented 3 months ago

Update: We now have an internal ticket to resolve this bug and improve tests to cover edge cases.

mullermp commented 3 months ago

This may take a long time to fix because it's not straight forward. In fact I see many issues with our ini parsing in relation to other SDKs. My best advice would be to fix the config file as this will take some time to change and we'd like to fully align with the specification. I think the Ruby SDKs implementation predates that specification as such.

mullermp commented 2 months ago

I confirmed with the internal specification that this preceding space is NOT part of the specification and this is not a bug. INI specifications are generally not universal. I am closing this. The recommended fix is to fix your formatting as you've already pointed out. We've taken a backlog item to improve other parsing cases outlined in the internal specification not related to this specific issue.

github-actions[bot] commented 2 months ago

This issue is now closed. Comments on closed issues are hard for our team to see. If you need more assistance, please open a new issue that references this one.