feat(http): allow custom OutgoingHandler implementations
#6272
Conversation
rvolosatovs
force-pushed
the
feat/http-config
branch
2 times, most recently
from
9ef5874 to
91bb237
Compare
- Remove unused variables and functions - Fix URI construction Signed-off-by: Roman Volosatovs <[email protected]>
This should be redundant, but is required by the existing Wasmtime design Signed-off-by: Roman Volosatovs <[email protected]>
|
I think it makes the most sense for @pchickey to review this, but Pat, I'm happy to do it if you'd like me to. |
| let mut req = Request::builder() | ||
| .method(method) | ||
| .uri(uri) | ||
| .header(hyper::header::HOST, &request.authority); |
There was a problem hiding this comment.
Should we really be injecting headers on behalf of the caller?
There was a problem hiding this comment.
I agree, the responsibility of setting the Host header should be on the user of this interface, rather than on the implementer. It is both valid and frequently useful to set a different host header than the authority the network connection is made to
There was a problem hiding this comment.
I agree as well, this logic was simply moved from existing implementation, will update
|
one small comment, but generall LGTM. What is the use-case for a user supplying their own handler? Different TLS implementations? |
A custom TLS implementation (e.g. selecting a set of trust roots) is definitely one generally useful use case. Our use case at Cosmonic/wasmCloud is that we need to be able to determine at runtime whether the guest should be allowed to make outgoing HTTP requests at all or not |
|
Not necessary anymore now that we have #7288 |
Remove unused variables and functions
Fix URI construction (partially)
Follow-up on #5929 #6228
cc @brendandburns @pchickey