Normalize URI paths during signing (except for S3) #2018
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit fixes a bug reported in awslabs/aws-sdk-rust#661. Note that this de-duping is not part of URI path normalization because it must happen to a request for any service, including S3.
This commit enables URI path normalization in aws-sigv4. It follows https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html to create the canonical URI that is part of the canonical request.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
This commit updates the implementation of normalize_path_segment so that it does not use an OS specific path separator, specifically "\\" on Windows and always uses a forward slash "/". It addresses a test failure on Windows: https://github.com/awslabs/smithy-rs/actions/runs/3518627877/jobs/5897764889.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
I think this should only be done for the services that explicitly require it e.g. S3. Stripping out leading forward slashes could be in violation of the spec. I say could because I raised this with the Smithy team in smithy-lang/smithy#1024, and no conclusion was made. Note that if we strip out leading forward slashes we may be stripping out empty path segments that could be bound to string members. For example, consider the model: If I send the request In the server SDK, we're not stripping out empty segments from incoming requests. |
Thanks for the great feedback! This is certainly something I was not aware of. It totally makes sense for de-duping leading forward slashes to be applied just to S3. Addressed in 1ee2d85. |
This commit addresses #2018 (comment). There is a case where an empty string between leading double forward slashes is meaningful so blindly de-duping them breaks the use case. Therefore, handling multiple leading forward slashes in this manner should be applied only to S3.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
This commit adds an integration test verifying the special rule for URI path normalization applies to S3. That is, leading forward slashes should be de-deduped and normalizing URI paths as prescribed in RFC 3986 should be ignored.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
This commit reverts the naming of variants in UriPathNormalizationMode. They were PerRfc3986 and ForS3, but they should be more generic like Enabled and Disabled as we had before.
ysaito1001
changed the title
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
This commit simplifies the integration test normalize-uri-path for s3. Previously, the test used ReplayingConnection but it was an overkill to test something as simple as whether the generated request contained the expected URI and signature.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
jdisanti
reviewed
Nov 29, 2022
| /// <https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html> | ||
| /// | ||
| /// Uri path normalization is performed based on <https://www.rfc-editor.org/rfc/rfc3986>. | ||
| /// The exception to this is that we do not normalize URI paths for requests to Amazon S3. |
There was a problem hiding this comment.
Seems like it would be better to provide S3 as an example on the doc comment for Disabled, since there may be other services where it needs to be disabled as well.
There was a problem hiding this comment.
👍 Addressed in 182729a
aws/rust-runtime/aws-sigv4/src/http_request/uri_path_normalization.rs
Outdated
Show resolved
Hide resolved
aws/rust-runtime/aws-sigv4/src/http_request/uri_path_normalization.rs
Outdated
Show resolved
Hide resolved
aws/rust-runtime/aws-sigv4/src/http_request/uri_path_normalization.rs
Outdated
Show resolved
Hide resolved
aws/rust-runtime/aws-sigv4/src/http_request/uri_path_normalization.rs
Outdated
Show resolved
Hide resolved
aws/rust-runtime/aws-sigv4/src/http_request/uri_path_normalization.rs
Outdated
Show resolved
Hide resolved
Co-authored-by: John DiSanti <jdisanti@amazon.com>
Co-authored-by: John DiSanti <jdisanti@amazon.com>
…tion.rs Co-authored-by: John DiSanti <jdisanti@amazon.com>
This commit addresses the following code review feedback: #2018 (comment) #2018 (comment) #2018 (comment)
This commit addresses #2018 (comment)
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
This commit addresses #2018 (comment). It counts the number of slashes to preallocate the capacity of a Vec used in normalize_path_segment.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
jdisanti
reviewed
Nov 30, 2022
Comment on lines
+48
to
+55
| let mut result = normalized.join("/"); | ||
|
|
||
| // Even though `uri_path` starts with a `/`, that may not be the case for `result`. | ||
| // An example of this is `uri_path` being "/../foo" where the corresponding `result` | ||
| // will be "foo". | ||
| if !result.starts_with('/') { | ||
| result.insert(0, '/'); | ||
| } |
There was a problem hiding this comment.
I think result will never start with / in this code, so the result.insert will always run.
There was a problem hiding this comment.
Interestingly, result starts with / most of the time. For instance, if the input to the function is /a/b/c, then the vector normalized will contain ["", "a", "b", "c"]. After normalized.join, result will then be /a/b/c.
There was a problem hiding this comment.
Ah, yeah, you're right. I forgot about the leading / causing a "" in the split.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
jdisanti
approved these changes
Dec 1, 2022
Comment on lines
+48
to
+55
| let mut result = normalized.join("/"); | ||
|
|
||
| // Even though `uri_path` starts with a `/`, that may not be the case for `result`. | ||
| // An example of this is `uri_path` being "/../foo" where the corresponding `result` | ||
| // will be "foo". | ||
| if !result.starts_with('/') { | ||
| result.insert(0, '/'); | ||
| } |
There was a problem hiding this comment.
Ah, yeah, you're right. I forgot about the leading / causing a "" in the split.
Co-authored-by: John DiSanti <jdisanti@amazon.com>
Co-authored-by: John DiSanti <jdisanti@amazon.com>
Co-authored-by: John DiSanti <jdisanti@amazon.com>
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
ysaito1001
deleted the
ysaito/normalize_uri_path_for_canonical_request
branch
aws-sdk-rust-ci
pushed a commit
to awslabs/aws-sdk-rust
that referenced
this pull request
Dec 14, 2022
* De-dupe multiple leading forward slashes in request This commit fixes a bug reported in #661. Note that this de-duping is not part of URI path normalization because it must happen to a request for any service, including S3. * Normalize URI paths for canonical requests This commit enables URI path normalization in aws-sigv4. It follows https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html to create the canonical URI that is part of the canonical request. * Avoid using OS specific path separator This commit updates the implementation of normalize_path_segment so that it does not use an OS specific path separator, specifically "\\" on Windows and always uses a forward slash "/". It addresses a test failure on Windows: https://github.com/awslabs/smithy-rs/actions/runs/3518627877/jobs/5897764889. * Fix the rustdoc::bare_urls warning from cargo doc * Fix a typo Normalzie -> Normalize * Limit de-duping multiple leading forward slashes to S3 This commit addresses smithy-lang/smithy-rs#2018 (comment). There is a case where an empty string between leading double forward slashes is meaningful so blindly de-duping them breaks the use case. Therefore, handling multiple leading forward slashes in this manner should be applied only to S3. * Add an integration test per #661 This commit adds an integration test verifying the special rule for URI path normalization applies to S3. That is, leading forward slashes should be de-deduped and normalizing URI paths as prescribed in RFC 3986 should be ignored. * Update CHANGELOG.next.toml * Revert 87b749c This commit reverts 87b749c, the reason being that we need to better understand the requirement for de-duplicating leading forward slashes in requests, i.e. do we even need to do it or if so where should we perform the de-dupe operation? * Revert naming of UriPathNormalizationMode variants This commit reverts the naming of variants in UriPathNormalizationMode. They were PerRfc3986 and ForS3, but they should be more generic like Enabled and Disabled as we had before. * Update CHANGELOG.next.toml * Simplify integration test using capture_request This commit simplifies the integration test normalize-uri-path for s3. Previously, the test used ReplayingConnection but it was an overkill to test something as simple as whether the generated request contained the expected URI and signature. * Update aws/rust-runtime/aws-sigv4/src/http_request/settings.rs Co-authored-by: John DiSanti <jdisanti@amazon.com> * Update aws/rust-runtime/aws-sigv4/src/http_request/settings.rs Co-authored-by: John DiSanti <jdisanti@amazon.com> * Update aws/rust-runtime/aws-sigv4/src/http_request/uri_path_normalization.rs Co-authored-by: John DiSanti <jdisanti@amazon.com> * Address review feedback on normalize_path_segment This commit addresses the following code review feedback: smithy-lang/smithy-rs#2018 (comment) smithy-lang/smithy-rs#2018 (comment) smithy-lang/smithy-rs#2018 (comment) * Update CHANGELOG.next.toml This commit addresses smithy-lang/smithy-rs#2018 (comment) * Preallocate the Vec capacity for normalize_path_segment This commit addresses smithy-lang/smithy-rs#2018 (comment). It counts the number of slashes to preallocate the capacity of a Vec used in normalize_path_segment. * Address smithy-lang/smithy-rs#2018 * Update aws/rust-runtime/aws-sigv4/src/http_request/settings.rs Co-authored-by: John DiSanti <jdisanti@amazon.com> * Update aws/rust-runtime/aws-sigv4/src/http_request/settings.rs Co-authored-by: John DiSanti <jdisanti@amazon.com> * Update aws/rust-runtime/aws-sigv4/src/http_request/settings.rs Co-authored-by: John DiSanti <jdisanti@amazon.com> Co-authored-by: Yuki Saito <awsaito@amazon.com> Co-authored-by: John DiSanti <jdisanti@amazon.com>
thomas-k-cameron
added a commit
to thomas-k-cameron/smithy-rs
that referenced
this pull request
Apr 9, 2023
* Add error refactor changelog entries and re-export `DisplayErrorContext` (#1951)
* Re-export `DisplayErrorContext`
* Update changelog
* Do not use deprecated from_timestamp from chrono (#1980)
* Do not use deprecated from_timestamp from chrono
CI is failing because
[from_timestamp](https://docs.rs/chrono/latest/chrono/naive/struct.NaiveDateTime.html#method.from_timestamp)
is now deprecated.
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
* Update changelog
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
* Fix error
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
* Use with_ymd_and_hms
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
* Remove left behind trailing whitespace from instrumentation closure (#1975)
* Fix `update-sdk-next` GitHub Actions workflow (#1979)
* Feature: configurable connectors for AWS clients (#1918)
* feature: make HTTP connectors configurable
* add: test for HTTP connector configuration customization
add: impl<B> From<TestConnection<B>> for HttpConnector
add: impl From<CaptureRequestHandler> for HttpConnector
add: impl From<NeverConnector> for HttpConnector
add: impl From<ReplayingConnection> for HttpConnector
* add: to_vec method to AggregatedBytes
update: method param names of FluentClientGenerics.sendBounds to be more explicit
update: restructure s3/s3control tests to be uniform in structure
* update: CHANGELOG.next.toml
update: codegen `impl From<&SdkConfig> for Builder` to support HTTP connectors
* update: CHANGELOG entry references
* add: missing copyright header
* fix: clippy lint
* format: run cargo fmt
* format: run cargo fmt on aws_smithy_client::dvr modules
* format: run ktlintFormat
* refactor: use from_conf instead of from_conf_conn
remove: from_conf_conn
* update: impl From<SmithyConnector> for HttpConnector
remove: other From<T> for HttpConnector impls
update: HttpConnector config setter examples
* update: CHANGELOG.next.toml
* Update CHANGELOG.next.toml
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* update: CHANGELOG.next.toml
remove: obsolete test
update: `ConfigLoader::http_connector` setter method
* Update CHANGELOG.next.toml
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Update CHANGELOG.next.toml
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Update CHANGELOG.next.toml
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Apply suggestions from code review
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* update: aws_config::loader::ConfigLoader doc comments
update: CHANGELOG.next.toml examples
* fix: doc issues
add: reëxport aws_smithy_types::endpoint module to aws-config
* format: run rustfmt in the weird CI way to get it to actually format.
* fix: incorrect reëxport
* add: "aws_smithy_http::endpoint" to allowed external types for aws-config
* update: move `hyper-rustls` to deps so that it doesn't break exotic arch CI check
* remove: `hyper-rustls` dep because it's not actually needed
* fix: aws-types dep issue blocking exotic arch CI check
* fix: broken doc comment
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Service with ConnectInfo (#1955)
* Service with ConnectInfo
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
* Use `escape` on the body in `@httpRequestTest` protocol test generation (#1949)
* Builders of builders (#1342)
This patchset, affectionately called "Builders of builders", lays the
groundwork for fully implementing [Constraint traits] in the server SDK
generator. [The RFC] illustrates what the end goal looks like, and is
recommended prerrequisite reading to understanding this cover letter.
This commit makes the sever deserializers work with _unconstrained_ types
during request parsing, and only after the entire request is parsed are
constraints enforced. Values for a constrained shape are stored in the
correspondingly unconstrained shape, and right before the operation input is
built, the values are constrained via a `TryFrom<UnconstrainedShape> for
ConstrainedShape` implementation that all unconstrained types enjoy. The
service owner only interacts with constrained types, the unconstrained ones are
`pub(crate)` and for use by the framework only.
In the case of structure shapes, the corresponding unconstrained shape is their
builders. This is what gives this commit its title: during request
deserialization, arbitrarily nested structures are parsed into _builders that
hold builders_. Builders keep track of whether their members are constrained or
not by storing its members in a `MaybeConstrained`
[Cow](https://doc.rust-lang.org/std/borrow/enum.Cow.html)-like `enum` type:
```rust
pub(crate) trait Constrained {
type Unconstrained;
}
#[derive(Debug, Clone)]
pub(crate) enum MaybeConstrained<T: Constrained> {
Constrained(T),
Unconstrained(T::Unconstrained),
}
```
Consult the documentation for the generator in `ServerBuilderGenerator.kt` for
more implementation details and for the differences with the builder types the
server has been using, generated by `BuilderGenerator.kt`, which after this
commit are exclusively used by clients.
Other shape types, when they are constrained, get generated with their
correspondingly unconstrained counterparts. Their Rust types are essentially
wrapper newtypes, and similarly enjoy `TryFrom` converters to constrain them.
See the documentation in `UnconstrainedShapeSymbolProvider.kt` for details and
an example.
When constraints are not met, the converters raise _constraint violations_.
These are currently `enum`s holding the _first_ encountered violation.
When a shape is _transitively but not directly_ constrained, newtype wrappers
are also generated to hold the nested constrained values. To illustrate their
need, consider for example a list of `@length` strings. Upon request parsing,
the server deserializers need a way to hold a vector of unconstrained regular
`String`s, and a vector of the constrained newtyped `LengthString`s. The former
requirement is already satisfied by the generated unconstrained types, but for
the latter we need to generate an intermediate constrained
`ListUnconstrained(Vec<LengthString>)` newtype that will eventually be
unwrapped into the `Vec<LengthString>` the user is handed. This is the purpose
of the `PubCrate*` generators: consult the documentation in
`PubCrateConstrainedShapeSymbolProvider.kt`,
`PubCrateConstrainedCollectionGenerator.kt`, and
`PubCrateConstrainedMapGenerator.kt` for more details. As their name implies,
all of these types are `pub(crate)`, and the user never interacts with them.
For users that would not like their application code to make use of constrained
newtypes for their modeled constrained shapes, a `codegenConfig` setting
`publicConstrainedTypes` has been added. They opt out of these by setting it to
`false`, and use the inner types directly: the framework will still enforce
constraints upon request deserialization, but once execution enters an
application handler, the user is on their own to honor (or not) the modeled
constraints. No user interest has been expressed for this feature, but I expect
we will see demand for it. Moreover, it's a good stepping stone for users that
want their services to honor constraints, but are not ready to migrate their
application code to constrained newtypes. As for how it's implemented, several
parts of the codebase inspect the setting and toggle or tweak generators based
on its value. Perhaps the only detail worth mentioning in this commit message
is that the structure shape builder types are generated by a much simpler and
entirely different generator, in
`ServerBuilderGeneratorWithoutPublicConstrainedTypes.kt`. Note that this
builder _does not_ enforce constraints, except for `required` and `enum`, which
are always (and already) baked into the type system. When
`publicConstrainedTypes` is disabled, this is the builder that end users
interact with, while the one that enforces all constraints,
`ServerBuilderGenerator`, is now generated as `pub(crate)` and left for
exclusive use by the deserializers. See the relevant documentation for the
details and differences among the builder types.
As proof that these foundations are sound, this commit also implements the
`length` constraint trait on Smithy map and string shapes. Likewise, the
`required` and `enum` traits, which were already baked in the generated types
as non-`Option`al and `enum` Rust types, respectively, are now also treated
like the rest of constraint traits upon request deserialization. See the
documentation in `ConstrainedMapGenerator.kt` and
`ConstrainedStringGenerator.kt` for details.
The rest of the constraint traits and target shapes are left as an exercise to
the reader, but hopefully the reader has been convinced that all of them can be
enforced within this framework, paving the way for straightforward
implementations. The diff is already large as it is. Any reamining work is
being tracked in #1401; this and other issues are referenced in the code as
TODOs.
So as to not give users the impression that the server SDK plugin _fully_
honors constraints as per the Smithy specification, a validator in
`ValidateUnsupportedConstraintsAreNotUsed.kt` has been added. This traverses
the model and detects yet-unsupported parts of the spec, aborting code
generation and printing informative warnings referencing the relevant tracking
issues. This is a regression in that models that used constraint traits
previously built fine (even though the constraint traits were silently not
being honored), and now they will break. To unblock generation of these models,
this commit adds another `codegenConfig` setting,
`ignoreUnsupportedConstraints`, that users can opt into.
Closes #1714.
Testing
-------
Several Kotlin unit test classes exercising the finer details of the added
generators and symbol providers have been added. However, the best way to test
is to generate server SDKs from models making use of constraint traits. The
biggest assurances come from the newly added `constraints.smithy` model, an
"academic" service that _heavily_ exercises constraint traits. It's a
`restJson1` service that also tests binding of constrained shapes to different
parts of the HTTP message. Deeply nested hierarchies and recursive shapes are
also featured.
```sh
./gradlew -P modules='constraints' codegen-server-test:build
```
This model is _additionally_ generated in CI with the `publicConstrainedTypes`
setting disabled:
```sh
./gradlew -P modules='constraints_without_public_constrained_types' codegen-server-test:build
``````
Similarly, models using currently unsupported constraints are now being
generated with the `ignoreUnsupportedConstraints` setting enabled.
See `codegen-server-test/build.gradle.kts` for more details.
[Constraint traits]: https://awslabs.github.io/smithy/2.0/spec/constraint-traits.html
[The RFC]: https://github.com/awslabs/smithy-rs/pull/1199
* Revamp errors in `aws-smithy-http` (#1884)
* Revamp errors in `aws-smithy-json` (#1888)
* Revamp errors in `aws-smithy-types` (#1893)
* Revamp errors in `aws-smithy-types-convert` and `aws-smithy-xml` (#1917)
* Revamp errors in AWS runtime crates (#1922)
Revamp errors in:
* `aws-types`
* `aws-endpoint`
* `aws-http`
* `aws-sig-auth`
* `aws-inlineable`
* Revamp errors in `aws-config` (#1934)
* Make `SdkError::into_service_error` infallible (#1974)
* Implement RFC23 - Evolve the new service builder API (#1954)
* Implement RFC 23, with the exception of PluginBuilder
* Update documentation.
* Elide implementation details in `Upgradable`.
* Update wording in docs to avoid personal pronouns.
* Update `Upgradable`'s documentation.
* Template the service builder name.
* Template MissingOperationsError directly.
* Code-generate an array directly.
* Update design/src/server/anatomy.md
Co-authored-by: david-perez <d@vidp.dev>
* Use `rust` where we do not need templating.
* Remove unused variable.
* Add `expect` message to point users at our issue board in case a panic slips through.
* Typo.
* Update `expect` error message.
* Refactor the `for` loop in ``requestSpecMap` into an `associateWith` call.
* Fix new pokemon bin example.
* Fix new pokemon bin example.
* Fix unresolved symbolProvider.
* Assign the `expect` error message to a variable.
* Omit additional generic parameters in Upgradable when it's first introduced.
Co-authored-by: david-perez <d@vidp.dev>
* Fix simple.smithy (#1991)
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
* Add support for Endpoints 2.0 Parameters (#1953)
* Add support for Endpoints 2.0 Parameters
This commit adds `EndpointDecorator` which injects Endpoint
parameters in to the operation property bag. These can come from
an ordered list of sources—this wires them all up. To facilitate
testing, this diff also writes the parameters into the property
bag during operation generation.
* remove println
* CR Feedback
* Implement RFC 23, Part 2 - Plugin pipeline (#1971)
* Implement RFC 23, with the exception of PluginBuilder
* Update documentation.
* Avoid star re-exports.
* Elide implementation details in `Upgradable`.
* Update wording in docs to avoid personal pronouns.
* Update `Upgradable`'s documentation.
* Template the service builder name.
* Template MissingOperationsError directly.
* Code-generate an array directly.
* Sketch out the implementation of `PluginPipeline`.
Remove `PluginExt`.
Add a public constructor for `FilterByOperationName`.
* Ask for a `PluginPipeline` as input for the generated builder.
* Rename `add` to `push`.
* Remove Pluggable.
Rename `composer` module to `pipeline`.
* Remove all mentions of `Pluggable` from docs and examples.
* Fix punctuation.
* Rename variable from `composer` to `pipeline` in doc examples.
* Add a free-standing function for filtering.
* Rename.
* Rename.
* Update design/src/server/anatomy.md
Co-authored-by: david-perez <d@vidp.dev>
* Use `rust` where we do not need templating.
* Remove unused variable.
* Add `expect` message to point users at our issue board in case a panic slips through.
* Typo.
* Update `expect` error message.
* Refactor the `for` loop in ``requestSpecMap` into an `associateWith` call.
* Remove unnecessary bound - since `new` is private, the condition is already enforced via `filter_by_operation_name`.
* Use `Self` in `new`.
* Rename `inner` to `into_inner`
* Rename `concat` to `append` to correctly mirror Vec's API terminology.
* Fix codegen to use renamed method.
* Cut down the public API surface to key methods for a sequence-like interface.
* Add a note about ordering.
* Add method docs.
* Add Default implementation.
* Fix new pokemon bin example.
* Fix new pokemon bin example.
* Fix code-generated builder.
* Fix unresolved symbolProvider.
* Assign the `expect` error message to a variable.
* Do not require a PluginPipeline as input to `builder_with_plugins`.
* Reverse plugin application order.
* Upgrade documentation.
* Add a test to verify that plugin layers are executed in registration order.
* Add license header.
* Update middleware.md
* Typo.
* Fix more builder() calls.
Co-authored-by: david-perez <d@vidp.dev>
* Improve error doc comments around logging errors (#1990)
* Rework enum for forward-compatibility (#1945)
* Make enum forward-compatible
This commit implements the suggested approach described in
https://github.com/awslabs/smithy-rs/issues/627#issuecomment-995923092
The idea is that once the user writes a match expression against an enum
and assumes that an execution path comes to a particular match arm, we
should guarantee that when the user upgrades a version of SDK, the
execution path should come to the same match arm as before.
* Add unit test to ensure enums are forward-compatible
The test first mimics the user's interaction with the enum generated from
a model where the user writes a match expression on the enum, wishing to
hit the match arm corresponding to Variant3, which is not yet supported
by the model. The test then simulates a scenario where the user now has
access to the updated enum generated from the next version of the model
that does support Variant3.
The user's code should compile in both of the use cases.
* Generate rustdoc for enum's forward-compatibility
This commits generates rustdoc explaining to the users how they might
write a match expression against a generated enum so their code is
forward-compatible. While it is explained in
https://github.com/awslabs/smithy-rs/issues/627#issuecomment-995923092,
it can be helpful if the users can read it in rustdoc right below where
the enum's definition is shown.
* Make snippet in rustdoc text
This commit updates a rustdoc code snippet generated for an enum from
`rust,no_run` to `text`. We observed that the snippet fails to compile
in CI because the name of the enum was not fully qualified; code in
rustdoc is treated in a similar way that code in integration tests is
treated as being outside of a crate, but not part of the crate, which
means the name of the crate needs to be spelled out for a `use` statement
if we want to import the enum to the code in rustdoc. However, the name
of the crate is usually one-off, generated on the fly for testing, making
it not obvious to hard-code it or obtain it programmatically from within
Kotlin code.
* Suppress missing doc lint for UnknownVariantValue
This commit marks UnknownVariantValue as `#[allow(missing_docs)]` because
failing to do so will cause tests in CI to fail.
* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt
Co-authored-by: Russell Cohen <rcoh@amazon.com>
* Generate UnknownVariantValue via forInlineFun
This commit attempts to create UnknownVariantValue using
RuntimeType.forInlineFun. Prior to this commit, it was generated per enum
but that caused multiple definitions of UnknownVariantValue in a single
file as there can be multiple enums in the file.
By using RuntimeType.forInlineFun, we can generate a single UnknownVariantValue
per module and the enums within the module can refer to the same instance.
This commit, however, fails to pass the unit tests in EnumGeneratorTest.
The issue seems to be that a RustWriter used in the tests does not end up
calling the finalize method, failing to render inline functions.
* Replace "target == CodegenTarget.CLIENT" with a helper
This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1014390811
but uses an existing helper CodegenTarget.renderUnknownVariant.
* Update EnumGeneratorTests to use TestWorkspace
This commit addresses failures for unit tests in EnumGeneratorTests.
Now that we use RuntimeType.forInlineFcn in EnumGenerator, we need
to ensure that the inline functions should be rendered to a Rust
source file during testing. RustWriter, used in EnumGeneratorTests
prior to this commit, was not capable of doing so. We thus update
EnumGeneratorTests to use TestWorkspace by which we ensure that the
inline functions are rendered during testing.
* Make sure to use the passed-in variable for shapeId
This commit fixes a copy-and-paste error introduced in 712c983. The
function should use the passed-in variable rather than the hard-coded
literal "test#SomeEnum".
* Address https://github.com/awslabs/smithy-rs/pull/1945\#discussion_r1014389852
* Update CHANGELOG.next.toml
* Update CHANGELOG.next.toml
This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1017235315
* Avoid potential name collisions by UnknownVariantValue
This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1017237745.
It changes the module in which the `UnknownVariantValue` is rendered.
Before the commit, it was emitted to the `model` module but that might cause
name collisions in the future when a Smithy model has a shape named
`UnknownVariantValue`. After this commit, we render it to the `types` module.
* Move re-exports from lib.rs to types.rs
This commit moves re-export statements in client crates from `lib.rs` to
`types.rs`. The motivation is that now that we render the struct
`UnknownVariantValue` in a separate file for the `types` module, we can
no longer have a `pub mod types {...}` block in `lib.rs` as it cannot
coexist with `pub mod types;`.
* Add docs on UnknownVariantValue
This commit adds public docs on `UnknownVariantValue`. Since it is
rendered in `types.rs`, the users may not find the `Unknown` variant
of an enum in the same file and may wonder what this struct is for when
seeing it in isolation.
* Update CHANGELOG.next.toml
This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1020414459
* Update the module documentation for types
This commit updates rustdoc for the types module to reflect that fact
that the module now contains not only re-exports but also an auxiliary
struct `UnknownVariantValue`.
* Add extensions to run code block depending on the target
This commit introduces extensions on CodegenTarget that execute the block
of code depending on the target is for client or for server. These
extensions are intended to replace if-else expressions throughout the
codebase explicitly checking whether the target is for client or for server.
We do not update such if-else expressions all at once in this commit.
Rather, we are planning to make incremental changes to update them
opportunistically.
* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Move extensions into CodegenTarget as methods
This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1021952411.
By moving extensions into the CodegenTarget enum, no separate import is
required to use them.
Co-authored-by: Saito <awsaito@c889f3b5ddc4.ant.amazon.com>
Co-authored-by: Russell Cohen <rcoh@amazon.com>
Co-authored-by: Yuki Saito <awsaito@amazon.com>
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* fix: use idiomatic method of setting smithy gradle plugin version (#1995)
* formatting: run code cleanup on entire project (#1993)
* formatting: run code cleanup on entire project
* revert: deletion of SdkSettings.defaultsConfigPath
* Improve `Endpoint` panic-safety and ergonomics (#1984)
- `Endpoint::set_endpoint` no longer panics when called on an endpoint
without a scheme
- `Endpoint::mutable` and `Endpoint::immutable` now both return a result
so that constructing an endpoint without a scheme is an error
- `Endpoint::mutable` and `Endpoint::immutable` both now take a string
instead of a `Uri` as a convenience
- `Endpoint::mutable_uri` and `Endpoint::immutable_uri` were added
to construct an endpoint directly from a `Uri`
* Remove `fromCoreProtocol` in favour of `as` downcasting (#2000)
* Render full shape IDs in server error responses (#1982)
RestJson1 implementations can denote errors in responses in several
ways. New server-side protocol implementations MUST use a header field
named `X-Amzn-Errortype`.
Note that the spec says that implementations SHOULD strip the error
shape ID's namespace. However, our server implementation renders the
full shape ID (including namespace), since some existing clients rely on
it to deserialize the error shape and fail if only the shape name is
present. This is compliant with the spec, see
https://github.com/awslabs/smithy/pull/1493.
See https://github.com/awslabs/smithy/issues/1494 too.
* fix: use new version of deprecated fn (#1994)
update: crate-hasher sha256 dep to 1.1
* update: CargoDependency companion fn names for smithy runtime crates (#1996)
* update: CargoDependency companion fn names for smithy runtime crates
rename: CargoDependency.asType to CargoDependency.toType
fix: errors in InlineDependency doc comment
formatting: run import optimizer for all kotlin files
fix: gradle issue with incorrectly named tasks
* fix: server test broken by removal of rustName
* Improve `Plugin` toolkit (#2003)
Co-authored-by: Julian Antonielli <julianantonielli@gmail.com>
Co-authored-by: Luca Palmieri <20745048+LukeMathWalker@users.noreply.github.com>
* Multiple FromParts in handlers (#1999)
* Multiple FromParts in handlers
Handlers can have up to 8 extensions:
```
pub async fn handler(
input: input::Input,
ext0: Extension<...>,
ext1: Extension<...>,
ext2: Extension<...>,
ext3: Extension<...>,
ext4: Extension<...>,
ext5: Extension<...>,
ext6: Extension<...>,
ext7: Extension<...>,
) -> Result<output::Output, error::Error> { ... }
```
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
Co-authored-by: Harry Barber <hlbarber@amazon.co.uk>
* Tidy up PR template slightly (#2006)
* Re-export service from root of the generated crate (#2010)
* Always write required query param keys, even if value is unset or empty (#1973)
* fix: issues when sending multipart-upload-related operations with no upload ID by making upload ID required
add: Mandatory trait
add: S3 customization to make Upload Ids mandatory
* add: CHANGELOG.next.toml entry
* update: strategy for tagging shapes as mandatory
* remove: Mandatory trait
undo: S3Decorator changes
add: codegen to generate "else" branch after required query string serialization `if let Some` check
add: missing tokio feature to `TokioTest`
* update: mandatory-query-param.rs tests
* format: run cargo fmt
* update: return BuildError for missing required query params
update: required-query-params.rs tests
* formatting: make RustWriter.paramList more readable
fix: code broken by merge from `main`
* fix: handling of enum strings
* add: codegen tests for required query params
update: required enum query param codegen
add: smithy-rs CHANGELOG.next.toml entry
* fix: presigning.rs test
* fix: use `toType` instead of `asType`
fix: indentation in test
* Detect if the `uniqueItems` trait is used (#2001)
And act like in the rest of the unsupported constraint traits cases.
I missed this in the implementation of #1342.
* fix: missing Tokio feature in S3 integration test (#2017)
fix: remove extra "the" in CDK readme
* Collect extractors into `request` module (#2008)
* Add const to generated enum values() (#2011)
* Add const to generated enum values()
Enum values() functions return static arrays of static strings and could
be of compile time use. Make callable in const contexts.
* Add reference to PR in changelog next for const enum values()
* Correct changelog next target to all for const enum values()
Co-authored-by: Luca Palmieri <20745048+LukeMathWalker@users.noreply.github.com>
* Improve code generated documentation (#2019)
Co-authored-by: Luca Palmieri <20745048+LukeMathWalker@users.noreply.github.com>
* Implement `@pattern` on `string` shapes (#1998)
* Refactor `ConstrainedStringGenerator` to extract length checking
* Extract `TryFrom` rendering into its own function
* Extract `ConstraintViolation` definition to separate function
* Add pattern check mock for `@pattern` trait
* Add `regex` to list of server dependencies
* Use real regex check in `check_pattern`
* Add `@pattern` to traits that make `string` shapes directly constrained
* Remove unsupported validation for `@pattern` on strings
* Fix test cases in `ConstraintsTest`
* Remove test disallowing `@pattern` on strings
* Add first test case for `@pattern` on strings
* Fix codegen in `ConstraintViolation::as_validation_exception_field`
* Use `OnceCell` to store `@pattern`'s regex
* Tidy up `ConstrainedStringGenerator` to work with many constraints
* Rename `handleTrait` -> `fromTrait` and make it a static method
* Add docs for `ConstraintViolation` variants
* Fix unescaped slashes in regexes
* Quick hack to get tests compiling
* Fix `sensitive_string_display_implementation` test
* Use new fn name: `asType` -> `toType`
* Refactor `ConstrainedStringGenerator` to not pass in `constraints`
* Add `@pattern` test
* Use `Lazy` instead of `OnceCell`
* Store `&'static str` in `Pattern` error instead of `String`
* Move `TraitInfo` to the bottom
* Extract branches in `TraitInfo.fromTrait` into separate functions
* Add `PatternTrait.validationErrorMessage`
* Add more tests for `@pattern`
* Add entry to `CHANGELOG.next.toml`
* Fix a `@length` + `@pattern` test case
* Remove unused binding in `ConstrainedStringGeneratorTest`
* Remove calls to `trimIndent`
* Use raw Rust strings instead of `escapedPattern`
* Require `regex 1.5.5` instead of `1.7.0`
* Use `Writable`s in `TraitInfo`
* Use single `rust` call for `impl $name` block
* Move `supportedStringConstraintTraits` to `Constraints.kt`
* Fix error message mentioning wrong `ignoreUnsupportedConstraintTraits` key
* Fix usage of `:W` in `rustTemplate` and raw Rust strings
* Use shorthand form for `PatternTrait.validationErrorMessage`
* Fix protocol tests by adjusting the validation message
* Add uses of `@pattern` in `constraints.smithy` model
* Fix broken `RestJsonMalformedPatternReDOSString` test
* Move `TraitInfo` to its own module and separate string-specific details
* Update codegen-core/common-test-models/constraints.smithy
Co-authored-by: david-perez <d@vidp.dev>
* Fix nits in `constraints.smithy`
* Add license to `TraitInfo.kt`
* Make `StringTraitInfo.fromTrait` not return a nullable
* Remove overzealous formatting
* Add some padding to json in `fixRestJsonMalformedPatternReDOSString`
* Add `compile_regex` function for `@pattern` strings
* Add helpful error message when regexes fail to compile
* Add missing coverage in `constraints.smithy`
* Fix examples in `constraints.smithy`
* Fix failing test
* Combine writables in `ConstrainedStringGenerator`
* Fix uses of `Writable.join`
* Use `expect` over `unwrap_or_else`
* Use `once_cell::sync::Lazy` over `once_cell::sync::OnceCell`
Co-authored-by: david-perez <d@vidp.dev>
* Overhaul RustModule system to support nested modules (#1992)
* Overhaul RustModule system to support nested modules
* Cleanups / CR feedback
* Get server unit tests passing
* Fix compilation
* Remove unused import
* CR Feedback II
* customize isn't actually a reserved word—move it to the reserved member section
* Improve service documentation (#2020)
Co-authored-by: Luca Palmieri <20745048+LukeMathWalker@users.noreply.github.com>
* Add module documentation to `plugin.rs`.
* Remove reference to `FromRequest` in `FromParts`.
* Improve `Route` documentation.
* @range implementation for integer shapes (#2005)
* Draft of @range implementation for integer shapes
* Green tests.
* Fix serialization of constrained integers.
* Fix tagger to include Integer simple shapes.
Fix de/serialization codegen.
* Add range trait to the entry about constraint traits in our changelog.
* Bind a range-constrained integer to various HTTP parts to test our implementation.
* Rename ConstrainedIntGeneratorTest to ConstrainedIntegerGeneratorTest for consistency.
* Remove AsRef implementation.
* Fix constraints models.
* Fix conversion.
* Use ReturnSymbolToParse to dry up.
* Fix builder when constrained types should not be exposed.
* Add customisation to unwrap constrained integers.
* Getting closer - collections need to be handled differently to make everything fall into place.
* Refactor `renderHeaders`
* Fix constraints test.
* Fix ebs.
* Rename for clarity.
* Remove unnecessary From implementation.
* Rename `Size` variant to `Range`.
* Remove stray comments.
* Rename for clarity
* Rename for consistency
* Add test to guard against types for which we do not support range yet
* DRY up branches and the relevant tests.
* Fix header name.
* Fix serialization bug for default values in headers.
* Fix serialization issue for primitive headers.
* Remove SetOfRangeInteger
* Fix pubcrateconstrained unit test.
* Remove duplication
* Revert "Remove SetOfRangeInteger"
This reverts commit f37a560bd0233ad65512c4916292f0f7f8c571e1.
* Reintroduce `SetOfRangeInteger`, but keep them commented out.
* Ignore leading whitespace.
* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt
Co-authored-by: david-perez <d@vidp.dev>
* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/parse/JsonParserGenerator.kt
Co-authored-by: david-perez <d@vidp.dev>
* Update codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ValidateUnsupportedConstraints.kt
Co-authored-by: david-perez <d@vidp.dev>
* Update codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ConstrainedIntegerGeneratorTest.kt
Co-authored-by: david-perez <d@vidp.dev>
* Unify with a single rustTemplate invocation.
* Rename `Type` to `NumberType`
* Update codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ConstrainedShapeSymbolProviderTest.kt
Co-authored-by: david-perez <d@vidp.dev>
* Formatting issue.
* Move and rename test helper.
* Dry up the logic for default serialization.
* Ooops, I swapped the two branches.
* Add a wrapper block
* Fix support detection.
* Fix CHANGELOG.
* Add (failing) protocol tests for @range on byte/integer/long.
* Fix validation message.
Fix test definitions.
* Mark some tests as expected to fail.
Rename service.
* Use ValueExpression everywhere for more granular control of the ownership component.
* Use the new enhanced module facilities
* Fixes.
* Fix formatting
* Remove unnecessary when.
* Update codegen-core/common-test-models/constraints.smithy
Co-authored-by: david-perez <d@vidp.dev>
* Remove unused shapes.
* Avoid mixing concerns in our test shapes for integer constraints.
* Reuse the trait info machinery
* Update stale comment
* Fix unused attribute.
* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt
Co-authored-by: Zelda Hessler <zhessler@amazon.com>
* Avoid unnecessary bindings by manipulating the serialization context directly in the customisations.
* Avoid unnecessary bindings in header serialization by introducing and manipulating the header value serialization context directly in the customisations.
* Add code examples.
* Move `safeName` call into the if branch.
Co-authored-by: david-perez <d@vidp.dev>
Co-authored-by: Zelda Hessler <zhessler@amazon.com>
* Fix Canary (#2016)
* add: "release-2022-10-26" to canary runner PINNED_SMITHY_RS_VERSIONS
* update: canary-runner README.md
* Implement @range on short (#2034)
* Extract a ConstrainedNumberGenerator from ConstrainedIntegerGenerator
* Update tests' expectations to assume that we support @range for short
* Add ShortShape to all the key places to get @range support
* Generalize generator tests.
* Add tests for the @range trait on short to our constraints.smithy model.
* Update changelog
* Fix type in constraints.smithy
* Introduce an `aws-lambda` feature. (#2035)
* Introduce an `aws-lambda` feature.
* Add CHANGELOG
* Use the new and shiny feature syntax to avoid creating an implicit `lambda_http` feature.
* Add `aws-lambda` feature to the Python server. Enable the `aws-lambda` feature in our example. Be explicit in providing an implementation of request rejection for Box<dyn Error>.
* Add an `aws-lambda` feature flag to the generated Python-specific crate. We enable it by default to make sure the Lambda method ends up visible in the final Python wrapper library.
* Implement @range on long and byte shapes (#2036)
* Group all override-related test cases together.
* Add @range examples for both byte and long in constraints.smithy.
* Implement @range for long and byte shapes.
* Update CHANGELOG
* Fix unit tests.
* Address comments
* Dry up further.
* Upgrade cargo tools (#2032)
* Upgrade `cargo-check-external-types` to 0.1.6
* Upgrade `cargo-deny` to 0.13.5
* Upgrade `cargo-udeps` to 0.1.35
* Upgrade `cargo-hack` to 0.5.23
* Upgrade `cargo-minimal-versions` to 0.1.8
* `CODEOWNERS`: Move Python related folders to `@awslabs/smithy-rs-python-server` team (#2033)
* Move Python related folders to `@awslabs/smithy-rs-python-server` team
* Add `@awslabs/smithy-rs-server` team to Python related folders
Co-authored-by: Zelda Hessler <zhessler@amazon.com>
* Restore `into_make_service_with_connect_info` module (#2039)
* RFC: RequestId for services (#1942)
* RFC 24 RequestId
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
* Add lambda extractors (#2038)
* Implement `@length` on collections (#2028)
* Refactor `ConstrainedMapGenerator` slightly
* Add `@length` list operation in `constraints.smithy`
* Add more setup required for rendering constraint `list`s
* Add initial draft of `ConstrainedCollectionGenerator`
* Add license header to `LengthTraitCommon`
* Add draft of collection constraint violation generation
Copy `MapCostraintViolationGenerator` into
`CollectionConstraintViolationGenerator` for now.
* Add `Visibility.toRustQualifier`
* Implement `CollectionConstraintViolationGenerator`
* Use `TraitInfo` on `CollectionConstraintViolationGenerator`
* Update docs on `ConstrainedCollectionGenerator`
* Improve formatting on rust code
* Don't generate `ConstraintViolation` enum twice
* Make symbol provider work with constrained lists
* Fix call to `ConstraintViolation::Member`
* Render constraint validation functions for lists
* Fix call to `usize::to_string`
* Use map json customisation for collections as well
* Update to use overhauled module system
* Add constraint traits check for collection shapes
* Remove test checking that `@length` is not used in `list`s
* Refactor use of `shape.isDirectlyConstrained`
* Fix failing `UnconstrainedCollectionGeneratorTest` test
* Fix test
* Actually fix the test
* Update changelog
* Fix `constrainedCollectionCheck`
* Add tests for `ConstrainedCollectionGenerator`
* Make tests work for when sets are implemented
* Remove mention of `set` in changelog
Co-authored-by: david-perez <d@vidp.dev>
* Fix styling in `constraints.smithy`
Co-authored-by: david-perez <d@vidp.dev>
* Use `check` instead of `assert`
* Grammar fix in comment about `Option<Box<_>>`
* Rename unsupported length data class for blobs
- `UnsupportedLengthTraitOnCollectionOrOnBlobShape` -> `UnsupportedLengthTraitOnBlobShape`
* Add TODO about `uniqueItems` not being implemented yet
* Change error message: `unconstrained` -> `not constrained`
* Add imports to fix docs
* Remove unused `modelsModuleWriter` parameter
* Use `filterIsInstance` and coalesce `filter + map`
* Add `check` in json customization
* Use `Set` instead of `List` for supported contraints
* Use `toMutableList` to avoid creating an extra list
* Add `@length` list to `ConA`
* Add `@httpQuery`-bound `@length` list example
* Add `@httpHeader`-bound `@length` list
* Fix `UnconstrainedCollectionGeneratorTest` test
* Fix rendering of constrained lists as header values
* Split very long line
* Add docs for `ConstraintViolation::Member` for lists
* Pass `length` variable name to `LengthTrait.rustCondition`
* Refactor long conditional
* Homogenise conditional
Co-authored-by: david-perez <d@vidp.dev>
* Improve extractor errors (#2041)
Co-authored-by: Luca Palmieri <20745048+LukeMathWalker@users.noreply.github.com>
* Document the new service builder (#2021)
* Document the new service builder
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
* The TODO comment should not be visible in the public docs. (#2045)
* Add server SDK constraint traits RFC (#1199)
* Add Endpoint Resolver Implementation (#2030)
* Add Endpoint Resolver Implementation
This commit adds `EndpointDecorator`, standard libary + tests that can be used to add endpoints 2.0 to the AWS SDK.
It _does not_ actually wire these things up. We'll follow up with a PR that actually integrates everything.
* CR Feedback
* CR feedback II
* Add default for the Smithy test workspace (#1981)
* Add default for the Smithy test workspace
* CR feedback
* Upgrade test SDK models to IDLv2 (#2049)
* Various small corrections to server documentation (#2050)
* Link to super from Handler and OperationService
* Note that structure documentation is from model
* Don't refer to ZSTs in operation_shape.rs opener
* Re-export everything from service to root
* Add reference to root documentation on service
* Fix spelling of MissingOperationsError
* #[doc(hidden)] for opaque_future
* Rename from-parts.md to from_parts.md
* Fix Connected link
* Use S type parameter and service as variable name
* Remove MissingOperation dead code
* Remove Operation header from operation.rs
* Remove reference to closures
* Document ConnectInfo<T> .0
* Add BoxBody documentation
* Rephrase operation.rs documentation
* Reference PluginPipeline from PluginStack
* Move the BoxBody documentation
* Document Plugin associated types
* Add example implementation for Plugin
* Link to plugin module from Plugin
* Improve FromParts/FromRequest documentation
* Remove links to doc(hidden) RuntimeError
* Add link from Upgradable to operation module
* Fix service builder docs (#2046)
* Fix doc prefix in service generator
* Move documentation to root module
* Documentation improvements
* Expose handler imports
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
Co-authored-by: Harry Barber <hlbarber@amazon.co.uk>
* Unhide new service builder and deprecate the prior (#1886)
Co-authored-by: david-perez <d@vidp.dev>
* Normalize URI paths during signing (except for S3) (#2018)
* De-dupe multiple leading forward slashes in request
This commit fixes a bug reported in
https://github.com/awslabs/aws-sdk-rust/issues/661.
Note that this de-duping is not part of URI path normalization because it
must happen to a request for any service, including S3.
* Normalize URI paths for canonical requests
This commit enables URI path normalization in aws-sigv4. It follows
https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html
to create the canonical URI that is part of the canonical request.
* Avoid using OS specific path separator
This commit updates the implementation of normalize_path_segment so that it
does not use an OS specific path separator, specifically "\\" on Windows and
always uses a forward slash "/". It addresses a test failure on Windows:
https://github.com/awslabs/smithy-rs/actions/runs/3518627877/jobs/5897764889.
* Fix the rustdoc::bare_urls warning from cargo doc
* Fix a typo Normalzie -> Normalize
* Limit de-duping multiple leading forward slashes to S3
This commit addresses https://github.com/awslabs/smithy-rs/pull/2018#issuecomment-1325283254.
There is a case where an empty string between leading double forward
slashes is meaningful so blindly de-duping them breaks the use case.
Therefore, handling multiple leading forward slashes in this manner
should be applied only to S3.
* Add an integration test per https://github.com/awslabs/aws-sdk-rust/issues/661
This commit adds an integration test verifying the special rule for URI
path normalization applies to S3. That is, leading forward slashes should
be de-deduped and normalizing URI paths as prescribed in RFC 3986 should
be ignored.
* Update CHANGELOG.next.toml
* Revert 87b749c
This commit reverts 87b749c, the reason being that we need to better
understand the requirement for de-duplicating leading forward slashes
in requests, i.e. do we even need to do it or if so where should we
perform the de-dupe operation?
* Revert naming of UriPathNormalizationMode variants
This commit reverts the naming of variants in UriPathNormalizationMode.
They were PerRfc3986 and ForS3, but they should be more generic like
Enabled and Disabled as we had before.
* Update CHANGELOG.next.toml
* Simplify integration test using capture_request
This commit simplifies the integration test normalize-uri-path for s3.
Previously, the test used ReplayingConnection but it was an overkill
to test something as simple as whether the generated request contained
the expected URI and signature.
* Update aws/rust-runtime/aws-sigv4/src/http_request/settings.rs
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Update aws/rust-runtime/aws-sigv4/src/http_request/settings.rs
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Update aws/rust-runtime/aws-sigv4/src/http_request/uri_path_normalization.rs
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Address review feedback on normalize_path_segment
This commit addresses the following code review feedback:
https://github.com/awslabs/smithy-rs/pull/2018#discussion_r1034192229
https://github.com/awslabs/smithy-rs/pull/2018#discussion_r1034194621
https://github.com/awslabs/smithy-rs/pull/2018#discussion_r1034195625
* Update CHANGELOG.next.toml
This commit addresses https://github.com/awslabs/smithy-rs/pull/2018#discussion_r1034198861
* Preallocate the Vec capacity for normalize_path_segment
This commit addresses https://github.com/awslabs/smithy-rs/pull/2018#discussion_r1034195625.
It counts the number of slashes to preallocate the capacity of a Vec used
in normalize_path_segment.
* Address https://github.com/awslabs/smithy-rs/pull/2018\#discussion_r1034188849
* Update aws/rust-runtime/aws-sigv4/src/http_request/settings.rs
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Update aws/rust-runtime/aws-sigv4/src/http_request/settings.rs
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Update aws/rust-runtime/aws-sigv4/src/http_request/settings.rs
Co-authored-by: John DiSanti <jdisanti@amazon.com>
Co-authored-by: Yuki Saito <awsaito@amazon.com>
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Fix inability to create service clients when `default-features = false` (#2055)
* add: tests for config/client construction when default features are disabled
fix: enable users to create service clients when default features are disabled
update: missing HTTP connector panic messages
* Apply suggestions from code review
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Fix server book documentation links (#2059)
* Respect the sensitive trait on container shapes (#1983)
* Make enum forward-compatible
This commit implements the suggested approach described in
https://github.com/awslabs/smithy-rs/issues/627#issuecomment-995923092
The idea is that once the user writes a match expression against an enum
and assumes that an execution path comes to a particular match arm, we
should guarantee that when the user upgrades a version of SDK, the
execution path should come to the same match arm as before.
* Add unit test to ensure enums are forward-compatible
The test first mimics the user's interaction with the enum generated from
a model where the user writes a match expression on the enum, wishing to
hit the match arm corresponding to Variant3, which is not yet supported
by the model. The test then simulates a scenario where the user now has
access to the updated enum generated from the next version of the model
that does support Variant3.
The user's code should compile in both of the use cases.
* Generate rustdoc for enum's forward-compatibility
This commits generates rustdoc explaining to the users how they might
write a match expression against a generated enum so their code is
forward-compatible. While it is explained in
https://github.com/awslabs/smithy-rs/issues/627#issuecomment-995923092,
it can be helpful if the users can read it in rustdoc right below where
the enum's definition is shown.
* Make snippet in rustdoc text
This commit updates a rustdoc code snippet generated for an enum from
`rust,no_run` to `text`. We observed that the snippet fails to compile
in CI because the name of the enum was not fully qualified; code in
rustdoc is treated in a similar way that code in integration tests is
treated as being outside of a crate, but not part of the crate, which
means the name of the crate needs to be spelled out for a `use` statement
if we want to import the enum to the code in rustdoc. However, the name
of the crate is usually one-off, generated on the fly for testing, making
it not obvious to hard-code it or obtain it programmatically from within
Kotlin code.
* Suppress missing doc lint for UnknownVariantValue
This commit marks UnknownVariantValue as `#[allow(missing_docs)]` because
failing to do so will cause tests in CI to fail.
* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt
Co-authored-by: Russell Cohen <rcoh@amazon.com>
* Generate UnknownVariantValue via forInlineFun
This commit attempts to create UnknownVariantValue using
RuntimeType.forInlineFun. Prior to this commit, it was generated per enum
but that caused multiple definitions of UnknownVariantValue in a single
file as there can be multiple enums in the file.
By using RuntimeType.forInlineFun, we can generate a single UnknownVariantValue
per module and the enums within the module can refer to the same instance.
This commit, however, fails to pass the unit tests in EnumGeneratorTest.
The issue seems to be that a RustWriter used in the tests does not end up
calling the finalize method, failing to render inline functions.
* Replace "target == CodegenTarget.CLIENT" with a helper
This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1014390811
but uses an existing helper CodegenTarget.renderUnknownVariant.
* Update EnumGeneratorTests to use TestWorkspace
This commit addresses failures for unit tests in EnumGeneratorTests.
Now that we use RuntimeType.forInlineFcn in EnumGenerator, we need
to ensure that the inline functions should be rendered to a Rust
source file during testing. RustWriter, used in EnumGeneratorTests
prior to this commit, was not capable of doing so. We thus update
EnumGeneratorTests to use TestWorkspace by which we ensure that the
inline functions are rendered during testing.
* Make sure to use the passed-in variable for shapeId
This commit fixes a copy-and-paste error introduced in 712c983. The
function should use the passed-in variable rather than the hard-coded
literal "test#SomeEnum".
* Address https://github.com/awslabs/smithy-rs/pull/1945\#discussion_r1014389852
* Update CHANGELOG.next.toml
* Update CHANGELOG.next.toml
This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1017235315
* Avoid potential name collisions by UnknownVariantValue
This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1017237745.
It changes the module in which the `UnknownVariantValue` is rendered.
Before the commit, it was emitted to the `model` module but that might cause
name collisions in the future when a Smithy model has a shape named
`UnknownVariantValue`. After this commit, we render it to the `types` module.
* Move re-exports from lib.rs to types.rs
This commit moves re-export statements in client crates from `lib.rs` to
`types.rs`. The motivation is that now that we render the struct
`UnknownVariantValue` in a separate file for the `types` module, we can
no longer have a `pub mod types {...}` block in `lib.rs` as it cannot
coexist with `pub mod types;`.
* Add docs on UnknownVariantValue
This commit adds public docs on `UnknownVariantValue`. Since it is
rendered in `types.rs`, the users may not find the `Unknown` variant
of an enum in the same file and may wonder what this struct is for when
seeing it in isolation.
* Update CHANGELOG.next.toml
This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1020414459
* Update the module documentation for types
This commit updates rustdoc for the types module to reflect that fact
that the module now contains not only re-exports but also an auxiliary
struct `UnknownVariantValue`.
* Add extensions to run code block depending on the target
This commit introduces extensions on CodegenTarget that execute the block
of code depending on the target is for client or for server. These
extensions are intended to replace if-else expressions throughout the
codebase explicitly checking whether the target is for client or for server.
We do not update such if-else expressions all at once in this commit.
Rather, we are planning to make incremental changes to update them
opportunistically.
* Respect the sensitive trait on enums
This commit fixes #1745. It allows enums to respect the sensitive trait.
When annotated as such, an enum will not display its data and instead
will show the redacted text upon debug print.
* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Move extensions into CodegenTarget as methods
This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1021952411.
By moving extensions into the CodegenTarget enum, no separate import is
required to use them.
* Update codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt
Co-authored-by: david-perez <d@vidp.dev>
* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt
Co-authored-by: david-perez <d@vidp.dev>
* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt
Co-authored-by: david-perez <d@vidp.dev>
* Update CHANGELOG.next.toml
* Configure the (in|ex)clusion of the Debug trait for containers per members' sensitive trait (#2029)
* Removes Debug for shape with sensitive trait
This commit centralizes a place where the codegen excludes the Debug
trait if a shape has the sensitive trait. Previously the exclusion was
handled locally in each shape, e.g. StructureGenerator and EnumGenerator.
However, that approach may overlook a certain shape we also need to treat
as such. We now handle the exclusion of the Debug trait in one place,
BaseSymbolMetadataProvider.
* Stop excluding the Debug trait locally
This commit updates EnumGenerator and StructureGenerator based on the
change made to BaseSymbolMetadataProvider in the previous commit.
Now that the exclusion of the Debug trait was centralized, those
classes in question no longer need to do so individually.
* Implement a custom Debug trait in BuilderGenerator
This commit implements a custom Debug trait in BuilderGenerator now that
the derived Debug trait is excluded from BaseSymbolMetadataProvider for
the structure container. The implementation of the custom Debug trait
pretty much follows that of StructureGenerator.
* Implement a custom Debug trait in UnionGenerator
This commit implements a custom Debug trait in BuilderGenerator now that
the derived Debug trait is excluded from BaseSymbolMetadataProvider for
the union container. The implementation of the custom Debug trait pretty
much follows that of EnumGenerator.
* Implement a custom Debug trait in ServerBuilderGenerator
This commit implements a custom Debug trait in ServerBuilderGenerator now
that the derived Debug trait is excluded from BaseSymbolMetadataProvider
for the structure container. The implementation of the custom Debug trait
pretty much follows that of StructureGenerator.
* Add the Copyright header
* Update CHANGELOG.next.toml
* Update Debug impl for UnionGenerator
This commit updates the implementation of a custom Debug trait impl for
UnionGenerator. Turns out that in a Union, a member target can be marked
as sensitive separately outside the Union. Therefore, the implementation
of a custom Debug trait has two cases depending on where the sensitive
trait appears, either it is applied to the whole Union or to a member
target.
* Peek at member sensitivity for Debug trait (in|ex)clusion
This commit addresses https://github.com/awslabs/smithy-rs/pull/2029#discussion_r1034205685.
With this change, structure shapes no longer need to exclude the Debug
trait unconditionally. The upshot is that we may be able to avoid a
custom Debug impl for a structure where the derived Debug will do, i.e.
when there is no sensitive trait either at a container level or at a
member level.
* Remove statement that does not seem to take effect
This commit addresses https://github.com/awslabs/smithy-rs/pull/2029#discussion_r1036288146
* Rename renderDebugImplForUnion -> renderFullyRedactedDebugImpl
This commit addresses https://github.com/awslabs/smithy-rs/pull/2029#discussion_r1036290722
* Rename renderDebugImplForUnionMemberWise -> renderDebugImpl
This commit addresses https://github.com/awslabs/smithy-rs/pull/2029#discussion_r1036291209
Co-authored-by: Yuki Saito <awsaito@amazon.com>
Co-authored-by: Saito <awsaito@c889f3b5ddc4.ant.amazon.com>
Co-authored-by: Russell Cohen <rcoh@amazon.com>
Co-authored-by: Yuki Saito <awsaito@amazon.com>
Co-authored-by: John DiSanti <jdisanti@amazon.com>
Co-authored-by: david-perez <d@vidp.dev>
* Tracing output json (#2060)
* Tracing output json
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
* Remove client body callbacks (#2065)
* Implement FromParts for Option, Result (#2068)
* Implement FromParts for Option, Result
Signed-off-by: Daniele Ahmed <ahmeddan@amazon.de>
* Endpoints 2.0 Integration pre-work (#2063)
* Split endpoint resolution middleware into two parts & refactor endpoint generation
* Endpoints 2.0 Integration pre-work
This PR does a 3 bits of pre-work ahead of ep2 integration:
1. Split endpoint resolution into two separate middlewares:
1. A smithy native middleware that applies URI and headers
2. An AWS middleware that applies the auth schemes
2. Add vendorParams support to the ProtocolTestGenerator so that protocol tests can insert a region.
3. Simplify endpoint resolution logic by allowing `make_operation` to fail when an endpoint cannot be resolved.
* Back out previous change to insert endpoint directly into the bag
* backout changes to property bag
* Update changelog & add more docs
* Fix AWS test
* Fix test
* allow parse_url functions to be unused (#2071)
* Establish default max idle connections on default connectors (#2064)
* Render a Union member of type Unit to an enum variant without inner data (#1989)
* Avoid explicitly emitting Unit type within Union
This commit addresses #1546. Previously, the Unit type in a Union was
rendered as an enum variant whose inner data was crate::model::Unit.
The way such a variant appears in Rust code feels a bit odd as it
usually does not carry inner data for `()`. We now render a Union
member of type Unit to an enum variant without inner data.
* Address test failures washed out in CI
This commit updates places that need to be aware of the Unit type attached
to a Union member. Those places will render the Union member in a way that
the generated Rust code does not include inner data of type Unit. It should
help pass `codegen-client-test:test` and `codegen-server-test:test`.
Further refactoring is required because each place we updated performs an
explicit if-else check for special casing, which we should avoid.
* Update codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGeneratorTest.kt
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Remove commented-out code
* Add a helper for comparing against ShapeId for Unit
This commit adds a helper for checking whether MemberShape targets the
ShapeId of the Unit type. The benefit of the helper is that each call
site does not have to construct a ShapeId for the Unit type every time
comparison is made.
* Move Unit type bifurcation logic to jsonObjectWriter
This commit moves the if-else expression hard-coded in the StructureShape
matching case within RustWriter.serializeMemberValue to jsonObjectWriter.
The previous approach of inlining the if-else expression out of
jsonObjectWriter to the StructureShape case and tailoring it to the call
site violated the DRY principle.
* Make QuerySerializerGenerator in sync with the change
This commit brings the Unit type related change we've made so far to
QuerySerializerGenerator. All tests in CI passed even without this
commit, but noticing how similar QuerySerializerGenerator is to
JsonSerializerGenerator, we should make the former in sync with the
latter.
* Update CHANGELOG.next.toml
* Refactor ofTypeUnit -> isTargetUnit
This commit addresses https://github.com/awslabs/smithy-rs/pull/1989#discussion_r1035372417.
The extension should be renamed to isTargetUnit and moved to Smithy.kt.
* Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/protocols/serialize/JsonSerializerGenerator.kt
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Simplify if-else in jsonObjectWriter
This commit addresses https://github.com/awslabs/smithy-rs/pull/1989#discussion_r1037651893
* Avoid the union member's reference name being empty
This commit addresses https://github.com/awslabs/smithy-rs/pull/1989#discussion_r1037654601
Even if member's reference name "inner" is present, it will not be an
issue when the member is the Unit type where the reference name "inner"
cannot be extracted. The reason is jsonObjectWriter won't render the
serialization function if the member is the Unit type.
That said, the same change in QuerySerializerGenerator may not be the
case so we'll run the tests in CI and see what breaks.
* Ensure Union with Unit target can be serialized
This commit updates serialization of a Union with a Unit target in
different types of serializers. We first updated protocol tests by
adding a new field of type Unit and some of them failed as expected.
We worked our way back from those failed tests and fixed the said
implementation for serialization accordingly.
* Ensure Union with Unit target can be parsed
This commit handles deserialization of a Union with a Unit target in
XmlBindingTraitParserGenerator. Prior to the commit, we already handled
the said deserialization correctly in JsonParserGenerator but missed it
in XmlBindingTraitParserGenerator. We added a new field of type Unit to
a Union in parser protocols tests, ran them, and worked our way back from
the failed tests.
* Ensure match arm for Unit works in custom Debug impl
This commit handles a use case that came up as a result of combining two
updates, implementing a custom Debug impl for a Union and avoid rendering
the Unit type in a Union. In the custom Debug impl, we now make sure that
if the target is of type Unit, we render the corresponding match arm
without the inner data. We add a new unit test in UnionGeneratorTest.
* Fix unused variables warnings in CI
This commit adds the #[allow(unused_variables)] annotation to a block of code
generated for a member being the Unit type. The annotation is put before we
call the handleOptional function so that it covers the whole block that the
function generates.
* Fix E0658 on unused_variables
This commit fixes an error where attributes on expressions are experimental.
It does so by rearranging code in a way that achieves the same effect but
without #[allow(unused_variables)] on an expression.
Co-authored-by: Yuki Saito <awsaito@amazon.com>
Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Improve client tracing spans (#2044)
* Emit spans for implementers of map request middleware traits
* Instrument dispatch with its own span
* Fix trace span hierarchy
* Partially flatten the middleware span hierarchy
* Make `MapRequest::name` required
* Add sub-spans to the `load_response` span
* Add `tracing` events to signing an…
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently a draft PR to wash out test failures in CI.Motivation and Context
Fixes #2042.
Description
This PR adds the functionality for normalizing URI paths as described here. As documented, however, URI path normalization will be ignored for requests against S3.
Due to a difference in the scope, #1308, awslabs/aws-sdk-rust#661 will be addressed in a separate PR.
Testing
normalize_uri_path.Checklist
CHANGELOG.next.tomlif I made changes to the AWS SDK, generated SDK code, or SDK runtime cratesBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.