Skip to content

Commit

Permalink
Make profile keys case-insensitive (#3344)
Browse files Browse the repository at this point in the history
## Motivation and Context
- https://github.com/aws/aws-sdk/issues/574
- awslabs/aws-sdk-rust#1020

## Description
To match the CLI behavior and improve customer experience, make the keys
on the profile file case-insensitive.

## Testing
- New credentials integration test

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
  • Loading branch information
rcoh authored Jan 9, 2024
1 parent 1446cad commit 49622b2
Show file tree
Hide file tree
Showing 12 changed files with 81 additions and 20 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,9 @@ message = "Fix bug in `CredentialsProcess` provider where `expiry` was incorrect
references = ["smithy-rs#3335", "aws-sdk-rust#1021"]
meta = { "breaking" = false, "tada" = false, "bug" = true }
author = "rcoh"

[[aws-sdk-rust]]
message = "~/.aws/config and ~/.aws/credentials now parse keys in a case-insensitive way. This means the `AWS_SECRET_ACCESS_KEY` is supported in addition to `aws_secret_access_key`."
references = ["aws-sdk#574", "aws-sdk-rust#1020", "smithy-rs#3344"]
meta = { "breaking" = false, "bug" = true, "tada" = false }
author = "rcoh"
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ mod test {

make_test!(prefer_environment);
make_test!(profile_static_keys);
make_test!(profile_static_keys_case_insensitive);
make_test!(web_identity_token_env);
make_test!(web_identity_source_profile_no_env);
make_test!(web_identity_token_invalid_jwt);
Expand Down
6 changes: 4 additions & 2 deletions aws/rust-runtime/aws-config/src/profile/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

use crate::profile::parser::parse::parse_profile_file;
use crate::profile::parser::parse::{parse_profile_file, to_ascii_lowercase};
use crate::profile::parser::source::Source;
use crate::profile::profile_file::ProfileFiles;
use aws_types::os_shim_internal::{Env, Fs};
Expand Down Expand Up @@ -169,7 +169,9 @@ impl Profile {

/// Returns a reference to the property named `name`
pub fn get(&self, name: &str) -> Option<&str> {
self.properties.get(name).map(|prop| prop.value())
self.properties
.get(to_ascii_lowercase(name).as_ref())
.map(|prop| prop.value())
}
}

Expand Down
7 changes: 4 additions & 3 deletions aws/rust-runtime/aws-config/src/profile/parser/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ pub(super) fn merge_in(
}
}

fn merge_into_base(target: &mut Profile, profile: HashMap<&str, Cow<'_, str>>) {
fn merge_into_base(target: &mut Profile, profile: HashMap<Cow<'_, str>, Cow<'_, str>>) {
for (k, v) in profile {
match validate_identifier(k) {
match validate_identifier(k.as_ref()) {
Ok(k) => {
target
.properties
Expand Down Expand Up @@ -146,6 +146,7 @@ fn validate_identifier(input: &str) -> Result<&str, ()> {

#[cfg(test)]
mod tests {
use std::borrow::Cow;
use std::collections::HashMap;

use tracing_test::traced_test;
Expand Down Expand Up @@ -218,7 +219,7 @@ mod tests {
let mut profile: RawProfileSet<'_> = HashMap::new();
profile.insert("default", {
let mut out = HashMap::new();
out.insert("invalid key", "value".into());
out.insert(Cow::Borrowed("invalid key"), "value".into());
out
});
let mut base = ProfileSet::empty();
Expand Down
41 changes: 28 additions & 13 deletions aws/rust-runtime/aws-config/src/profile/parser/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use std::error::Error;
use std::fmt::{self, Display, Formatter};

/// A set of profiles that still carries a reference to the underlying data
pub(super) type RawProfileSet<'a> = HashMap<&'a str, HashMap<&'a str, Cow<'a, str>>>;
pub(super) type RawProfileSet<'a> = HashMap<&'a str, HashMap<Cow<'a, str>, Cow<'a, str>>>;

/// Characters considered to be whitespace by the spec
///
Expand Down Expand Up @@ -98,7 +98,7 @@ enum State<'a> {
Starting,
ReadingProfile {
profile: &'a str,
property: Option<&'a str>,
property: Option<Cow<'a, str>>,
is_subproperty: bool,
},
}
Expand Down Expand Up @@ -152,7 +152,7 @@ impl<'a> Parser<'a> {
.map_err(|err| err.into_error("property", location.clone()))?;
self.state = State::ReadingProfile {
profile: name,
property: Some(k),
property: Some(k.clone()),
is_subproperty: v.is_empty(),
};
current_profile.insert(k, v.into());
Expand Down Expand Up @@ -184,7 +184,7 @@ impl<'a> Parser<'a> {
self.data
.get_mut(*profile)
.expect("profile must exist")
.get_mut(*property)
.get_mut(property.as_ref())
.expect("property must exist")
}
State::ReadingProfile {
Expand Down Expand Up @@ -246,15 +246,23 @@ impl PropertyError {
}

/// Parse a property line into a key-value pair
fn parse_property_line(line: &str) -> Result<(&str, &str), PropertyError> {
fn parse_property_line(line: &str) -> Result<(Cow<'_, str>, &str), PropertyError> {
let line = prepare_line(line, true);
let (k, v) = line.split_once('=').ok_or(PropertyError::NoEquals)?;
let k = k.trim_matches(WHITESPACE);
let v = v.trim_matches(WHITESPACE);
if k.is_empty() {
return Err(PropertyError::NoName);
}
Ok((k, v))
Ok((to_ascii_lowercase(k), v))
}

pub(crate) fn to_ascii_lowercase(s: &str) -> Cow<'_, str> {
if s.bytes().any(|b| b.is_ascii_uppercase()) {
Cow::Owned(s.to_ascii_lowercase())
} else {
Cow::Borrowed(s)
}
}

/// Prepare a line for parsing
Expand Down Expand Up @@ -291,23 +299,30 @@ mod test {
use crate::profile::parser::parse::{parse_property_line, PropertyError};
use crate::profile::parser::source::File;
use crate::profile::profile_file::ProfileFileKind;
use std::borrow::Cow;

// most test cases covered by the JSON test suite

#[test]
fn property_parsing() {
assert_eq!(parse_property_line("a = b"), Ok(("a", "b")));
assert_eq!(parse_property_line("a=b"), Ok(("a", "b")));
assert_eq!(parse_property_line("a = b "), Ok(("a", "b")));
assert_eq!(parse_property_line(" a = b "), Ok(("a", "b")));
assert_eq!(parse_property_line(" a = b 🐱 "), Ok(("a", "b 🐱")));
fn ok<'a>(key: &'a str, value: &'a str) -> Result<(Cow<'a, str>, &'a str), PropertyError> {
Ok((Cow::Borrowed(key), value))
}

assert_eq!(parse_property_line("a = b"), ok("a", "b"));
assert_eq!(parse_property_line("a=b"), ok("a", "b"));
assert_eq!(parse_property_line("a = b "), ok("a", "b"));
assert_eq!(parse_property_line(" a = b "), ok("a", "b"));
assert_eq!(parse_property_line(" a = b 🐱 "), ok("a", "b 🐱"));
assert_eq!(parse_property_line("a b"), Err(PropertyError::NoEquals));
assert_eq!(parse_property_line("= b"), Err(PropertyError::NoName));
assert_eq!(parse_property_line("a = "), Ok(("a", "")));
assert_eq!(parse_property_line("a = "), ok("a", ""));
assert_eq!(
parse_property_line("something_base64=aGVsbG8gZW50aHVzaWFzdGljIHJlYWRlcg=="),
Ok(("something_base64", "aGVsbG8gZW50aHVzaWFzdGljIHJlYWRlcg=="))
ok("something_base64", "aGVsbG8gZW50aHVzaWFzdGljIHJlYWRlcg==")
);

assert_eq!(parse_property_line("ABc = DEF"), ok("abc", "DEF"));
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"HOME": "/home"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[default]
region = us-east-1
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[default]
AWS_ACCESS_KEY_ID = correct_key
AWS_SECRET_ACCESS_KEY = correct_secret
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"events": [],
"docs": "test case uses static creds, no network requests",
"version": "V0"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "profile_static_keys",
"docs": "load static keys from a profile",
"result": {
"Ok": {
"access_key_id": "correct_key",
"secret_access_key": "correct_secret"
}
}
}
15 changes: 14 additions & 1 deletion aws/rust-runtime/aws-config/test-data/profile-parser-tests.json
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,19 @@
}
}
},
{
"name": "Duplicate properties in duplicate profiles use the last one defined (case insensitive).",
"input": {
"configFile": "[profile foo]\nName = value\n[profile foo]\nname = value2"
},
"output": {
"profiles": {
"foo": {
"name": "value2"
}
}
}
},
{
"name": "Default profile with profile prefix overrides default profile without prefix when profile prefix is first.",
"input": {
Expand Down Expand Up @@ -518,7 +531,7 @@
"output": {
"profiles": {
"foo": {
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_": "value"
"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz0123456789-_": "value"
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion tools/ci-build/smithy-rs-tool-common/src/changelog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ impl FromStr for Reference {
),
Some((repo, number)) => {
let number = number.parse::<usize>()?;
if !matches!(repo, "smithy-rs" | "aws-sdk-rust") {
if !matches!(repo, "smithy-rs" | "aws-sdk-rust" | "aws-sdk") {
bail!("unexpected repo: {}", repo);
}
Ok(Reference {
Expand Down

0 comments on commit 49622b2

Please sign in to comment.