fix: don't set request body read deadline after the request is finished

frankenphp.go

@@ -643,8 +643,13 @@ func go_read_post(threadIndex C.uintptr_t, cBuf *C.char, countBytes C.size_t) (r
 		return 0
 	}
 
+	// The read deadline is set on the responseWriter, which is only valid until
+	// the response is finished. A script that finishes the request (e.g. via
+	// frankenphp_finish_request()) and then reads the body would otherwise set a
+	// deadline on a finalized HTTP/2 stream, dereferencing a nil pointer and
+	// crashing the process. See https://github.com/php/frankenphp/issues/2535.
 	var rc *http.ResponseController
-	if fc.requestBodyTimeout > 0 {
+	if fc.requestBodyTimeout > 0 && !fc.isDone {
 		if fc.responseController == nil {
 			fc.responseController = http.NewResponseController(fc.responseWriter)
 		}

requestbodytimeout_test.go

@@ -1,18 +1,51 @@
 package frankenphp_test
 
 import (
+	"context"
+	"crypto/tls"
 	"fmt"
 	"io"
 	"net"
 	"net/http"
 	"os"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/dunglas/frankenphp"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/net/http2"
 )
 
+// newH2CServer starts a cleartext HTTP/2 (h2c) server for handler and returns
+// its address and a matching client. h2c drives the SetReadDeadline path that
+// only HTTP/2 exercises. The listener and server are closed via t.Cleanup.
+func newH2CServer(t *testing.T, handler http.HandlerFunc) (addr string, client *http.Client) {
+	t.Helper()
+
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+
+	protocols := new(http.Protocols)
+	protocols.SetUnencryptedHTTP2(true)
+	srv := &http.Server{Handler: handler, Protocols: protocols}
+
+	go func() { _ = srv.Serve(ln) }()
+	t.Cleanup(func() {
+		_ = srv.Close()
+		_ = ln.Close()
+	})
+
+	client = &http.Client{Transport: &http2.Transport{
+		AllowHTTP: true,
+		DialTLSContext: func(_ context.Context, network, addr string, _ *tls.Config) (net.Conn, error) {
+			return net.Dial(network, addr)
+		},
+	}}
+
+	return ln.Addr().String(), client
+}
+
 // TestRequestBodyTimeout proves that WithRequestBodyTimeout bounds a slow-POST
 // client: it announces a large Content-Length, then stalls without sending the
 // body. Without the option the PHP thread would block in Body.Read until the
@@ -59,6 +92,90 @@ func TestRequestBodyTimeout(t *testing.T) {
 	require.Contains(t, string(resp), "read=0")
 }
 
+// TestRequestBodyTimeoutHTTP2 is the HTTP/2 counterpart of the test above: over
+// HTTP/2 the deadline lands on the stream (via x/net) rather than the net.Conn,
+// so this exercises the SetReadDeadline path that HTTP/1 never touches. A slow
+// POST that trips the idle timeout must bound the read and return cleanly.
+// The nil-dereference crash of php/frankenphp#2535 needs the writer to be used
+// after its stream is finalized; see TestSetReadDeadlineRecoversFromPanic.
+func TestRequestBodyTimeoutHTTP2(t *testing.T) {
+	require.NoError(t, frankenphp.Init())
+	defer frankenphp.Shutdown()
+
+	cwd, _ := os.Getwd()
+	handler := func(w http.ResponseWriter, r *http.Request) {
+		req, err := frankenphp.NewRequestWithContext(r,
+			frankenphp.WithRequestDocumentRoot(cwd+"/testdata/", false),
+			frankenphp.WithRequestBodyTimeout(300*time.Millisecond),
+		)
+		require.NoError(t, err)
+		require.NoError(t, frankenphp.ServeHTTP(w, req))
+	}
+
+	addr, client := newH2CServer(t, handler)
+
+	// A body that never sends data: the server blocks in Body.Read until the
+	// idle timeout fires. Close the writer once the request returns.
+	pr, pw := io.Pipe()
+	defer func() { _ = pw.Close() }()
+
+	req, err := http.NewRequest(http.MethodPost, "http://"+addr+"/read-input.php", pr)
+	require.NoError(t, err)
+	req.Header.Set("Content-Type", "application/octet-stream")
+
+	start := time.Now()
+	resp, err := client.Do(req)
+	require.NoError(t, err)
+	defer func() { _ = resp.Body.Close() }()
+
+	body, err := io.ReadAll(resp.Body)
+	require.NoError(t, err)
+	elapsed := time.Since(start)
+
+	require.Less(t, elapsed, 4*time.Second, "slow body must be bounded by the timeout")
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+	require.Contains(t, string(body), "read=0")
+}
+
+// TestFinishRequestThenReadBodyHTTP2 reproduces php/frankenphp#2535: a script
+// that calls frankenphp_finish_request() and then reads php://input triggers
+// go_read_post after the HTTP/2 responseWriter has been finalized. Setting a
+// read deadline on that dead writer would dereference a nil pointer and crash
+// the whole process. enable_post_data_reading=Off defers the body read until
+// the explicit php://input access, i.e. after the request is finished.
+func TestFinishRequestThenReadBodyHTTP2(t *testing.T) {
+	iniDir := t.TempDir()
+	require.NoError(t, os.WriteFile(iniDir+"/php.ini", []byte("enable_post_data_reading=Off\n"), 0o600))
+	t.Setenv("PHPRC", iniDir+"/php.ini")
+
+	require.NoError(t, frankenphp.Init())
+	defer frankenphp.Shutdown()
+
+	cwd, _ := os.Getwd()
+	handler := func(w http.ResponseWriter, r *http.Request) {
+		req, err := frankenphp.NewRequestWithContext(r,
+			frankenphp.WithRequestDocumentRoot(cwd+"/testdata/", false),
+			frankenphp.WithRequestBodyTimeout(300*time.Millisecond),
+		)
+		require.NoError(t, err)
+		require.NoError(t, frankenphp.ServeHTTP(w, req))
+	}
+
+	addr, client := newH2CServer(t, handler)
+
+	req, err := http.NewRequest(http.MethodPost, "http://"+addr+"/finish-then-read-input.php", strings.NewReader("hello world"))
+	require.NoError(t, err)
+	req.Header.Set("Content-Type", "application/octet-stream")
+
+	resp, err := client.Do(req)
+	require.NoError(t, err)
+	defer func() { _ = resp.Body.Close() }()
+
+	_, err = io.ReadAll(resp.Body)
+	require.NoError(t, err)
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+}
+
 // rawServer is a minimal HTTP server exposing its listener address so a test
 // can drive it with a raw TCP connection (needed to simulate a stalled body).
 type rawServer struct {

testdata/finish-then-read-input.php

@@ -0,0 +1,12 @@
+<?php
+
+// Finish the request (sends the response, closes the Go-side context) and only
+// then read the request body. This exercises go_read_post after the HTTP
+// responseWriter has been finalized. See php/frankenphp#2535.
+frankenphp_finish_request();
+
+// Give the Go handler goroutine time to return and finalize the HTTP/2
+// responseWriter before we touch the request body.
+usleep(200000);
+
+file_get_contents('php://input');