Skip to content

Commit 7b30ffc

Browse files
rcohjdisanti
andauthored
HTTP Wrapper Type RFC & Implementation (#2912)
## Motivation and Context Implementation of the HTTP request wrapper type from RFC ## Description RFC + implementation of HttpRequest ## Testing - IT/UT - Separate PR which uses this type in implementation of the SDK ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: John DiSanti <[email protected]>
1 parent bf82453 commit 7b30ffc

File tree

8 files changed

+778
-2
lines changed

8 files changed

+778
-2
lines changed

.github/workflows/github-pages.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ jobs:
3737
3838
pushd design &>/dev/null
3939
cargo install mdbook
40-
cargo install mdbook-mermaid
40+
cargo install --locked mdbook-mermaid
4141
mdbook build --dest-dir ../../output
4242
popd &>/dev/null
4343

design/src/SUMMARY.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
- [RFC-0034: Smithy Orchestrator](./rfcs/rfc0034_smithy_orchestrator.md)
6464
- [RFC-0035: Collection Defaults](./rfcs/rfc0035_collection_defaults.md)
6565
- [RFC-0036: HTTP Dependency Exposure](./rfcs/rfc0036_http_dep_elimination.md)
66+
- [RFC-0037: The HTTP Wrapper](./rfcs/rfc0037_http_wrapper.md)
6667

6768
- [Contributing](./contributing/overview.md)
6869
- [Writing and debugging a low-level feature that relies on HTTP](./contributing/writing_and_debugging_a_low-level_feature_that_relies_on_HTTP.md)

design/src/rfcs/overview.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,7 @@
4343
- [RFC-0031: Providing Fallback Credentials on Timeout](./rfc0031_providing_fallback_credentials_on_timeout.md)
4444
- [RFC-0032: Better Constraint Violations](./rfc0032_better_constraint_violations.md)
4545
- [RFC-0033: Improving access to request IDs in SDK clients](./rfc0033_improve_sdk_request_id_access.md)
46+
- [RFC-0034: The Orchestrator Architecture](./rfc0034_smithy_orchestrator.md)
47+
- [RFC-0035: Sensible Defaults for Collection Values](./rfc0035_collection_defaults.md)
48+
- [RFC-0036: Enabling HTTP crate upgrades in the future](./rfc0036_http_dep_elimination.md)
49+
- [RFC-0037: The HTTP wrapper type](./rfc0037_http_wrapper_type.md)
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
<!-- Give your RFC a descriptive name saying what it would accomplish or what feature it defines -->
2+
RFC: The HTTP Wrapper Type
3+
=============
4+
5+
<!-- RFCs start with the "RFC" status and are then either "Implemented" or "Rejected". -->
6+
> Status: RFC
7+
>
8+
> Applies to: client
9+
10+
<!-- A great RFC will include a list of changes at the bottom so that the implementor can be sure they haven't missed anything -->
11+
For a summarized list of proposed changes, see the [Changes Checklist](#changes-checklist) section.
12+
13+
<!-- Insert a short paragraph explaining, at a high level, what this RFC is for -->
14+
This RFC defines the API of our wrapper types around `http::Request` and `http::Response`. For more information about why we are wrapping these types, see [RFC 0036: The HTTP Dependency](./rfc0036_http_dep_elimination.md).
15+
16+
<!-- The "Terminology" section is optional but is really useful for defining the technical terms you're using in the RFC -->
17+
Terminology
18+
-----------
19+
- `Extensions` / "Request Extensions": The `http` crate Request/Response types include a typed property bag to store additional metadata along with the request.
20+
21+
<!-- Explain how users will use this new feature and, if necessary, how this compares to the current user experience -->
22+
The user experience if this RFC is implemented
23+
----------------------------------------------
24+
25+
In the current version of the SDK, external customers and internal code interacts directly with the [`http`](https://crates.io/crates/http) crate. Once this RFC is implemented, interactions at the **public** API level will occur with our own `http` types instead.
26+
27+
Our types aim to be nearly drop-in-compatible for types in the `http` crate, however:
28+
1. We will not expose existing HTTP types in public APIs in ways that are ossified.
29+
2. When possible, we aim to simplify the APIs to make them easier to use.
30+
3. We will add SDK specific helper functionality when appropriate, e.g. first-level support for applying an endpoint to a request.
31+
32+
<!-- Explain the implementation of this new feature -->
33+
How to actually implement this RFC
34+
----------------------------------
35+
36+
We will need to add two types, `HttpRequest` and `HttpResponse`.
37+
38+
#### To string or not to String
39+
Our header library restricts header names and values to `String`s (UTF-8).
40+
41+
Although the `http` library is very precise in its representation—it allows for `HeaderValue`s that are both a super and subset of `String`—a superset because headers support arbitrary binary data but a subset because headers cannot contain control characters like `\n`.
42+
43+
Although technically allowed, headers containing arbitrary binary data are not widely supported. Generally, Smithy protocols will use base-64 encoding when storing binary data in headers.
44+
45+
Finally, it's nicer for users if they can stay in "string land". Because of this, HttpRequest and Response expose header names and values as strings. Internally, the current design uses `HeaderName` and `HeaderValue`, however, there is a gate on construction that enforces that values are valid UTF-8.
46+
47+
**This is a one way door because `.as_str()` would panic in the future if we allow non-string values into headers.**
48+
49+
#### Where should these types live?
50+
These types will be used by all orchestrator functionality, so they will be housed in `aws-smithy-runtime-api`
51+
52+
#### What's in and what's out?
53+
At the onset, these types focus on supporting the most ossified usages: `&mut` modification of HTTP types. They **do not**
54+
support construction of HTTP types, other than `impl From<http::Request>` and `From<http::Response>`. We will also make it
55+
possible to use `http::HeaderName` / `http::HeaderValue` in a zero-cost way.
56+
57+
#### The `AsHeaderComponent` trait
58+
All header insertion methods accept `impl AsHeaderComponent`. This allows us to provide a nice user experience while taking
59+
advantage of zero-cost usage of `'static str`. We will seal this trait to prevent external usage. We will have separate implementation for:
60+
- `&'static str`
61+
- `String`
62+
- http03x::HeaderName
63+
64+
#### Additional Functionality
65+
Our wrapper type will add the following additional functionality:
66+
67+
1. Support for `self.try_clone()`
68+
2. Support for `&mut self.apply_endpoint(...)`
69+
70+
#### Handling failure
71+
There is no stdlib type that cleanly defines what may be placed into headers—String is too broad (even if we restrict to ASCII). This RFC proposes moving fallibility to the APIs:
72+
```rust,ignore
73+
impl HeadersMut<'_> {
74+
pub fn try_insert(
75+
&mut self,
76+
key: impl AsHeaderComponent,
77+
value: impl AsHeaderComponent,
78+
) -> Result<Option<String>, BoxError> {
79+
// ...
80+
}
81+
}
82+
```
83+
84+
This allows us to offer user-friendly types while still avoiding runtime panics. We also offer `insert` and `append` which panic on invalid values.
85+
86+
#### Request Extensions
87+
There is ongoing work which MAY restrict HTTP extensions to clone types. We will preempt that by:
88+
1. Preventing `Extensions` from being present when initially constructing our HTTP request wrapper.
89+
2. Forbidding non-clone extensions from being inserted into the wrapped request.
90+
91+
This also enables supporting request extensions for different downstream providers by allowing cloning into different extension types.
92+
93+
#### Proposed Implementation
94+
<details>
95+
<summary>Proposed Implementation of `request`</summary>
96+
97+
```rust,ignore
98+
{{#include ../../../rust-runtime/aws-smithy-runtime-api/src/client/http/request.rs}}
99+
```
100+
</details>
101+
102+
### Future Work
103+
Currently, the only way to construct `Request` is from a compatible type (e.g. `http03x::Request`)
104+
105+
Changes checklist
106+
-----------------
107+
- [x] Implement initial implementation and test it against the SDK as written
108+
- [ ] Add test suite of `HTTP` wrapper
109+
- [ ] External design review
110+
- [x] Update the SigV4 crate to remove `http` API dependency
111+
- [ ] Update the SDK to use the new type (breaking change)

rust-runtime/aws-smithy-runtime-api/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ aws-smithy-async = { path = "../aws-smithy-async" }
2020
aws-smithy-http = { path = "../aws-smithy-http" }
2121
aws-smithy-types = { path = "../aws-smithy-types" }
2222
bytes = "1"
23-
http = "0.2.3"
23+
http = "0.2.9"
2424
pin-project-lite = "0.2"
2525
tokio = { version = "1.25", features = ["sync"] }
2626
tracing = "0.1"

rust-runtime/aws-smithy-runtime-api/src/client/http.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@
5050
//! [`tower`]: https://crates.io/crates/tower
5151
//! [`aws-smithy-runtime`]: https://crates.io/crates/aws-smithy-runtime
5252
53+
pub mod request;
54+
pub mod response;
55+
5356
use crate::client::orchestrator::{HttpRequest, HttpResponse};
5457
use crate::client::runtime_components::RuntimeComponents;
5558
use crate::impl_shared_conversions;

0 commit comments

Comments
 (0)