Trailers sent twice if no body written

[anuraaga]

• With issues:
  • Use the search tool before opening a new issue.
  • Please provide source code and commit sha if you found a bug.
  • Review existing issues and provide feedback or react to them.

Description

When using c.Header("Trailer", "my-header") and later c.Header("my-header", "value") later, if no data has been written, the value is sent to the client twice, both as a header and a trailer. Any value must only be sent once or it's possible clients get confused.

I found this behavior when using Gin with connect-go+grpc-js, which triggers failure due to duplicate values.

https://github.com/connectrpc/connect-go/blob/main/protocol_grpc.go#L550

How to reproduce

package main

import (
    "net/http"

    "github.com/gin-gonic/gin"
)

func main() {
    app := gin.New()
    app.UseH2C = true

    app.POST(("/*path"), func(c *gin.Context) {
        c.Header("Trailer", "my-status")
        c.Status(http.StatusOK)
        c.Header("my-status", "OK")
    })

    app.Run(":8080")
}

Expectations

❯ curl --http2-prior-knowledge -X POST -H "content-type: application/grpc" -v http://localhost:8080/foo
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
* h2 [:method: POST]
* h2 [:scheme: http]
* h2 [:authority: localhost:8080]
* h2 [:path: /foo]
* h2 [user-agent: curl/8.1.2]
* h2 [accept: */*]
* h2 [content-type: application/grpc]
* Using Stream ID: 1 (easy handle 0x15000f600)
> POST /foo HTTP/2
> Host: localhost:8080
> User-Agent: curl/8.1.2
> Accept: */*
> content-type: application/grpc
>
< HTTP/2 200
< trailer: my-status
< content-length: 0
< date: Mon, 29 Jan 2024 06:03:37 GMT
<
< my-status: OK
* Connection #0 to host localhost left intact

my-status: OK only once in response

Actual result

❯ curl --http2-prior-knowledge -X POST -H "content-type: application/grpc" -v http://localhost:8080/foo
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
* h2 [:method: POST]
* h2 [:scheme: http]
* h2 [:authority: localhost:8080]
* h2 [:path: /foo]
* h2 [user-agent: curl/8.1.2]
* h2 [accept: */*]
* h2 [content-type: application/grpc]
* Using Stream ID: 1 (easy handle 0x15000f600)
> POST /foo HTTP/2
> Host: localhost:8080
> User-Agent: curl/8.1.2
> Accept: */*
> content-type: application/grpc
>
< HTTP/2 200
< my-status: OK
< trailer: my-status
< content-length: 0
< date: Mon, 29 Jan 2024 06:03:37 GMT
<
< my-status: OK
* Connection #0 to host localhost left intact

my-status: OK twice in response.

Environment

• go version: go version go1.21.6 darwin/arm64
• gin version (or commit ref): v1.9.1
• operating system: MacOS

[RedCrazyGhost]

Try manual flush

package main

import (
    "net/http"

    "github.com/gin-gonic/gin"
)

func main() {
    app := gin.New()
    app.UseH2C = true

    app.POST("/*path", func(c *gin.Context) {
        c.Header("Trailer", "my-status")
        c.Status(http.StatusOK)

        c.Writer.Flush()

        c.Header("my-status", "OK")
    })

    app.Run(":8080")
}

  │  ~ ▓▒░ curl --http2-prior-knowledge -X POST -H "content-type: application/grpc" -v http://localhost:8080/foo
* processing: http://localhost:8080/foo
*   Trying [::1]:8080...
* Connected to localhost (::1) port 8080
* h2 [:method: POST]
* h2 [:scheme: http]
* h2 [:authority: localhost:8080]
* h2 [:path: /foo]
* h2 [user-agent: curl/8.2.1]
* h2 [accept: */*]
* h2 [content-type: application/grpc]
* Using Stream ID: 1
> POST /foo HTTP/2
> Host: localhost:8080
> User-Agent: curl/8.2.1
> Accept: */*
> content-type: application/grpc
> 
< HTTP/2 200 
< trailer: my-status
< date: Wed, 31 Jan 2024 17:06:38 GMT
< 
< my-status: OK
* Connection #0 to host localhost left intact

[anuraaga]

Ah forgot to mention it, yes flush works as a workaround. But it seems like a bug to require it, in comparison with normal http servemux, trailers would only be sent once even without flush.

[kbiits]

I think it's not a bug. That's how gin works, you need to flush the buffer to write response to client. This is equivalent code when using net/http lib.

package main

import (
    "net/http"

    "golang.org/x/net/http2"
    "golang.org/x/net/http2/h2c"
)

func main() {
    srv := http2.Server{}
    handler := h2c.NewHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

        w.Header().Add("Trailer", "my-status")
        w.WriteHeader(200)

        w.Header().Add("my-status", "OK")
    }), &srv)

    http.ListenAndServe(":8080", handler)
}

And it works as what you expected, the my-status header only sent once. That's because when calling w.WriteHeader in net/http, it will directly write the headers to client. With gin, it doesn't work like that. Gin need to buffer the header and body that you write in your handler even if you already call the c.Status.

[anuraaga]

I don't think the issue is really about timing, if it gets buffered and sent as a response header instead of a trailer, it could be ok though a bit weird.

The main issue is the value is getting sent twice when flush isn't used - the user code only calls Header once so having two values sent isn't reflecting that. There seems to be a bug that gin writes that header both in response header phase and trailer phase.

[kbiits]

There seems to be a bug that gin writes that header both in response header phase and trailer phase.

No, gin just buffers the headers and proxy it to http2 server that writes the header directly to the connection. The problem is gin doesn't handle and check your trailer headers, it got bufferred and send it to http2 server.

That's why I think gin actually need to introduce a new method to set trailers, or just adding new logic in Header call to check if client code trying to set trailer headers