diff --git a/internal/phpheaders/phpheaders.go b/internal/phpheaders/phpheaders.go index 711d8abc55..3f227620ee 100644 --- a/internal/phpheaders/phpheaders.go +++ b/internal/phpheaders/phpheaders.go @@ -120,10 +120,9 @@ var CommonRequestHeaders = map[string]string{ // Cache up to 256 uncommon headers // This is ~2.5x faster than converting the header each time var ( - headerKeyCache = otter.Must(&otter.Options[string, string]{MaximumSize: 256}) - headerNameReplacer = strings.NewReplacer("-", "_") - loader = otter.LoaderFunc[string, string](func(_ context.Context, key string) (string, error) { - return "HTTP_" + headerNameReplacer.Replace(strings.ToUpper(key)) + "\x00", nil + headerKeyCache = otter.Must(&otter.Options[string, string]{MaximumSize: 256}) + loader = otter.LoaderFunc[string, string](func(_ context.Context, key string) (string, error) { + return headerName(key), nil }) ) @@ -136,3 +135,23 @@ func GetUnCommonHeader(key string) string { return phpHeaderKey } + +// headerName converts a validated ASCII header name to its NUL-terminated PHP variable name. +// net/http rejects invalid header names before they reach the handler. +func headerName(key string) string { + var b strings.Builder + b.Grow(len(key) + len("HTTP_") + 1) + b.WriteString("HTTP_") + for i := range len(key) { + c := key[i] + switch { + case c >= 'a' && c <= 'z': + c -= 'a' - 'A' + case c == '-': + c = '_' + } + b.WriteByte(c) + } + b.WriteByte(0) + return b.String() +} diff --git a/internal/phpheaders/phpheaders_test.go b/internal/phpheaders/phpheaders_test.go index 792eeb4ee3..f89a11d4a9 100644 --- a/internal/phpheaders/phpheaders_test.go +++ b/internal/phpheaders/phpheaders_test.go @@ -1,7 +1,7 @@ package phpheaders import ( - "io" + "bufio" "net" "net/http" "net/http/httptest" @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/net/http/httpguts" ) func TestAllCommonHeadersAreCorrect(t *testing.T) { @@ -26,24 +27,67 @@ func TestAllCommonHeadersAreCorrect(t *testing.T) { } } -// Go's net/http server rejects header names containing spaces with a 400 response -// before the request reaches the handler, so headerNameReplacer does not need to -// translate spaces to underscores. -func TestHeaderWithSpaceIsRejectedByNetHTTP(t *testing.T) { +// Header names arriving over HTTP must be valid ASCII before conversion to PHP variables. +func TestInvalidHeaderNamesAreRejectedByNetHTTP(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { t.Errorf("handler should not be reached, got headers: %v", r.Header) })) t.Cleanup(ts.Close) - conn, err := net.Dial("tcp", strings.TrimPrefix(ts.URL, "http://")) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, conn.Close()) }) + for _, name := range []string{"Bad Header", "X-Ä-Trace", "X-ı-ſ", "X-\xff-Trace", "X-\x00-Trace"} { + t.Run(name, func(t *testing.T) { + conn, err := net.Dial("tcp", strings.TrimPrefix(ts.URL, "http://")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, conn.Close()) }) - _, err = conn.Write([]byte("GET / HTTP/1.1\r\nHost: localhost\r\nBad Header: x\r\nConnection: close\r\n\r\n")) - require.NoError(t, err) + _, err = conn.Write([]byte("GET / HTTP/1.1\r\nHost: localhost\r\n" + name + ": x\r\nConnection: close\r\n\r\n")) + require.NoError(t, err) - resp, err := io.ReadAll(conn) - require.NoError(t, err) - assert.Contains(t, string(resp), "400 Bad Request") - assert.Contains(t, string(resp), "invalid header name") + resp, err := http.ReadResponse(bufio.NewReader(conn), nil) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, resp.Body.Close()) }) + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + }) + } +} + +func TestHeaderName(t *testing.T) { + for _, key := range []string{ + "X-Custom-Trace-Id", "X-CUSTOM-TRACE-ID", "x-custom-trace-id", + "X-With_Underscore.And.Dot", "!#$%&'*+-.^_`|~0123456789", + } { + t.Run(key, func(t *testing.T) { + want := "HTTP_" + strings.NewReplacer("-", "_").Replace(strings.ToUpper(key)) + "\x00" + assert.Equal(t, want, headerName(key)) + }) + } +} + +func FuzzHeaderName(f *testing.F) { + for _, key := range []string{"X-Custom-Trace-Id", "x-custom-trace-id", "!#$%&'*+-.^_`|~0123456789"} { + f.Add(key) + } + replacer := strings.NewReplacer("-", "_") + f.Fuzz(func(t *testing.T, key string) { + if !httpguts.ValidHeaderFieldName(key) { + return + } + want := "HTTP_" + replacer.Replace(strings.ToUpper(key)) + "\x00" + if got := headerName(key); got != want { + t.Fatalf("headerName(%q) = %q, want %q", key, got, want) + } + }) +} + +var headerNameResult string + +func BenchmarkHeaderName(b *testing.B) { + for _, key := range []string{"X-Custom-Trace-Id", "X-CUSTOM-TRACE-ID", "x-custom-trace-id"} { + b.Run(key, func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + headerNameResult = headerName(key) + } + }) + } }