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

Use tokio::process to allow timeouts to occur #3052

Merged
merged 1 commit into from
Oct 25, 2023
Merged

Conversation

rcoh
Copy link
Collaborator

@rcoh rcoh commented Oct 10, 2023

Motivation and Context

The existing credentials provider was a DOS risk and didn't obey timeout settings because it used std::timeout::spawn but relied on a async-based timeout mechanism.

Description

  • Use tokio::process instead
  • introduce new cargo feature

Testing

  • unit test

Checklist

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

@rcoh rcoh force-pushed the cred-process-spawn branch from 8adaa93 to ed6c0d4 Compare October 10, 2023 17:51
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@rcoh rcoh force-pushed the cred-process-spawn branch from ed6c0d4 to 8aef0a5 Compare October 19, 2023 17:18
@rcoh rcoh marked this pull request as ready for review October 19, 2023 17:19
@rcoh rcoh requested review from a team as code owners October 19, 2023 17:19
@rcoh rcoh force-pushed the cred-process-spawn branch from 8aef0a5 to c0c4bca Compare October 19, 2023 17:19
@github-actions
Copy link

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@rcoh rcoh force-pushed the cred-process-spawn branch from c0c4bca to 2331c34 Compare October 20, 2023 17:00
@github-actions
Copy link

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@rcoh rcoh force-pushed the cred-process-spawn branch from fd89bcf to d6fbcc4 Compare October 23, 2023 15:14
@github-actions
Copy link

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@rcoh rcoh force-pushed the cred-process-spawn branch from d6fbcc4 to 656a79a Compare October 25, 2023 15:01
@github-actions
Copy link

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@@ -3,57 +3,20 @@
* SPDX-License-Identifier: Apache-2.0
*/

#![cfg(feature = "credentials-process")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question because I don't know: Is this strictly equivalent to adding the same flag to the credential_process mod declaration?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah

self.inner.write(buf)
self.inner.write_all(buf)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh, write vs. write_all is such an easy mistake to make.

@rcoh rcoh mentioned this pull request Oct 25, 2023
9 tasks
@rcoh rcoh force-pushed the cred-process-spawn branch from 656a79a to 08522ec Compare October 25, 2023 15:55
@github-actions
Copy link

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@rcoh rcoh added this pull request to the merge queue Oct 25, 2023
Merged via the queue into main with commit 6ceabf8 Oct 25, 2023
39 of 41 checks passed
@rcoh rcoh deleted the cred-process-spawn branch October 25, 2023 18:55
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.

2 participants