Skip to content

x/net/http2: should not panic if func "called after Handler finished" #65844

@markus-wa

Description

@markus-wa

The way the stdlib http package handles duplicate calls to WriteHeader is by logging them, e.g.

2024/02/21 14:01:46 http: superfluous response.WriteHeader call from main.main.func1 (main.go:17)

The x/net/http2 pkg instead panics in the same situation (e.g. WriteHeader called after Handler finished).
This is problematic as it makes it more difficult to implement something like a timeout middleware.

Is there a strict reason why x/net/http2 should behave differently to http in this scenario? Or can the implementation be changed to resemble the http behaviour more closely?

http example:

package main

import (
    "context"
    "fmt"
    "net/http"
    "time"
)

// longRunningHandler simulates a task that takes 2 seconds to complete
func longRunningHandler(w http.ResponseWriter, r *http.Request) {
    time.Sleep(2 * time.Second)
    w.WriteHeader(200)
    fmt.Fprintln(w, "Operation completed")
}

// timeoutMiddleware wraps an http.Handler and adds a timeout of 1 second
func timeoutMiddleware(next http.Handler) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        ctx, cancel := context.WithTimeout(r.Context(), 1*time.Second)
        defer cancel()

        finished := make(chan bool)
        go func() {
            next.ServeHTTP(w, r.WithContext(ctx))
            finished <- true
        }()

        select {
        case <-finished:
            // Request finished within timeout
        case <-ctx.Done():
            // Timeout occurred
            http.Error(w, "Request timed out", http.StatusGatewayTimeout)
        }
    })
}

func main() {
    mux := http.NewServeMux()
    mux.Handle("/", timeoutMiddleware(http.HandlerFunc(longRunningHandler)))

    server := &http.Server{
        Addr:    ":8080",
        Handler: mux,
    }

    fmt.Println("Starting http server on :8080")
    if err := server.ListenAndServe(); err != nil {
        fmt.Println("Server error:", err)
    }
}

This works fine with

 curl localhost:8080

x/net/http2 example:

package main

import (
    "context"
    "fmt"
    "net/http"
    "time"

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

// longRunningHandler simulates a task that takes 2 seconds to complete
func longRunningHandler(w http.ResponseWriter, r *http.Request) {
    time.Sleep(2 * time.Second)
    w.WriteHeader(200)
    fmt.Fprintln(w, "Operation completed")
}

// timeoutMiddleware wraps an http.Handler and adds a timeout of 1 second
func timeoutMiddleware(next http.Handler) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        ctx, cancel := context.WithTimeout(r.Context(), 1*time.Second)
        defer cancel()

        finished := make(chan bool)
        go func() {
            next.ServeHTTP(w, r.WithContext(ctx))
            finished <- true
        }()

        select {
        case <-finished:
            // Request finished within timeout
        case <-ctx.Done():
            // Timeout occurred
            http.Error(w, "Request timed out", http.StatusGatewayTimeout)
        }
    })
}

func main() {
    mux := http.NewServeMux()
    mux.Handle("/", timeoutMiddleware(http.HandlerFunc(longRunningHandler)))

    server := &http.Server{
        Addr:    ":8080",
        Handler: h2c.NewHandler(mux, &http2.Server{}),
    }

    fmt.Println("Starting H2C server on :8080")
    if err := server.ListenAndServe(); err != nil {
        fmt.Println("Server error:", err)
    }
}

This crashes the server:

curl --http2-prior-knowledge localhost:8080

This "works" (with warning log):

curl localhost:8080

Metadata

Metadata

Assignees

No one assigned

    Labels

    NeedsDecisionFeedback is required from experts, contributors, and/or the community before a change can be made.

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions