diff --git a/frankenphp.go b/frankenphp.go index e560031e62..3f5df594d6 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -783,16 +783,53 @@ func go_read_cookies(threadIndex C.uintptr_t) *C.char { return nil } - cookie := strings.Join(request.Header.Values("Cookie"), "; ") - if cookie == "" { + return cookieHeader(request.Header.Values("Cookie")) +} + +// cookieHeader joins Cookie fields and removes NUL bytes directly in the final +// C buffer, which is freed in frankenphp_free_request_context(). +func cookieHeader(values []string) *C.char { + const maxInt = int(^uint(0) >> 1) + rawLen, size := 0, 0 + if len(values) > 1 { + if 2 >= maxInt/(len(values)-1) { + panic("strings: Join output length overflow") + } + rawLen = 2 * (len(values) - 1) + size = rawLen + } + for _, value := range values { + if len(value) > maxInt-rawLen { + panic("strings: Join output length overflow") + } + rawLen += len(value) + size += len(value) - strings.Count(value, "\x00") + } + // Preserve the distinction between no cookie and a nonempty field that + // becomes empty after removing NUL bytes. + if rawLen == 0 { return nil } - // remove potential null bytes - cookie = strings.ReplaceAll(cookie, "\x00", "") - - // freed in frankenphp_free_request_context() - return C.CString(cookie) + ptr := C.malloc(C.size_t(size + 1)) + buf := unsafe.Slice((*byte)(ptr), size+1) + pos := 0 + for i, value := range values { + if i > 0 { + pos += copy(buf[pos:], "; ") + } + for { + j := strings.IndexByte(value, 0) + if j < 0 { + pos += copy(buf[pos:], value) + break + } + pos += copy(buf[pos:], value[:j]) + value = value[j+1:] + } + } + buf[pos] = 0 + return (*C.char)(ptr) } // getLogger returns the logger and context safely even if phpThreads have not been created yet diff --git a/frankenphp_test.go b/frankenphp_test.go index 84199ba81d..5d37adab27 100644 --- a/frankenphp_test.go +++ b/frankenphp_test.go @@ -428,6 +428,45 @@ func TestMalformedCookie(t *testing.T) { }, &testOptions{nbParallelRequests: 1}) } +func TestCookieHeaderFields(t *testing.T) { + for _, workerScript := range []string{"", "cookies.php"} { + mode := "module" + if workerScript != "" { + mode = "worker" + } + t.Run(mode, func(t *testing.T) { + for _, tc := range []struct { + name string + fields []string + want []string + }{ + {"absent", nil, []string{"array (\n)"}}, + {"empty", []string{""}, []string{"array (\n)"}}, + {"empty fields", []string{"", ""}, []string{"array (\n)"}}, + {"NUL only", []string{"\x00\x00"}, []string{"array (\n)"}}, + {"multiple fields", []string{"foo=bar", "second=test"}, []string{"'foo' => 'bar'", "'second' => 'test'"}}, + {"empty separators", []string{"", "foo=bar", ""}, []string{"'foo' => 'bar'"}}, + {"NUL within values", []string{"foo=ba\x00r", "second=te\x00st"}, []string{"'foo' => 'bar'", "'second' => 'test'"}}, + {"first duplicate wins", []string{"foo=first", "foo=second"}, []string{"'foo' => 'first'"}}, + {"long values", []string{"foo=" + strings.Repeat("a", 256), "second=" + strings.Repeat("b", 256)}, []string{"'foo' => '" + strings.Repeat("a", 256) + "'", "'second' => '" + strings.Repeat("b", 256) + "'"}}, + } { + t.Run(tc.name, func(t *testing.T) { + runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) { + req := httptest.NewRequest("GET", "http://example.com/cookies.php", nil) + for _, field := range tc.fields { + req.Header.Add("Cookie", field) + } + body, _ := testRequest(req, handler, t) + for _, want := range tc.want { + assert.Contains(t, body, want) + } + }, &testOptions{workerScript: workerScript, nbParallelRequests: 1}) + }) + } + }) + } +} + func TestSession_module(t *testing.T) { testSession(t, nil) } func TestSession_worker(t *testing.T) { testSession(t, &testOptions{workerScript: "session.php"})