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

docs(s2n-tls-hyper): Add hyper client/server example #5069

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

goatgoose
Copy link
Contributor

Resolved issues:

Addresses #5000 (review)

Description of changes:

Adds a client and server example for using s2n-tls with hyper. The example client sends an HTTP request to the server, and the server echos the request body back the client in its response.

Call-outs:

Let me know if I can make anything in the examples more clear!

Testing:

The client/server examples will be built in CI. I ran them locally to make sure they work.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Jan 28, 2025
@goatgoose goatgoose marked this pull request as ready for review January 29, 2025 00:23
@goatgoose goatgoose force-pushed the hyper-example branch 2 times, most recently from f84e8eb to d8b1f65 Compare January 29, 2025 16:09
@goatgoose goatgoose requested a review from maddeleine January 29, 2025 17:37
bindings/rust-examples/Cargo.toml Outdated Show resolved Hide resolved
Comment on lines +15 to +16
struct Args {
#[clap(short, long, default_value = "localhost:1142")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, mostly curious: Why 1142? I'm not sure that's a random choice I've seen before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the last part of my phone number :D

Comment on lines +13 to +15
```
cargo run --bin client -- --body "some text to send to the server"
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Could I run the client without the server? Like cargo run --bin client -- --addr "google.com"?

Right now I think it'd fail because of no echo, but should it maybe just print the response or something instead or asserting the response?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it'd work on other servers too! That makes sense, I'll remove the assertion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I confirmed that it works by testing amazon.com. 🤔

❯ cargo run --bin client -- --addr www.amazon.com
...
</div></body></html>
<!--       _
       .__(.)< (MEOW)
        \___)   
 ~~~~~~~~~~~~~~~~~~-->
<!-- sp:eh:4jC4pgPb20I/tYlRalVa1l4nPSi6Gt0bZQsubiOGQJlqO8NWckvdYiuBYPNtEiusycvzshYbU3Ch/J3giUKgYFukhBylN1bgfwad0xtHjb+r6Cf25uNOaFnyego= -->

Comment on lines +41 to +43
// Enable HTTP/2 by including it in the server's supported ALPN values. The "http2"
// hyper-util feature must also be enabled.
builder.set_application_protocol_preference([b"h2"])?;
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember this was discussed in a previous PR. So we landed on manually setting the alpn via s2n-tls? Idk, this example might make me rethink that choice :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ALPN doesn't need to be manually configured with s2n-tls-hyper. It will attempt to negotiate HTTP/2 by automatically setting the ALPN:

// Attempt to negotiate HTTP/2 by including it in the ALPN extension. Other supported HTTP
// versions are also included to prevent the server from rejecting the TLS connection if
// HTTP/2 isn't supported:
//
// https://datatracker.ietf.org/doc/html/rfc7301#section-3.2
// In the event that the server supports no
// protocols that the client advertises, then the server SHALL respond
// with a fatal "no_application_protocol" alert.
let builder = connection::ModifiedBuilder::new(self.conn_builder.clone(), |conn| {
conn.set_application_protocol_preference([
b"h2".to_vec(),
b"http/1.1".to_vec(),
b"http/1.0".to_vec(),
])
});

This is a hyper server example which directly uses s2n-tls-tokio, not s2n-tls-hyper. s2n-tls-hyper is only used with the hyper client.

@goatgoose goatgoose requested a review from lrstewart January 31, 2025 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants