Lcos-000 opened a new issue, #3667:
URL: https://github.com/apache/dubbo-go/issues/3667
## Description
`SendHeader` is documented to "append response headers from a server handler
and send them immediately" (same semantics as `SetHeader`, but flushed on the
first `Send`). However, its implementation merges the user-supplied headers
into `conn.RequestHeader()` rather than `conn.ResponseHeader()`, so the headers
never reach the wire.
## Root cause
`protocol/triple/triple_protocol/header.go:307-314`
```go
func SendHeader(ctx context.Context, header http.Header) error {
conn, ok := ctx.Value(handlerOutgoingKey{}).(StreamingHandlerConn)
if !ok {
return errorf(CodeInternal, "triple: handler outgoing context
not found; SendHeader must be called within a Triple handler")
}
mergeHeaders(conn.RequestHeader(), header) // ← should be
conn.ResponseHeader()
return conn.Send(nil)
}
```
Compare the sibling `SetHeader` at `header.go:263`, which correctly calls
`conn.ResponseHeader()`.
## Impact
Taking the gRPC handler conn (`protocol_grpc.go`) as an example,
`RequestHeader()` returns the inbound `hc.request.Header`, and
`grpcHandlerConn.Send` only flushes `hc.responseHeader` into the response
writer on the first `Send`. As a result:
1. **Response headers are silently dropped** — `SendHeader`'s payload is
never sent to the client, contradicting both its doc comment and the grpc-style
metadata contract.
2. **Request headers are polluted** — the user's response headers are merged
into the shared inbound request header map, so subsequent `RequestHeader()` /
`FromIncomingContext` reads observe headers that were never sent by the client.
This can cause cross-talk / misinterpretation of inbound metadata in downstream
handlers and interceptors.
## Why CI hasn't caught it
There is zero test coverage for `SendHeader`:
```
$ grep -rn "SendHeader" protocol/triple/triple_protocol/*_test.go
# (no matches)
```
`header_test.go` has `TestSetHeader` and `TestSetTrailer` but no
`TestSendHeader`. The existing `mockHandlerConn.RequestHeader()` returns `nil`
(`header_test.go:185`), so even a naive test would not surface the mismatch.
## Suggested fix
One-line change plus a regression test:
```go
func SendHeader(ctx context.Context, header http.Header) error {
conn, ok := ctx.Value(handlerOutgoingKey{}).(StreamingHandlerConn)
if !ok {
return errorf(CodeInternal, "triple: handler outgoing context
not found; SendHeader must be called within a Triple handler")
}
mergeHeaders(conn.ResponseHeader(), header)
return conn.Send(nil)
}
```
And in `header_test.go`, add a `TestSendHeader` that asserts the headers
land in `ResponseHeader()` and that `RequestHeader()` is **not** mutated. The
mock should return a real, distinct `http.Header` from `RequestHeader()` so
pollution can be detected.
## Notes
- Verified on latest `origin/develop` (`a31acfb6`): line 312 still reads
`conn.RequestHeader()`.
- Related PRs (#3633 error return, #3649 / #3663 docs & tests) do not touch
this line.
- I can submit a PR with the fix + test if maintainers agree on the
direction.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
