Skip to content

feat(client): allow to set a specific sni hostname per request#1171

Open
joelwurtz wants to merge 1 commit intoHFQR:mainfrom
joelwurtz:feat/client-sni-hostname
Open

feat(client): allow to set a specific sni hostname per request#1171
joelwurtz wants to merge 1 commit intoHFQR:mainfrom
joelwurtz:feat/client-sni-hostname

Conversation

@joelwurtz
Copy link
Contributor

@joelwurtz joelwurtz commented Dec 26, 2024 •
edited
Loading

This allow to set a specific hostname for sni, use the request hostname like before by default, but allow to change it if needed.

@joelwurtz joelwurtz marked this pull request as ready for review December 26, 2024 14:49
@joelwurtz joelwurtz force-pushed the feat/client-sni-hostname branch from f1b82f5 to d45c9ff Compare January 9, 2025 13:53
@joelwurtz joelwurtz force-pushed the feat/client-sni-hostname branch 2 times, most recently from 2c9b9e8 to e20d8af Compare February 5, 2025 16:02
@joelwurtz joelwurtz force-pushed the feat/client-sni-hostname branch 2 times, most recently from bf5cce0 to 8f47b24 Compare February 13, 2025 09:32
@fakeshadow
Copy link
Collaborator

fakeshadow commented Feb 14, 2025 •
edited
Loading

Sorry for late review. I have looked into this last month but I cant decide if we should follow this "rule" in all xitca crates:

xitca-web/http/src/http.rs

Line 124 in cec4ef4

// http::Extensions is often brought up as an alternative for extended states but in general

Ideally we want to use http crate for it's public types but also we prefer strongly typed extension than using runtime dynamic type casting. I also can see if we change the client API to use Request<RequestExt<Body>> it can be cumbersome and tiring for library user.

@joelwurtz
Copy link
Contributor Author

joelwurtz commented Feb 15, 2025

Not sure what's the best here, we could either :

  • Use a specific request like the server that add this specific extension instead of the one from http
  • Add specific field to this struct instead of the extensions
  • Or simply make this field external and link it in the service request struct (like the timeout)

It's mainly a api difference when writing middleware, all works, but having those fields in either a specific request or in the extensions of the request reduce future bc break if we want to add more field (so either 1 or 2).

Personally I would go for a specific request struct, where we add all fields that the builder allow to set, extensions point should be only used for data exchange between middleware if needed.

@joelwurtz
Copy link
Contributor Author

joelwurtz commented Feb 25, 2025

There is 3 PR that add information on the service request :

The first two, i did not use the extensions system (because value implement Copy), and put them in the ServiceRequest struct directly, but if we decide to have a specific request struct there may be added there.

Let me know what you want to do for those extra fields and i will adapt others PR accordingly

@joelwurtz joelwurtz force-pushed the feat/client-sni-hostname branch from 8f47b24 to 69fd6bb Compare February 25, 2025 13:11
@joelwurtz joelwurtz mentioned this pull request Mar 6, 2025
Open
@joelwurtz joelwurtz force-pushed the feat/client-sni-hostname branch from 69fd6bb to 82f77b8 Compare January 13, 2026 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@joelwurtz @fakeshadow