From b77e6cf6c50dfd624d51fbc84113e4cdbad5644c Mon Sep 17 00:00:00 2001 From: "zhaojunlin.0405" Date: Mon, 6 Jul 2026 17:52:00 +0800 Subject: [PATCH] fix: preserve original filename in multipart file upload BuildFormdata read local files into a bytes.Reader before handing them to the SDK, so the SDK's part-filename detection (which only reads *os.File) fell back to "unknown-file" for every local --file upload. Use AddFileWithName with the file's basename instead. --- cmd/api/api_test.go | 158 +++++++++++++++++++++++++++++++++ cmd/service/service_test.go | 61 +++++++++++++ internal/cmdutil/fileupload.go | 3 +- 3 files changed, 221 insertions(+), 1 deletion(-) diff --git a/cmd/api/api_test.go b/cmd/api/api_test.go index fa91d8912..14d09aaae 100644 --- a/cmd/api/api_test.go +++ b/cmd/api/api_test.go @@ -4,10 +4,14 @@ package api import ( + "bytes" "context" "encoding/json" "errors" + "mime" + "mime/multipart" "os" + "path/filepath" "sort" "strings" "testing" @@ -1054,3 +1058,157 @@ func TestApiCmd_JsonFlag_Accepted(t *testing.T) { t.Errorf("expected method GET, got %s", gotOpts.Method) } } + +// parseMultipartFilenames drives one api --file upload through the mock +// transport and returns a map of field name -> part filename parsed from the +// captured multipart body, plus the map of text form fields. It fails the test +// if the captured request is not multipart/form-data. +func parseMultipartFilenames(t *testing.T, stub *httpmock.Stub) (map[string]string, map[string]string) { + t.Helper() + ct := stub.CapturedHeaders.Get("Content-Type") + mediaType, params, err := mime.ParseMediaType(ct) + if err != nil { + t.Fatalf("parse Content-Type %q: %v", ct, err) + } + if !strings.HasPrefix(mediaType, "multipart/") { + t.Fatalf("Content-Type = %q, want multipart/*", mediaType) + } + filenames := map[string]string{} + fields := map[string]string{} + mr := multipart.NewReader(bytes.NewReader(stub.CapturedBody), params["boundary"]) + for { + part, err := mr.NextPart() + if err != nil { + break + } + if fn := part.FileName(); fn != "" { + filenames[part.FormName()] = fn + } else { + buf := &bytes.Buffer{} + _, _ = buf.ReadFrom(part) + fields[part.FormName()] = buf.String() + } + } + return filenames, fields +} + +func TestApiCmd_FileUpload_PreservesFilename(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu, + }) + + dir := t.TempDir() + cmdutil.TestChdir(t, dir) + if err := os.WriteFile(filepath.Join(dir, "invoice.pdf"), []byte("%PDF-1.4 fake"), 0600); err != nil { + t.Fatalf("write test file: %v", err) + } + + stub := &httpmock.Stub{ + URL: "/open-apis/approval/v4/files/upload", + Body: map[string]interface{}{"code": 0, "msg": "success", "data": map[string]interface{}{"code": "file_xxx"}}, + } + reg.Register(stub) + + cmd := NewCmdApi(f, nil) + cmd.SetArgs([]string{"POST", "/open-apis/approval/v4/files/upload", "--as", "bot", "--file", "invoice.pdf"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + filenames, _ := parseMultipartFilenames(t, stub) + if got := filenames["file"]; got != "invoice.pdf" { + t.Fatalf("part filename for field %q = %q, want %q", "file", got, "invoice.pdf") + } +} + +func TestApiCmd_FileUpload_FieldPrefixKeepsBasename(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu, + }) + + dir := t.TempDir() + cmdutil.TestChdir(t, dir) + if err := os.MkdirAll(filepath.Join(dir, "sub"), 0700); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, "sub", "invoice.pdf"), []byte("%PDF-1.4 fake"), 0600); err != nil { + t.Fatalf("write test file: %v", err) + } + + stub := &httpmock.Stub{ + URL: "/open-apis/approval/v4/files/upload", + Body: map[string]interface{}{"code": 0, "msg": "success", "data": map[string]interface{}{"code": "file_xxx"}}, + } + reg.Register(stub) + + cmd := NewCmdApi(f, nil) + cmd.SetArgs([]string{"POST", "/open-apis/approval/v4/files/upload", "--as", "bot", "--file", "upload=sub/invoice.pdf"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + filenames, _ := parseMultipartFilenames(t, stub) + if _, ok := filenames["upload"]; !ok { + t.Fatalf("expected field name %q from field=path form, got fields %v", "upload", filenames) + } + if got := filenames["upload"]; got != "invoice.pdf" { + t.Fatalf("part filename for field %q = %q, want %q (basename only)", "upload", got, "invoice.pdf") + } +} + +func TestApiCmd_FileUpload_WithDataFields(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu, + }) + + dir := t.TempDir() + cmdutil.TestChdir(t, dir) + if err := os.WriteFile(filepath.Join(dir, "invoice.pdf"), []byte("%PDF-1.4 fake"), 0600); err != nil { + t.Fatalf("write test file: %v", err) + } + + stub := &httpmock.Stub{ + URL: "/open-apis/approval/v4/files/upload", + Body: map[string]interface{}{"code": 0, "msg": "success", "data": map[string]interface{}{"code": "file_xxx"}}, + } + reg.Register(stub) + + cmd := NewCmdApi(f, nil) + cmd.SetArgs([]string{"POST", "/open-apis/approval/v4/files/upload", "--as", "bot", + "--file", "invoice.pdf", "--data", `{"type":"attachment"}`}) + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + filenames, fields := parseMultipartFilenames(t, stub) + if got := filenames["file"]; got != "invoice.pdf" { + t.Fatalf("part filename = %q, want %q", got, "invoice.pdf") + } + if got := fields["type"]; got != "attachment" { + t.Fatalf("text field type = %q, want %q", got, "attachment") + } +} + +func TestApiCmd_FileUpload_StdinFallsBackToUnknown(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu, + }) + f.IOStreams.In = bytes.NewReader([]byte("stdin-bytes")) + + stub := &httpmock.Stub{ + URL: "/open-apis/approval/v4/files/upload", + Body: map[string]interface{}{"code": 0, "msg": "success", "data": map[string]interface{}{"code": "file_xxx"}}, + } + reg.Register(stub) + + cmd := NewCmdApi(f, nil) + cmd.SetArgs([]string{"POST", "/open-apis/approval/v4/files/upload", "--as", "bot", "--file", "-"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + filenames, _ := parseMultipartFilenames(t, stub) + if got := filenames["file"]; got != "unknown-file" { + t.Fatalf("stdin part filename = %q, want %q (no stable local name, fallback)", got, "unknown-file") + } +} diff --git a/cmd/service/service_test.go b/cmd/service/service_test.go index 70d5658dc..1eb6260b9 100644 --- a/cmd/service/service_test.go +++ b/cmd/service/service_test.go @@ -4,10 +4,14 @@ package service import ( + "bytes" "context" "encoding/json" "errors" + "mime" + "mime/multipart" "os" + "path/filepath" "strings" "testing" @@ -1132,6 +1136,63 @@ func TestDetectFileFields(t *testing.T) { } } +// parseMultipartFilenames drives one service-method --file upload through the +// mock transport and returns a map of field name -> part filename parsed from +// the captured multipart body. Mirrors cmd/api's helper of the same name +// (inlined here rather than shared, since the two live in different packages) +// to give BuildFormdata's shared local-file fix a second real entry-point +// covering it. +func parseMultipartFilenames(t *testing.T, stub *httpmock.Stub) map[string]string { + t.Helper() + ct := stub.CapturedHeaders.Get("Content-Type") + mediaType, params, err := mime.ParseMediaType(ct) + if err != nil { + t.Fatalf("parse Content-Type %q: %v", ct, err) + } + if !strings.HasPrefix(mediaType, "multipart/") { + t.Fatalf("Content-Type = %q, want multipart/*", mediaType) + } + filenames := map[string]string{} + mr := multipart.NewReader(bytes.NewReader(stub.CapturedBody), params["boundary"]) + for { + part, err := mr.NextPart() + if err != nil { + break + } + if fn := part.FileName(); fn != "" { + filenames[part.FormName()] = fn + } + } + return filenames +} + +func TestServiceMethod_FileUpload_PreservesFilename(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, testConfig) + + dir := t.TempDir() + cmdutil.TestChdir(t, dir) + if err := os.WriteFile(filepath.Join(dir, "photo.jpg"), []byte("fake-image"), 0600); err != nil { + t.Fatalf("write test file: %v", err) + } + + stub := &httpmock.Stub{ + URL: "/open-apis/im/v1/images", + Body: map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{"image_key": "img_xxx"}}, + } + reg.Register(stub) + + cmd := NewCmdServiceMethod(f, imSpec(), imImageMethod(), "create", "images", nil) + cmd.SetArgs([]string{"--file", "photo.jpg", "--data", `{"image_type":"message"}`, "--as", "bot"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + filenames := parseMultipartFilenames(t, stub) + if got := filenames["image"]; got != "photo.jpg" { + t.Fatalf("part filename for field %q = %q, want %q", "image", got, "photo.jpg") + } +} + func TestServiceMethod_JsonFlag_Accepted(t *testing.T) { f, _, _, _ := cmdutil.TestFactory(t, testConfig) diff --git a/internal/cmdutil/fileupload.go b/internal/cmdutil/fileupload.go index cf6a92d87..37cd8903e 100644 --- a/internal/cmdutil/fileupload.go +++ b/internal/cmdutil/fileupload.go @@ -7,6 +7,7 @@ import ( "bytes" "fmt" "io" + "path/filepath" "strconv" "strings" @@ -128,7 +129,7 @@ func BuildFormdata(fileIO fileio.FileIO, fieldName, filePath string, isStdin boo WithParam("--file"). WithCause(err) } - fd.AddFile(fieldName, bytes.NewReader(data)) + fd.AddFileWithName(fieldName, filepath.Base(filePath), bytes.NewReader(data)) } // Add top-level JSON keys as text form fields.