fix(std): restore the h2c Upgrade handshake the stdlib migration dropped

engine/std/engine.go

@@ -15,6 +15,17 @@ import (
 	"github.com/goceleris/celeris/engine"
 	"github.com/goceleris/celeris/protocol/h2/stream"
 	"github.com/goceleris/celeris/resource"
+
+	"golang.org/x/net/http2"
+	//nolint:staticcheck // SA1019: h2c is deprecated, and its replacement
+	// (http.Server.Protocols + SetUnencryptedHTTP2) does NOT cover the
+	// RFC 7540 3.2 Upgrade handshake -- net/http implements the upgrade path
+	// internally but exposes no way for a caller to reach it. celeris#440
+	// migrated to the stdlib and silently dropped h2c Upgrade from this
+	// engine; the epoll and io_uring engines still honour it, and the
+	// nightly caught the asymmetry. Until the stdlib exposes an equivalent,
+	// this package is the only way to keep the three engines agreeing.
+	"golang.org/x/net/http2/h2c"
 )
 
 // Engine wraps net/http.Server to implement the engine.Engine interface.
@@ -55,9 +66,29 @@ func New(cfg resource.Config, handler stream.Handler) (*Engine, error) {
 
 	bridge := &Bridge{engine: e, handler: handler}
 
+	// h2c through the deprecated handler rather than http.Protocols,
+	// because it is the only one of the two that performs the RFC 7540 3.2
+	// Upgrade handshake. See the import comment: dropping it took the
+	// std engine out of step with epoll and io_uring, which both honour
+	// Config.EnableH2Upgrade, and made the validation harness's h2c-churn
+	// slice vacuous on std -- 3,597 upgrade preambles, not one 101.
+	//
+	// h2c.NewHandler serves BOTH shapes: a client that opens with the
+	// HTTP/2 preface (prior knowledge) and one that asks to upgrade from
+	// HTTP/1.1. Plain HTTP/1.1 requests fall through to the wrapped
+	// handler, so an H2C listener still serves H1.
+	var httpHandler http.Handler = bridge
+	if cfg.Protocol == engine.H2C || cfg.Protocol == engine.Auto {
+		h2s := &http2.Server{
+			MaxConcurrentStreams: cfg.MaxConcurrentStreams,
+			MaxReadFrameSize:     cfg.MaxFrameSize,
+		}
+		httpHandler = h2c.NewHandler(bridge, h2s) //nolint:staticcheck // SA1019: see the import comment -- no stdlib equivalent covers Upgrade.
+	}
+
 	e.server = &http.Server{
 		Addr:              cfg.Addr,
-		Handler:           bridge,
+		Handler:           httpHandler,
 		ReadTimeout:       cfg.ReadTimeout,
 		ReadHeaderTimeout: cfg.ReadHeaderTimeout,
 		WriteTimeout:      cfg.WriteTimeout,
@@ -67,28 +98,6 @@ func New(cfg resource.Config, handler stream.Handler) (*Engine, error) {
 		BaseContext:       func(net.Listener) context.Context { return e.baseCtx },
 	}
 
-	if cfg.Protocol == engine.H2C || cfg.Protocol == engine.Auto {
-		// Cleartext HTTP/2 through net/http's own HTTP/2 server, selected by
-		// Protocols.SetUnencryptedHTTP2. This replaces x/net/http2/h2c, which
-		// is deprecated (celeris#440). HTTP/1.1 stays enabled alongside it so
-		// a plain H1 request on an H2C listener is still served, matching what
-		// h2c.NewHandler did by falling through to the wrapped handler.
-		//
-		// Scope: this covers prior-knowledge h2c (client preface on a fresh
-		// connection). It does NOT cover the RFC 7540 3.2 HTTP/1.1 Upgrade
-		// handshake, which RFC 9113 removed from the specification and
-		// net/http does not implement. The io_uring and epoll engines still
-		// honour Config.EnableH2Upgrade; the std engine no longer does.
-		p := new(http.Protocols)
-		p.SetHTTP1(true)
-		p.SetUnencryptedHTTP2(true)
-		e.server.Protocols = p
-		e.server.HTTP2 = &http.HTTP2Config{
-			MaxConcurrentStreams: int(cfg.MaxConcurrentStreams),
-			MaxReadFrameSize:     int(cfg.MaxFrameSize),
-		}
-	}
-
 	return e, nil
 }
 

engine/std/h2c_upgrade_test.go

@@ -0,0 +1,134 @@
+package std
+
+import (
+	"bufio"
+	"context"
+	"encoding/base64"
+	"fmt"
+	"net"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/goceleris/celeris/engine"
+	"github.com/goceleris/celeris/resource"
+)
+
+// TestStdEngineAnswersH2CUpgradeWith101 is the regression guard for the hole
+// celeris#440 fell through.
+//
+// That change moved this engine off x/net/http2/h2c and onto
+// http.Server.Protocols. The stdlib serves prior-knowledge h2c but performs
+// no RFC 7540 §3.2 Upgrade handshake, so the engine silently stopped
+// answering upgrade requests -- while epoll and io_uring, which implement
+// the handshake themselves via Config.EnableH2Upgrade, carried on. Three
+// engines, two behaviours.
+//
+// Nothing in this repo caught it. test/spec/h2c_upgrade_test.go skips std
+// for every upgrade case, and its comment says why: "std engine uses
+// x/net/http2/h2c middleware; its upgrade path is separate. These tests
+// target the custom engines." The coverage had been delegated to the
+// middleware, so removing the middleware removed the coverage with it. The
+// cluster nightly found it instead -- 3,597 upgrade preambles against
+// kitchen_sink/std on both architectures and not one 101.
+//
+// This test is that missing case, on the engine that actually owns the
+// behaviour.
+func TestStdEngineAnswersH2CUpgradeWith101(t *testing.T) {
+	addr := startH2CEngine(t)
+
+	c, err := net.DialTimeout("tcp", addr, 5*time.Second)
+	if err != nil {
+		t.Fatalf("dial: %v", err)
+	}
+	defer func() { _ = c.Close() }()
+	_ = c.SetDeadline(time.Now().Add(10 * time.Second))
+
+	// RFC 7540 §3.2: an HTTP/1.1 request carrying Upgrade: h2c plus a
+	// base64url SETTINGS payload. An empty SETTINGS frame is valid.
+	settings := base64.RawURLEncoding.EncodeToString(nil)
+	req := fmt.Sprintf("GET / HTTP/1.1\r\nHost: %s\r\n"+
+		"Connection: Upgrade, HTTP2-Settings\r\n"+
+		"Upgrade: h2c\r\n"+
+		"HTTP2-Settings: %s\r\n\r\n", addr, settings)
+	if _, err := c.Write([]byte(req)); err != nil {
+		t.Fatalf("write upgrade request: %v", err)
+	}
+
+	line, err := bufio.NewReader(c).ReadString('\n')
+	if err != nil {
+		t.Fatalf("read status line: %v", err)
+	}
+	if !strings.HasPrefix(line, "HTTP/1.1 101") {
+		t.Fatalf("h2c upgrade got %q, want a 101 Switching Protocols.\n"+
+			"The std engine is not performing the RFC 7540 §3.2 handshake, so it "+
+			"disagrees with epoll and io_uring, which both honour Config.EnableH2Upgrade.",
+			strings.TrimSpace(line))
+	}
+}
+
+// TestStdEngineStillServesPlainHTTP1OnAnH2CListener guards the other half of
+// the contract, which is easy to break while fixing the first: an H2C
+// listener must still answer an ordinary HTTP/1.1 request. h2c.NewHandler
+// gets this right by falling through to the wrapped handler, and a
+// hand-rolled upgrade path is exactly where it would be lost.
+func TestStdEngineStillServesPlainHTTP1OnAnH2CListener(t *testing.T) {
+	addr := startH2CEngine(t)
+
+	c, err := net.DialTimeout("tcp", addr, 5*time.Second)
+	if err != nil {
+		t.Fatalf("dial: %v", err)
+	}
+	defer func() { _ = c.Close() }()
+	_ = c.SetDeadline(time.Now().Add(10 * time.Second))
+
+	if _, err := c.Write([]byte("GET / HTTP/1.1\r\nHost: " + addr + "\r\nConnection: close\r\n\r\n")); err != nil {
+		t.Fatalf("write: %v", err)
+	}
+	line, err := bufio.NewReader(c).ReadString('\n')
+	if err != nil {
+		t.Fatalf("read status line: %v", err)
+	}
+	if !strings.HasPrefix(line, "HTTP/1.1 2") {
+		t.Fatalf("plain HTTP/1.1 on an H2C listener got %q, want a 2xx", strings.TrimSpace(line))
+	}
+}
+
+// startH2CEngine brings up an H2C std engine on a free port and returns its
+// address, shutting it down when the test ends.
+func startH2CEngine(t *testing.T) string {
+	t.Helper()
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	addr := ln.Addr().String()
+	if err := ln.Close(); err != nil {
+		t.Fatalf("close probe listener: %v", err)
+	}
+
+	e, err := New(resource.Config{
+		Addr:     addr,
+		Engine:   engine.Std,
+		Protocol: engine.H2C,
+	}, &echoHandler{})
+	if err != nil {
+		t.Fatalf("New: %v", err)
+	}
+	ctx, cancel := context.WithCancel(context.Background())
+	t.Cleanup(cancel)
+	go func() { _ = e.Listen(ctx) }()
+
+	// Wait for the listener rather than sleeping a fixed amount.
+	deadline := time.Now().Add(10 * time.Second)
+	for time.Now().Before(deadline) {
+		c, derr := net.DialTimeout("tcp", addr, 200*time.Millisecond)
+		if derr == nil {
+			_ = c.Close()
+			return addr
+		}
+		time.Sleep(20 * time.Millisecond)
+	}
+	t.Fatalf("engine did not accept on %s within the deadline", addr)
+	return ""
+}