From b01189e7310edafc6d6cb19bc98f7547773fb59d Mon Sep 17 00:00:00 2001 From: Levi Neely Date: Mon, 10 Aug 2026 20:06:35 +0200 Subject: [PATCH] toolsrv: fix multiline content truncation in tool args Bug: spec9p.go was double-processing payloads - parseKV unescaped \n to newlines, then rebuilt the payload without re-escaping, then fs9p.go's parsePayload tried to unescape again. Result: multiline content like file contents was truncated to first line. Fix: Pass raw payload data through to fs9p.go. Escaping/unescaping happens once in parsePayload. Also: - Add proper one-pass unescape function (handles \\n correctly) - Add TestIntegration_MultilineContent to verify fix - Add InputSchema verification to TestIntegration_ToolExecution --- toolsrv/client9p.go | 3 +- toolsrv/fs9p.go | 26 ++++++++++++++-- toolsrv/integration_test.go | 53 ++++++++++++++++++++++++++++++++ toolsrv/spec9p.go | 61 ++++++++++++++++++------------------- 4 files changed, 107 insertions(+), 36 deletions(-) diff --git a/toolsrv/client9p.go b/toolsrv/client9p.go index f6b202a..202b4de 100644 --- a/toolsrv/client9p.go +++ b/toolsrv/client9p.go @@ -131,7 +131,8 @@ func (c *Conn) CallTool(ctx context.Context, name string, args json.RawMessage) payload := fmt.Sprintf("token=%s\ntool=%s\n", c.token, name) for k, v := range argMap { - payload += fmt.Sprintf("%s=%v\n", k, escapeValue(fmt.Sprintf("%v", v))) + escaped := escapeValue(fmt.Sprintf("%v", v)) + payload += fmt.Sprintf("%s=%s\n", k, escaped) } if _, err := fid.Write([]byte(payload)); err != nil { diff --git a/toolsrv/fs9p.go b/toolsrv/fs9p.go index fdeaa0f..05090e9 100644 --- a/toolsrv/fs9p.go +++ b/toolsrv/fs9p.go @@ -401,14 +401,34 @@ func parsePayload(payload string) map[string]string { } key := line[:idx] value := line[idx+1:] - // Unescape \n and \\ - value = strings.ReplaceAll(value, "\\n", "\n") - value = strings.ReplaceAll(value, "\\\\", "\\") + // Unescape \n and \\ in one pass to handle \\n correctly + value = unescapeValue(value) args[key] = value } return args } +// unescapeValue reverses escapeValue: \\n -> newline, \\\\ -> backslash +func unescapeValue(s string) string { + result := make([]byte, 0, len(s)) + for i := 0; i < len(s); i++ { + if s[i] == '\\' && i+1 < len(s) { + switch s[i+1] { + case 'n': + result = append(result, '\n') + i++ + continue + case '\\': + result = append(result, '\\') + i++ + continue + } + } + result = append(result, s[i]) + } + return string(result) +} + // argsToJSON converts the args map to JSON for the existing tool path. func argsToJSON(args map[string]string) []byte { // Build JSON manually to avoid import cycle diff --git a/toolsrv/integration_test.go b/toolsrv/integration_test.go index ed2a35e..086fe14 100644 --- a/toolsrv/integration_test.go +++ b/toolsrv/integration_test.go @@ -279,6 +279,59 @@ func TestIntegration_ToolExecution(t *testing.T) { t.Logf("Tool result: %+v", toolResult) } +func TestIntegration_MultilineContent(t *testing.T) { + // Skip if tools directory doesn't exist (CI environment) + if _, err := os.ReadDir(ToolsPath()); err != nil { + t.Skipf("skipping: tools directory not available: %v", err) + } + + socketPath, cleanup := startTestServer(t) + defer cleanup() + + conn, err := Dial(socketPath, "test-secret-multiline") + if err != nil { + t.Fatalf("Dial failed: %v", err) + } + defer conn.Close() + + // Load the file_write tool + if err := conn.LoadTool("file_write"); err != nil { + t.Skipf("skipping: LoadTool failed (file_write tool not available): %v", err) + } + + // Create a temp file path + tmpFile := filepath.Join(t.TempDir(), "multiline_test.txt") + + // Test with multiline content including special characters + multilineContent := "line one\nline two\nline three with \"quotes\"\nline four with 'single quotes'\nline five with \\backslash\n" + + ctx := context.Background() + args := json.RawMessage(fmt.Sprintf(`{"path": %q, "content": %q}`, tmpFile, multilineContent)) + result, err := conn.CallTool(ctx, "file_write", args) + if err != nil { + t.Fatalf("CallTool failed: %v", err) + } + + var toolResult ToolResult + if err := json.Unmarshal(result, &toolResult); err != nil { + t.Fatalf("failed to parse ToolResult: %v (raw: %s)", err, string(result)) + } + + if toolResult.IsError { + t.Fatalf("file_write returned error: %s", toolResult.Content[0].Text) + } + + // Read the file back and verify + written, err := os.ReadFile(tmpFile) + if err != nil { + t.Fatalf("failed to read written file: %v", err) + } + + if string(written) != multilineContent { + t.Errorf("content mismatch:\nexpected: %q\ngot: %q", multilineContent, string(written)) + } +} + func TestIntegration_ToolExecutionError(t *testing.T) { // Skip if tools directory doesn't exist (CI environment) if _, err := os.ReadDir(ToolsPath()); err != nil { diff --git a/toolsrv/spec9p.go b/toolsrv/spec9p.go index 0c94fac..e2a4b6c 100644 --- a/toolsrv/spec9p.go +++ b/toolsrv/spec9p.go @@ -102,21 +102,9 @@ func handleInfoRead(ctx ToolsrvCtx) ([]byte, error) { } func handleProcNew(ctx ToolsrvCtx, data []byte) ([]byte, error) { - args := parseKV(string(data)) - - // Token verification is now done at connection level via Tauth. - // Just pass through to the FS handler. - delete(args, "token") // remove token from payload if present - - var payload strings.Builder - for k, v := range args { - payload.WriteString(k) - payload.WriteString("=") - payload.WriteString(v) - payload.WriteString("\n") - } - - result, err := ctx.Server.fs.HandleProcNew(context.Background(), payload.String()) + // Pass raw payload through - fs9p.go's parsePayload will handle escaping. + // Token verification is done at connection level via Tauth. + result, err := ctx.Server.fs.HandleProcNew(context.Background(), string(data)) if err != nil { return nil, err } @@ -124,20 +112,9 @@ func handleProcNew(ctx ToolsrvCtx, data []byte) ([]byte, error) { } func handleProcNewBg(ctx ToolsrvCtx, data []byte) error { - args := parseKV(string(data)) - - // Token verification is now done at connection level via Tauth. - delete(args, "token") // remove token from payload if present - - var payload strings.Builder - for k, v := range args { - payload.WriteString(k) - payload.WriteString("=") - payload.WriteString(v) - payload.WriteString("\n") - } - - _, err := ctx.Server.fs.HandleProcNewBg(context.Background(), payload.String()) + // Pass raw payload through - fs9p.go's parsePayload will handle escaping. + // Token verification is done at connection level via Tauth. + _, err := ctx.Server.fs.HandleProcNewBg(context.Background(), string(data)) return err } @@ -214,10 +191,30 @@ func parseKV(input string) map[string]string { } k := line[:idx] v := line[idx+1:] - // Unescape \n and \\ - v = strings.ReplaceAll(v, "\\n", "\n") - v = strings.ReplaceAll(v, "\\\\", "\\") + // Unescape \n and \\ in one pass + v = unescapeKV(v) m[k] = v } return m } + +// unescapeKV reverses escape sequences: \n -> newline, \\ -> backslash +func unescapeKV(s string) string { + result := make([]byte, 0, len(s)) + for i := 0; i < len(s); i++ { + if s[i] == '\\' && i+1 < len(s) { + switch s[i+1] { + case 'n': + result = append(result, '\n') + i++ + continue + case '\\': + result = append(result, '\\') + i++ + continue + } + } + result = append(result, s[i]) + } + return string(result) +}