From 9e743cf658e8e59c0a3db023e56bfe6686291134 Mon Sep 17 00:00:00 2001 From: Abhijeet Sharma Date: Fri, 18 Sep 2026 17:15:16 +0530 Subject: [PATCH] fix(requestflag): ignore a leading UTF-8 BOM in YAML/JSON input Windows tools commonly start UTF-8 files with a byte order mark; PowerShell 5.1's Set-Content -Encoding UTF8 is one example. go-yaml decodes that BOM as content, so a request file piped to the CLI stopped being an object: - A JSON body decoded as one string and was sent to the API as a quoted JSON string, with exit status 0. - Adding any body flag failed with "Cannot merge flags with a body that is not a map", followed by what is plainly a map. - Path, query, and header values in the file were ignored, for example "Required flag "response-id" not set" from responses retrieve. - A YAML body kept the BOM on its first key, so "model" was sent as "\ufeffmodel". YAML allows a BOM at the start of a stream, and RFC 8259 lets JSON parsers ignore one. Strip a single leading UTF-8 BOM in UnmarshalYAMLOrJSON, which both stdin and JSON flag values go through. A U+FEFF anywhere else is still content, and nothing else about decoding changes. Upstream: https://github.com/goccy/go-yaml/issues/906 --- cmd/openai/main_stdinbom_test.go | 97 ++++++++++++++++++++++++++ internal/requestflag/unmarshal.go | 7 ++ internal/requestflag/unmarshal_test.go | 55 +++++++++++++++ 3 files changed, 159 insertions(+) create mode 100644 cmd/openai/main_stdinbom_test.go diff --git a/cmd/openai/main_stdinbom_test.go b/cmd/openai/main_stdinbom_test.go new file mode 100644 index 00000000..9350aba5 --- /dev/null +++ b/cmd/openai/main_stdinbom_test.go @@ -0,0 +1,97 @@ +package main + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "reflect" + "testing" +) + +// Windows tools such as PowerShell 5.1's Set-Content -Encoding UTF8 start UTF-8 +// files with a byte order mark. Piped request data must be read the same way +// with or without one. +func TestMainStdinByteOrderMark(t *testing.T) { + type request struct { + path string + body map[string]any + } + requests := make(chan request, 1) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body map[string]any + if r.Method == http.MethodPost { + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Errorf("decode request body: %v", err) + } + } + requests <- request{path: r.URL.Path, body: body} + w.Header().Set("Content-Type", "application/json") + io.WriteString(w, `{"id":"resp_test","object":"response","status":"completed","output":[]}`) + })) + t.Cleanup(server.Close) + + bom := string(rune(0xFEFF)) + for _, tc := range []struct { + name string + stdin string + args []string + wantPath string + wantBody map[string]any + }{ + { + name: "JSON body merged with a flag", + stdin: bom + `{"model":"test-model","input":"test input"}`, + args: []string{"responses", "create", "--instructions", "test instructions"}, + wantPath: "/responses", + wantBody: map[string]any{"model": "test-model", "input": "test input", "instructions": "test instructions"}, + }, + { + name: "YAML body", + stdin: bom + `model: test-model +input: test input +`, + args: []string{"responses", "create"}, + wantPath: "/responses", + wantBody: map[string]any{"model": "test-model", "input": "test input"}, + }, + { + name: "JSON path parameter", + stdin: bom + `{"response_id":"resp_test"}`, + args: []string{"responses", "retrieve"}, + wantPath: "/responses/resp_test", + }, + } { + t.Run(tc.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "request") + if err := os.WriteFile(path, []byte(tc.stdin), 0o600); err != nil { + t.Fatal(err) + } + stdin, err := os.Open(path) + if err != nil { + t.Fatal(err) + } + defer stdin.Close() + + args := append([]string{"openai", "--base-url", server.URL, "--api-key", "sk-test", "--format", "jsonl"}, tc.args...) + got := runMainDispatchWithStdin(t, "", nil, stdin, args...) + if got.code != 0 { + t.Fatalf("main = %+v, want exit code 0", got) + } + + select { + case req := <-requests: + if req.path != tc.wantPath { + t.Errorf("request path = %q, want %q", req.path, tc.wantPath) + } + if tc.wantBody != nil && !reflect.DeepEqual(req.body, tc.wantBody) { + t.Errorf("request body = %#v, want %#v", req.body, tc.wantBody) + } + default: + t.Fatal("main sent no request, want one") + } + }) + } +} diff --git a/internal/requestflag/unmarshal.go b/internal/requestflag/unmarshal.go index 92f96419..ea39b8f9 100644 --- a/internal/requestflag/unmarshal.go +++ b/internal/requestflag/unmarshal.go @@ -15,9 +15,16 @@ import ( // Python, JavaScript, and Go all emit that form for small floats, such as // 1e-05 or 1e-7. func UnmarshalYAMLOrJSON(data []byte, v any) error { + // A YAML stream may start with a byte order mark, and JSON parsers may ignore + // one (RFC 8259, section 8.1). Windows tools often write it, but go-yaml + // decodes it as content (https://github.com/goccy/go-yaml/issues/906). + data = bytes.TrimPrefix(data, utf8BOM) return yaml.Unmarshal(addJSONExponentFractions(data), v) } +// utf8BOM is U+FEFF encoded as UTF-8. +var utf8BOM = []byte{0xEF, 0xBB, 0xBF} + // addJSONExponentFractions rewrites numbers such as 1e-05 in valid JSON as // 1.0e-05, which go-yaml decodes to the same float64. All other input, // including YAML, is returned unchanged so the rest of decoding is unaffected. diff --git a/internal/requestflag/unmarshal_test.go b/internal/requestflag/unmarshal_test.go index 86bc4332..1caf483a 100644 --- a/internal/requestflag/unmarshal_test.go +++ b/internal/requestflag/unmarshal_test.go @@ -114,3 +114,58 @@ func TestJSONExponentNumbersInFlagValues(t *testing.T) { assert.Equal(t, map[string]any{"min": 1e-07}, flag.Get()) }) } + +func TestUnmarshalYAMLOrJSONByteOrderMark(t *testing.T) { + t.Parallel() + + bom := string(utf8BOM) + tests := []struct { + name string + input string + want any + }{ + { + name: "JSON object", + input: bom + `{"model":"test-model","input":"test input"}`, + want: map[string]any{"model": "test-model", "input": "test input"}, + }, + { + name: "YAML mapping keeps its first key", + input: bom + "model: test-model\ninput: test input\n", + want: map[string]any{"model": "test-model", "input": "test input"}, + }, + { + name: "JSON array", + input: bom + `[1,2]`, + want: []any{uint64(1), uint64(2)}, + }, + { + name: "JSON exponent numbers are still decoded as floats", + input: bom + `{"top_p":1e-05}`, + want: map[string]any{"top_p": 1e-05}, + }, + { + name: "U+FEFF inside a value is content", + input: `{"text":"a` + bom + `b"}`, + want: map[string]any{"text": "a" + bom + "b"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var got any + require.NoError(t, UnmarshalYAMLOrJSON([]byte(tt.input), &got)) + assert.Equal(t, tt.want, got) + }) + } + + t.Run("map flag", func(t *testing.T) { + t.Parallel() + + cv := &cliValue[map[string]any]{} + require.NoError(t, cv.Set(bom+`{"format":{"type":"text"}}`)) + assert.Equal(t, map[string]any{"format": map[string]any{"type": "text"}}, cv.Get()) + }) +}