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 @sensitive handling in Display implementations on error shapes #1802

Merged
merged 12 commits into from
Oct 7, 2022

Conversation

jjant
Copy link
Contributor

@jjant jjant commented Oct 3, 2022

Motivation and Context

Addresses #1743.
This doesn't yet use the Sensitive type to handle redaction of @sensitive fields (as suggested by @hlbarber) because that type currently lives in the aws-smithy-http-server crate, but the type is also needed by the client (see: #1804).

Description

This smithy model:

structure ResourceAlreadyExists {
    message: SensitiveString
}

@sensitive
string SensitiveString

Now generates:

impl std::fmt::Display for ResourceAlreadyExists {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        write!(f, "ResourceAlreadyExists")?;
        write!(f, ": {}", "*** Sensitive Data Redacted ***")?;
        Ok(())
    }
}

Testing

I've manually checked the generated output compiling via ./gradlew codegen-server-test:build.

Checklist

  • I have updated CHANGELOG.next.toml if I made changes to the smithy-rs codegen or 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.

@github-actions
Copy link

github-actions bot commented Oct 3, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@github-actions
Copy link

github-actions bot commented Oct 3, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@jjant jjant marked this pull request as ready for review October 3, 2022 14:59
@jjant jjant requested review from a team as code owners October 3, 2022 14:59
@github-actions
Copy link

github-actions bot commented Oct 3, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@github-actions
Copy link

github-actions bot commented Oct 4, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@jjant jjant added the needs-review A tag for PRs waiting on a review from one of the repo admins label Oct 4, 2022
@jjant jjant changed the title Fix @sensitive handling in Display implementations on structures Fix @sensitive handling in Display implementations on error shapes Oct 5, 2022
@github-actions
Copy link

github-actions bot commented Oct 7, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@jjant jjant merged commit 238cf8b into main Oct 7, 2022
@jjant jjant deleted the jjant/fix-sensitive-display-on-errors branch October 7, 2022 13:44
hlbarber pushed a commit that referenced this pull request Oct 7, 2022
…1802)

* Use `Sensitive` wrapper in Debug impl for structures

* Fix using the wrong import path for `Sensitive`

* Use redactMemberIfNecessary

* Fix display implementation on errors to respect @sensitive trait

* Don't use Sensitive type just yet

* Add entry in changelog

* Improve redaction of sensitive error message

* Use correct flags in changelog

* Run ktlint
@Velfi Velfi removed the needs-review A tag for PRs waiting on a review from one of the repo admins label Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants