Reply-From is always deleting TE headers
Prerequisites
- [x] I have written a descriptive issue title
- [x] I have searched existing issues to ensure the bug has not already been reported
Fastify version
5.4.0
Plugin version
12.3.1
Node.js version
24.7
Operating system
Linux
Operating system version (i.e. 20.04, 11.3, 10)
Kernel 6.17
Description
reply-from deliberately deletes a list of headers when forwarding the request to the target server (using HTTP/2, native grpc requires this).
This happens in utils.js, function stripHttp1ConnectionHeaders.
I am currently writing a small proxy with help of reply-from which converts grpc-web to native grpc. Some of the native grpc servers I tested with expect a TE header with value "trailers" TE and refuse to work if this is missing. I added the TE header in rewriteRequestHeaders (the client didn´t provide it) but it didn't work.
This seems also to be in the grpc standard: https://grpc.github.io/grpc/core/md_doc__p_r_o_t_o_c_o_l-_h_t_t_p2.html
Unfortunately using rewriteRequestHeaders does not work, because the stripHttp1ConnectionHeaders happens after.
Currently I work around this with a pnpm patch commenting out the case 'te': in utils.js, which works fine.
Sorry for not providing a minimal example, but I think it is easy to see in the code what happens here. Depending on the accepted solution I would be happy to provide a MR with test to fix the problem.
Link to code that reproduces the bug
See above.
Expected Behavior
TE header should not always be deleted, or make the stripped headers configurable.
Btw, even the gold standard for gprc proxy envoy had this problem before:
https://github.com/triton-inference-server/server/issues/6936
Possible solutions I see:
- remove TE header from the list in stripHttp1ConnectionHeaders
- make rewriteRequestHeaders happen after stripHttp1ConnectionHeaders
A PR would be fantastic.
@mcollina:
So a PR which just removes TE from the stripped connection headers and a regression test which makes sure that it doesn´t get removed anymore would be sufficient?
Ideally yes. In practice it depends why it was added there in the first place (some digging might be necessary).
I think it was just an oversight in the original implementation.
transfer-encoding and te are on subsequent lines in the case in stripHttp1ConnectionHeaders.
After a little research I came up with this:
While transfer-encoding is indeed invalid in HTTP/2, the te header serves a different purpose. It is a request header with which the client tells the server which transfer encodings it accepts. It is definitely valid for HTTP/2 requests and some GRPC server will not work if a te: trailer is not present (which was my problem).
So I think just removing te from the stripped headers is fine. I find it quite unlikely that existing users are affected by this.
I found this place in the Node.js http2 module: isIllegalConnectionSpecificHeader
So TE headers are not illegal for HTTP/2 requests if the value is 'trailers'
I've created a pull request now: https://github.com/fastify/fastify-reply-from/pull/443