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

Fix INI parsing of whitespace preceding config key #3058

Closed
jordanrmerrick opened this issue Jul 1, 2024 · 7 comments
Closed

Fix INI parsing of whitespace preceding config key #3058

jordanrmerrick opened this issue Jul 1, 2024 · 7 comments
Labels
bug This issue is a bug. queued This issues is on the AWS team's backlog

Comments

@jordanrmerrick
Copy link

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:in block 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

@jordanrmerrick jordanrmerrick added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 1, 2024
@jterapin jterapin added investigating Issue is being investigated and removed needs-triage This issue or PR still needs to be triaged. labels Jul 1, 2024
@jterapin
Copy link
Contributor

jterapin commented Jul 1, 2024

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

@jordanrmerrick
Copy link
Author

jordanrmerrick commented Jul 2, 2024

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
Copy link
Contributor

jterapin commented Jul 2, 2024

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
Copy link
Contributor

jterapin commented Jul 3, 2024

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

@jterapin jterapin added queued This issues is on the AWS team's backlog and removed investigating Issue is being investigated labels Jul 3, 2024
@mullermp
Copy link
Contributor

mullermp commented Jul 4, 2024

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
Copy link
Contributor

mullermp commented Jul 8, 2024

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.

@mullermp mullermp closed this as completed Jul 8, 2024
Copy link

github-actions bot commented Jul 8, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. queued This issues is on the AWS team's backlog
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants