fix: replace host header by default #880
Conversation
src/proxy.rs
Outdated
| // forward all inbound headers | ||
|
|
||
| for (key, value) in original_headers { | ||
| let Some(key) = key else { continue }; |
There was a problem hiding this comment.
This drops any header values for a particular header name, after the first one. I noticed this when looking at the API of the iterator for HeaderMap. Yes it's existing behavior, but it has some issues that will definitely bite someone in the future. It can drop some cookies from the request, for example.
An alternative could be to clone the original_headers, and remove HOST from the cloned map.
There was a problem hiding this comment.
Good idea, I'll do that.
|
|
||
| let mut outbound_req = state | ||
| .client | ||
| .request(req.method().clone(), outbound_uri.to_string()) |
There was a problem hiding this comment.
The request method got lost in the refactor.
There was a problem hiding this comment.
Thanks for spotting this!
ctron
force-pushed
the
feature/rework_host_header_1
branch
from
4ed0520
to
f647fd3
Compare
|
I just pushed an update, maybe you can give it another try. |
This also aligns the behavior with the WS proxy behavior and adds header support for WS proxy requests too. It also adds the XFP header. Closes: trunk-rs#879
ctron
force-pushed
the
feature/rework_host_header_1
branch
from
f647fd3
to
18b5cf6
Compare
|
Ok, pushed another update. Fixing stuff. 🤦 |
|
Looks good now, thanks a lot! |
This also aligns the behavior with the WS proxy behavior and adds header support for WS proxy requests too.
It also adds the XFP header.
Closes: #879