diff --git a/cmd/api.go b/cmd/api.go index 4dbb639..eefc188 100644 --- a/cmd/api.go +++ b/cmd/api.go @@ -23,7 +23,12 @@ var apiCmd = &cobra.Command{ The path argument is an API endpoint, e.g. /v1/environments/{environment_id}/campaigns. Placeholders like {environment_id} are substituted from --params. The HTTP method -defaults to GET (or POST if --json is provided); override with -X/--method. +defaults to GET (or POST if --json or --file is provided); override with -X/--method. + +Endpoints that take a file accept it through --file, which sends the request as +multipart/form-data. --json then supplies the request's other form fields instead +of a JSON body. Large uploads can outrun the default 30s budget; raise it with +--timeout. All standard flags work: --jq, --dry-run, --page-all, --page, --limit. @@ -34,13 +39,15 @@ Examples: cio api /v1/environments/{environment_id}/campaigns/{campaign_id} --params '{"environment_id": "456", "campaign_id": "789"}' cio api /v1/environments/{environment_id}/campaigns -X POST --params '{"environment_id": "456"}' --json '{"campaign": {"name": "Test"}}' cio api /v1/accounts/{account_id} --params '{"account_id": "123"}' - cio api /v1/environments/{environment_id}/segments --params '{"environment_id": "456"}' --dry-run`, + cio api /v1/environments/{environment_id}/segments --params '{"environment_id": "456"}' --dry-run + cio api /v1/environments/{environment_id}/knowledge_source_library/upload --params '{"environment_id": "456"}' --file @runbook.md --json '{"name": "Ops runbook"}' --timeout 120s`, Args: cobra.ExactArgs(1), RunE: runAPI, } func init() { - apiCmd.Flags().StringP("method", "X", "", "HTTP method (default: GET, or POST if --json is provided)") + apiCmd.Flags().StringP("method", "X", "", "HTTP method (default: GET, or POST if --json or --file is provided)") + apiCmd.Flags().StringArray("file", nil, "Send the request as multipart/form-data with a file part: --file @path, or --file field=@path to name the part (repeatable). --json then supplies the request's other form fields") rootCmd.AddCommand(apiCmd) } @@ -67,7 +74,13 @@ func runAPI(cmd *cobra.Command, args []string) error { return err } - httpMethod := resolveMethod(methodFlag, jsonBody) + fileParts, err := GetFileParts(cmd) + if err != nil { + output.PrintError(output.CodeValidationError, err.Error(), nil) + return err + } + + httpMethod := resolveMethod(methodFlag, jsonBody != nil || len(fileParts) > 0) // Parse --params: separate path params from query params. paramsRaw, _ := cmd.Flags().GetString("params") @@ -102,19 +115,31 @@ func runAPI(cmd *cobra.Command, args []string) error { jq := GetJQFlag(cmd) + // Ahead of the dry run: reporting "valid" for a combination the real run + // rejects is worse than no check. + if _, _, pageAllFlag := GetPaginationFlags(cmd); pageAllFlag && len(fileParts) > 0 { + err := fmt.Errorf("--page-all cannot be combined with --file") + output.PrintError(output.CodeValidationError, err.Error(), nil) + return err + } + // Dry run. if GetDryRun(cmd) { apiURL, _ := cmd.Flags().GetString("api-url") if apiURL == "" { apiURL = c.BaseURL() } + contentType := "application/json" + if len(fileParts) > 0 { + contentType = "multipart/form-data" + } dryRun := map[string]any{ "dry_run": true, "method": httpMethod, "url": apiURL + resolvedPath, "headers": map[string]string{ "Authorization": "Bearer [REDACTED]", - "Content-Type": "application/json", + "Content-Type": contentType, }, "validation": map[string]any{ "valid": true, @@ -124,7 +149,18 @@ func runAPI(cmd *cobra.Command, args []string) error { if len(queryParams) > 0 { dryRun["params"] = queryParams } - if jsonBody != nil { + if len(fileParts) > 0 { + // Names and sizes only — a dry run must not spill file contents. + dryRun["files"] = filePartsSummary(fileParts) + fields, err := formFieldsFromJSON(jsonBody) + if err != nil { + output.PrintError(output.CodeValidationError, err.Error(), nil) + return err + } + if len(fields) > 0 { + dryRun["fields"] = fields + } + } else if jsonBody != nil { dryRun["body"] = json.RawMessage(jsonBody) } return output.FprintJSON(cmd.OutOrStdout(), dryRun) @@ -146,7 +182,13 @@ func runAPI(cmd *cobra.Command, args []string) error { return doPageAll(cmd, c, resolvedPath, queryParams, page, limit) } - result, err := c.Do(cmd.Context(), httpMethod, resolvedPath, queryParams, jsonBody) + body, err := requestBody(jsonBody, fileParts) + if err != nil { + output.PrintError(output.CodeValidationError, err.Error(), nil) + return err + } + + result, err := c.DoWithBody(cmd.Context(), httpMethod, resolvedPath, queryParams, body) if err != nil { return handleAPIError(err) } @@ -154,12 +196,26 @@ func runAPI(cmd *cobra.Command, args []string) error { return output.FprintProcess(cmd.OutOrStdout(), result, jq, GetRawFlag(cmd)) } +func requestBody(jsonBody json.RawMessage, fileParts []client.FilePart) (*client.Body, error) { + if len(fileParts) == 0 { + if jsonBody == nil { + return nil, nil + } + return &client.Body{ContentType: "application/json", Bytes: jsonBody}, nil + } + fields, err := formFieldsFromJSON(jsonBody) + if err != nil { + return nil, err + } + return client.NewMultipartBody(fileParts, fields) +} + // resolveMethod determines the HTTP method from the flag or defaults. -func resolveMethod(flag string, body []byte) string { +func resolveMethod(flag string, hasBody bool) string { if flag != "" { return strings.ToUpper(flag) } - if body != nil { + if hasBody { return "POST" } return "GET" diff --git a/cmd/api_upload.go b/cmd/api_upload.go new file mode 100644 index 0000000..a0e3475 --- /dev/null +++ b/cmd/api_upload.go @@ -0,0 +1,143 @@ +package cmd + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" + + "github.com/customerio/cli/internal/client" + "github.com/spf13/cobra" +) + +// Matches the field every upload endpoint names its file part. +const defaultFilePartField = "file" + +// [] is the conventional encoding for a repeated (multi-file) field. +var filePartFieldRegex = regexp.MustCompile(`^[a-zA-Z0-9_-]+(\[\])?$`) + +func GetFileParts(cmd *cobra.Command) ([]client.FilePart, error) { + bindings, _ := cmd.Flags().GetStringArray("file") + if len(bindings) == 0 { + return nil, nil + } + + parts := make([]client.FilePart, 0, len(bindings)) + seen := make(map[string]bool, len(bindings)) + for _, binding := range bindings { + field, path, err := splitFileBinding(binding) + if err != nil { + return nil, err + } + if seen[field] && !strings.HasSuffix(field, "[]") { + return nil, fmt.Errorf("--file %s: field %q given more than once; name it %s[] to send several files under one field", binding, field, field) + } + seen[field] = true + + content, err := readUploadFile(path) + if err != nil { + return nil, fmt.Errorf("--file %s: %w", binding, err) + } + parts = append(parts, client.FilePart{ + Field: field, + Filename: filepath.Base(path), + Content: content, + }) + } + return parts, nil +} + +// Bounded: a bare os.ReadFile would pull a mistyped path at a huge file entirely +// into memory only to reject it for size. +func readUploadFile(path string) ([]byte, error) { + fh, err := os.Open(path) + if err != nil { + return nil, err + } + defer func() { _ = fh.Close() }() + + content, err := io.ReadAll(io.LimitReader(fh, client.MaxUploadBytes+1)) + if err != nil { + return nil, err + } + if len(content) == 0 { + return nil, fmt.Errorf("file is empty") + } + if len(content) > client.MaxUploadBytes { + return nil, fmt.Errorf("file is over the %d byte upload limit", client.MaxUploadBytes) + } + return content, nil +} + +func splitFileBinding(binding string) (field, path string, err error) { + if binding == "" { + return "", "", fmt.Errorf("--file: missing value, expected [field=]@path") + } + // A leading @ means the whole value is a path, so a filename containing '=' + // is not mistaken for a field binding. + if rest, ok := strings.CutPrefix(binding, "@"); ok { + field, path = defaultFilePartField, rest + } else if name, value, found := strings.Cut(binding, "="); found { + field, path = name, strings.TrimPrefix(value, "@") + } else { + field, path = defaultFilePartField, binding + } + + if !filePartFieldRegex.MatchString(field) { + return "", "", fmt.Errorf("--file %s: field name %q may use letters, digits, underscores and hyphens, with an optional trailing [] for a repeated field", binding, field) + } + if path == "" { + return "", "", fmt.Errorf("--file %s: missing filename", binding) + } + return field, path, nil +} + +// Reusing --json spares an upload a second flag for its metadata. Scalars only: +// a nested value has no unambiguous form representation. +func formFieldsFromJSON(body json.RawMessage) (map[string]string, error) { + if len(body) == 0 { + return nil, nil + } + // UseNumber, not the default float64: a resource ID like 1234567890123456789 + // would otherwise be sent as 1234567890123456800 — wrong but plausible. + dec := json.NewDecoder(bytes.NewReader(body)) + dec.UseNumber() + var raw map[string]any + if err := dec.Decode(&raw); err != nil { + return nil, fmt.Errorf("--json must be an object when --file is used: %w", err) + } + fields := make(map[string]string, len(raw)) + for name, v := range raw { + switch value := v.(type) { + case string: + fields[name] = value + case bool: + fields[name] = strconv.FormatBool(value) + case json.Number: + fields[name] = value.String() + case nil: + fields[name] = "" + default: + return nil, fmt.Errorf("--json field %q is not a string, number or boolean; a multipart request cannot carry nested values", name) + } + } + return fields, nil +} + +// Names and sizes only: a dry run must not echo file contents. +func filePartsSummary(parts []client.FilePart) []map[string]any { + out := make([]map[string]any, 0, len(parts)) + for _, p := range parts { + out = append(out, map[string]any{ + "field": p.Field, + "filename": p.Filename, + "size": len(p.Content), + }) + } + return out +} diff --git a/cmd/api_upload_test.go b/cmd/api_upload_test.go new file mode 100644 index 0000000..e1d8040 --- /dev/null +++ b/cmd/api_upload_test.go @@ -0,0 +1,343 @@ +package cmd + +import ( + "encoding/json" + "io" + "mime" + "mime/multipart" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/customerio/cli/internal/client" +) + +// Echoes the parts back so a test asserts on the wire format, not just on flag parsing. +func uploadServer(t *testing.T) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/v1/service_accounts/oauth/token" { + _, _ = w.Write([]byte(`{"access_token":"jwt-test-session","token_type":"Bearer","expires_in":3600}`)) + return + } + if r.Header.Get("Authorization") != "Bearer jwt-test-session" { + w.WriteHeader(http.StatusUnauthorized) + return + } + + mediaType, params, err := mime.ParseMediaType(r.Header.Get("Content-Type")) + if err != nil || mediaType != "multipart/form-data" { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(`{"error":"not multipart"}`)) + return + } + + out := map[string]any{"method": r.Method, "path": r.URL.Path} + // Keyed by a slice: a repeated field name sends several parts, and keying + // by name alone would silently drop all but the last. + files := map[string][]any{} + fields := map[string]string{} + mr := multipart.NewReader(r.Body, params["boundary"]) + for { + part, err := mr.NextPart() + if err == io.EOF { + break + } + if err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + body, _ := io.ReadAll(part) + if part.FileName() != "" { + files[part.FormName()] = append(files[part.FormName()], map[string]any{ + "filename": part.FileName(), + "content": string(body), + "content_type": part.Header.Get("Content-Type"), + }) + } else { + fields[part.FormName()] = string(body) + } + } + out["files"] = files + out["fields"] = fields + data, _ := json.Marshal(out) + _, _ = w.Write(data) + })) +} + +func writeTempFile(t *testing.T, name, content string) string { + t.Helper() + path := filepath.Join(t.TempDir(), name) + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatalf("write %s: %v", name, err) + } + return path +} + +func TestAPIFileUpload_SendsMultipart(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("CIO_TOKEN", "sa_live_test123") + t.Setenv("CIO_ACCESS_TOKEN", "") + server := uploadServer(t) + defer server.Close() + + path := writeTempFile(t, "runbook.md", "# Runbook\n") + stdout, _, err := executeCommand("api", "/v1/environments/{environment_id}/knowledge_source_library/upload", + "--params", `{"environment_id":"456"}`, + "--file", "@"+path, + "--json", `{"name":"Ops runbook","description":"how to"}`, + "--api-url", server.URL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var got struct { + Method string `json:"method"` + Path string `json:"path"` + Files map[string][]struct { + Filename string `json:"filename"` + Content string `json:"content"` + ContentType string `json:"content_type"` + } `json:"files"` + Fields map[string]string `json:"fields"` + } + if err := json.Unmarshal([]byte(stdout), &got); err != nil { + t.Fatalf("parse response: %v (%s)", err, stdout) + } + + if got.Method != "POST" { + t.Errorf("method = %q, want POST (--file should default the method)", got.Method) + } + if got.Path != "/v1/environments/456/knowledge_source_library/upload" { + t.Errorf("path = %q", got.Path) + } + parts, ok := got.Files["file"] + if !ok || len(parts) != 1 { + t.Fatalf("want exactly one part named \"file\": %+v", got.Files) + } + file := parts[0] + if file.Filename != "runbook.md" { + t.Errorf("filename = %q, want runbook.md", file.Filename) + } + if file.Content != "# Runbook\n" { + t.Errorf("content = %q", file.Content) + } + if got.Fields["name"] != "Ops runbook" || got.Fields["description"] != "how to" { + t.Errorf("fields = %+v, want --json to become form fields", got.Fields) + } +} + +func TestAPIFileUpload_NamedPartField(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("CIO_TOKEN", "sa_live_test123") + t.Setenv("CIO_ACCESS_TOKEN", "") + server := uploadServer(t) + defer server.Close() + + path := writeTempFile(t, "data.csv", "a,b\n1,2\n") + stdout, _, err := executeCommand("api", "/v1/environments/{environment_id}/imports", + "--params", `{"environment_id":"456"}`, + "--file", "attachment=@"+path, + "--api-url", server.URL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(stdout, `"attachment"`) { + t.Errorf("part not named from the binding: %s", stdout) + } + // Asserting a resolved type here would assert whatever the local OS mime + // table returns, which differs per platform — the point is that we send none. + if !strings.Contains(stdout, `"content_type":""`) { + t.Errorf("part should declare no Content-Type: %s", stdout) + } +} + +func TestAPIFileUpload_DryRunReportsPartsNotContents(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("CIO_TOKEN", "sa_live_test123") + t.Setenv("CIO_ACCESS_TOKEN", "") + + path := writeTempFile(t, "secrets.md", "topsecret contents") + stdout, _, err := executeCommand("api", "/v1/environments/{environment_id}/knowledge_source_library/upload", + "--params", `{"environment_id":"456"}`, + "--file", "@"+path, + "--json", `{"name":"Notes"}`, + "--dry-run") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if strings.Contains(stdout, "topsecret") { + t.Errorf("dry run leaked file contents: %s", stdout) + } + for _, want := range []string{"multipart/form-data", "secrets.md", `"size"`, `"fields"`} { + if !strings.Contains(stdout, want) { + t.Errorf("dry run missing %q: %s", want, stdout) + } + } +} + +func TestAPIFileUpload_Errors(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("CIO_TOKEN", "sa_live_test123") + t.Setenv("CIO_ACCESS_TOKEN", "") + + good := writeTempFile(t, "notes.md", "hello") + empty := writeTempFile(t, "empty.md", "") + oversize := writeTempFile(t, "big.txt", strings.Repeat("a", client.MaxUploadBytes+1)) + + tests := []struct { + name string + args []string + contains string + }{ + {"missing file", []string{"--file", "@" + filepath.Join(tmpDir, "nope.md")}, "no such file"}, + {"empty file", []string{"--file", "@" + empty}, "file is empty"}, + {"bad field name", []string{"--file", "bad field=@" + good}, "letters, digits, underscores and hyphens"}, + {"missing path", []string{"--file", "field=@"}, "missing filename"}, + {"duplicate field", []string{"--file", "@" + good, "--file", "file=@" + good}, "more than once"}, + {"oversize file", []string{"--file", "@" + oversize}, "upload limit"}, + {"nested json field", []string{"--file", "@" + good, "--json", `{"meta":{"a":1}}`}, "nested values"}, + {"page-all", []string{"--file", "@" + good, "--page-all"}, "--page-all cannot be combined with --file"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + args := append([]string{"api", "/v1/environments/{environment_id}/uploads", + "--params", `{"environment_id":"456"}`, "--api-url", "http://127.0.0.1:1"}, tt.args...) + _, _, err := executeCommand(args...) + if err == nil { + t.Fatalf("expected an error") + } + if !strings.Contains(err.Error(), tt.contains) { + t.Errorf("error = %q, want it to mention %q", err, tt.contains) + } + }) + } +} + +// A dry run that reports "valid" for a request the real run rejects is worse +// than no check at all. +func TestAPIFileUpload_DryRunAppliesTheSameGuards(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("CIO_TOKEN", "sa_live_test123") + t.Setenv("CIO_ACCESS_TOKEN", "") + + good := writeTempFile(t, "notes.md", "hello") + oversize := writeTempFile(t, "big.txt", strings.Repeat("a", client.MaxUploadBytes+1)) + + tests := []struct { + name string + args []string + contains string + }{ + {"oversize", []string{"--file", "@" + oversize}, "upload limit"}, + {"page-all", []string{"--file", "@" + good, "--page-all"}, "--page-all cannot be combined with --file"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + args := append([]string{"api", "/v1/environments/{environment_id}/uploads", + "--params", `{"environment_id":"456"}`, "--dry-run"}, tt.args...) + _, _, err := executeCommand(args...) + if err == nil { + t.Fatalf("dry run reported valid for a request the real run rejects") + } + if !strings.Contains(err.Error(), tt.contains) { + t.Errorf("error = %q, want it to mention %q", err, tt.contains) + } + }) + } +} + +// [] is the conventional encoding for a repeated field, so it is the one name +// that may appear twice. +func TestAPIFileUpload_RepeatedBracketField(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) + t.Setenv("CIO_TOKEN", "sa_live_test123") + t.Setenv("CIO_ACCESS_TOKEN", "") + server := uploadServer(t) + defer server.Close() + + a := writeTempFile(t, "a.csv", "1\n") + b := writeTempFile(t, "b.csv", "2\n") + stdout, _, err := executeCommand("api", "/v1/environments/{environment_id}/uploads", + "--params", `{"environment_id":"456"}`, + "--file", "files[]=@"+a, + "--file", "files[]=@"+b, + "--api-url", server.URL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(stdout, "a.csv") || !strings.Contains(stdout, "b.csv") { + t.Errorf("both files should be sent: %s", stdout) + } +} + +func TestSplitFileBinding(t *testing.T) { + tests := []struct { + binding string + wantField string + wantPath string + wantErr bool + }{ + {"@notes.md", "file", "notes.md", false}, + {"notes.md", "file", "notes.md", false}, + {"doc=@notes.md", "doc", "notes.md", false}, + {"doc=notes.md", "doc", "notes.md", false}, + // A leading @ wins over the '=' split, so a filename can contain '='. + {"@a=b.md", "file", "a=b.md", false}, + {"files[]=@a.csv", "files[]", "a.csv", false}, + {"source-file=@a.csv", "source-file", "a.csv", false}, + {"", "", "", true}, + {"=@notes.md", "", "", true}, + {"doc=", "", "", true}, + } + for _, tt := range tests { + t.Run(tt.binding, func(t *testing.T) { + field, path, err := splitFileBinding(tt.binding) + if (err != nil) != tt.wantErr { + t.Fatalf("err = %v, wantErr = %v", err, tt.wantErr) + } + if err != nil { + return + } + if field != tt.wantField || path != tt.wantPath { + t.Errorf("got (%q, %q), want (%q, %q)", field, path, tt.wantField, tt.wantPath) + } + }) + } +} + +func TestFormFieldsFromJSON(t *testing.T) { + fields, err := formFieldsFromJSON(json.RawMessage(`{"s":"x","n":12,"f":1.5,"b":true,"z":null}`)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := map[string]string{"s": "x", "n": "12", "f": "1.5", "b": "true", "z": ""} + for k, v := range want { + if fields[k] != v { + t.Errorf("fields[%q] = %q, want %q", k, fields[k], v) + } + } + + if _, err := formFieldsFromJSON(json.RawMessage(`{"a":[1,2]}`)); err == nil { + t.Error("expected an error for a nested value") + } + + // Resource IDs exceed float64's exact range; a silently-rounded ID is worse + // than an error. + big, err := formFieldsFromJSON(json.RawMessage(`{"id":1234567890123456789}`)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if big["id"] != "1234567890123456789" { + t.Errorf("id = %q, want it sent verbatim", big["id"]) + } +} diff --git a/cmd/auth_test.go b/cmd/auth_test.go index a0ebdfd..b693750 100644 --- a/cmd/auth_test.go +++ b/cmd/auth_test.go @@ -16,6 +16,7 @@ import ( "github.com/customerio/cli/internal/client" "github.com/customerio/cli/internal/clipboard" + "github.com/spf13/pflag" ) func executeCommand(args ...string) (stdout, stderr string, err error) { @@ -35,6 +36,11 @@ func executeCommand(args ...string) (stdout, stderr string, err error) { _ = rootCmd.PersistentFlags().Set("token", "") _ = rootCmd.PersistentFlags().Set("scope", "") _ = rootCmd.PersistentFlags().Set("profile", "") + // Pagination flags leak between runs otherwise: one test asking for + // --page-all leaves every later command emitting NDJSON. + _ = rootCmd.PersistentFlags().Set("page-all", "false") + _ = rootCmd.PersistentFlags().Set("page", "0") + _ = rootCmd.PersistentFlags().Set("limit", "0") // Clear the package-level profile selection so it doesn't leak between runs. client.SetActiveProfile("") @@ -42,6 +48,11 @@ func executeCommand(args ...string) (stdout, stderr string, err error) { if f := apiCmd.Flags().Lookup("method"); f != nil { _ = apiCmd.Flags().Set("method", "") } + // StringArray flags append rather than replace, so a plain Set would stack + // values from earlier tests onto this run. + if f, ok := apiCmd.Flags().Lookup("file").Value.(pflag.SliceValue); ok { + _ = f.Replace(nil) + } // Reset auth login flags; Changed must clear too, or the // mutually-exclusive-flags check sees stale state from earlier tests. diff --git a/internal/client/client.go b/internal/client/client.go index 8271cbf..6470e6c 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -423,6 +423,15 @@ func fetchAccountInfo(ctx context.Context, httpClient *http.Client, baseURL, acc // Returns the raw JSON response body on success (2xx status). // Returns an *APIError for 4xx/5xx responses. func (c *Client) Do(ctx context.Context, method, path string, params map[string]string, body json.RawMessage) (json.RawMessage, error) { + var b *Body + if body != nil { + b = &Body{ContentType: "application/json", Bytes: body} + } + return c.DoWithBody(ctx, method, path, params, b) +} + +// DoWithBody carries payloads that are not JSON, such as a multipart file upload. +func (c *Client) DoWithBody(ctx context.Context, method, path string, params map[string]string, body *Body) (json.RawMessage, error) { // Block non-GET requests in read-only mode as a client-side safety net. if c.readOnly && method != http.MethodGet { return nil, fmt.Errorf("read-only mode: %s requests are not permitted (use without --read-only to allow writes)", method) @@ -498,11 +507,18 @@ func (c *Client) Do(ctx context.Context, method, path string, params map[string] return nil, lastErr } +// Bytes rather than a stream: a body can be sent more than once, since the retry +// loop and the 401 token refresh both re-issue the request. +type Body struct { + ContentType string + Bytes []byte +} + // doOnce executes a single HTTP request (no retry). -func (c *Client) doOnce(ctx context.Context, method, rawURL, accessToken string, body json.RawMessage) (json.RawMessage, error) { +func (c *Client) doOnce(ctx context.Context, method, rawURL, accessToken string, body *Body) (json.RawMessage, error) { var bodyReader io.Reader if body != nil { - bodyReader = bytes.NewReader(body) + bodyReader = bytes.NewReader(body.Bytes) } req, err := http.NewRequestWithContext(ctx, method, rawURL, bodyReader) @@ -511,7 +527,7 @@ func (c *Client) doOnce(ctx context.Context, method, rawURL, accessToken string, } if body != nil { - req.Header.Set("Content-Type", "application/json") + req.Header.Set("Content-Type", body.ContentType) } req.Header.Set("Accept", "application/json") diff --git a/internal/client/multipart.go b/internal/client/multipart.go new file mode 100644 index 0000000..6d3fa9a --- /dev/null +++ b/internal/client/multipart.go @@ -0,0 +1,65 @@ +package client + +import ( + "bytes" + "fmt" + "mime/multipart" + "net/textproto" + "sort" +) + +// The API rejects anything larger, so failing here saves uploading megabytes +// only to be turned away. +const MaxUploadBytes = 25 * 1024 * 1024 + +type FilePart struct { + Field string + Filename string + Content []byte +} + +// Fields are written in sorted order so the encoding is reproducible. +func NewMultipartBody(files []FilePart, fields map[string]string) (*Body, error) { + if len(files) == 0 { + return nil, fmt.Errorf("multipart body requires at least one file part") + } + + var buf bytes.Buffer + w := multipart.NewWriter(&buf) + + for _, f := range files { + if len(f.Content) > MaxUploadBytes { + return nil, fmt.Errorf("%s: file is %d bytes, over the %d byte upload limit", f.Filename, len(f.Content), MaxUploadBytes) + } + h := textproto.MIMEHeader{} + h.Set("Content-Disposition", fmt.Sprintf(`form-data; name=%q; filename=%q`, f.Field, f.Filename)) + // No Content-Type: mime.TypeByExtension reads the OS table, so the same + // .csv is text/csv on macOS and application/vnd.ms-excel on Windows — + // which upload endpoints reject. They resolve the type from the filename + // extension when the part declares none, so declaring nothing is both + // deterministic and what a generic transport should assert. + part, err := w.CreatePart(h) + if err != nil { + return nil, fmt.Errorf("encode file part %q: %w", f.Field, err) + } + if _, err := part.Write(f.Content); err != nil { + return nil, fmt.Errorf("encode file part %q: %w", f.Field, err) + } + } + + names := make([]string, 0, len(fields)) + for name := range fields { + names = append(names, name) + } + sort.Strings(names) + for _, name := range names { + if err := w.WriteField(name, fields[name]); err != nil { + return nil, fmt.Errorf("encode field %q: %w", name, err) + } + } + + if err := w.Close(); err != nil { + return nil, fmt.Errorf("close multipart body: %w", err) + } + return &Body{ContentType: w.FormDataContentType(), Bytes: buf.Bytes()}, nil +} diff --git a/internal/client/multipart_test.go b/internal/client/multipart_test.go new file mode 100644 index 0000000..6656b0b --- /dev/null +++ b/internal/client/multipart_test.go @@ -0,0 +1,86 @@ +package client + +import ( + "bytes" + "io" + "mime" + "mime/multipart" + "strings" + "testing" +) + +func TestNewMultipartBody(t *testing.T) { + body, err := NewMultipartBody( + []FilePart{{Field: "file", Filename: "notes.md", Content: []byte("hello")}}, + map[string]string{"name": "Notes", "description": "a file"}, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + mediaType, params, err := mime.ParseMediaType(body.ContentType) + if err != nil { + t.Fatalf("parse content type %q: %v", body.ContentType, err) + } + if mediaType != "multipart/form-data" { + t.Errorf("media type = %q", mediaType) + } + + files := map[string]string{} + fields := map[string]string{} + mr := multipart.NewReader(bytes.NewReader(body.Bytes), params["boundary"]) + for { + part, err := mr.NextPart() + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("read part: %v", err) + } + content, _ := io.ReadAll(part) + if part.FileName() != "" { + files[part.FormName()] = part.FileName() + } else { + fields[part.FormName()] = string(content) + } + } + if files["file"] != "notes.md" { + t.Errorf("files = %v", files) + } + if fields["name"] != "Notes" || fields["description"] != "a file" { + t.Errorf("fields = %v", fields) + } +} + +// The body is buffered, not streamed, so the retry loop and the 401 refresh can +// re-send it; nothing consumes it on the first attempt. +func TestNewMultipartBodyIsReusable(t *testing.T) { + body, err := NewMultipartBody([]FilePart{{Field: "file", Filename: "a.txt", Content: []byte("x")}}, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + first, _ := io.ReadAll(bytes.NewReader(body.Bytes)) + second, _ := io.ReadAll(bytes.NewReader(body.Bytes)) + if !bytes.Equal(first, second) || len(first) == 0 { + t.Errorf("body is not re-readable: %d vs %d bytes", len(first), len(second)) + } +} + +func TestNewMultipartBodyRejectsOversizeFile(t *testing.T) { + _, err := NewMultipartBody( + []FilePart{{Field: "file", Filename: "big.txt", Content: bytes.Repeat([]byte("a"), MaxUploadBytes+1)}}, + nil, + ) + if err == nil { + t.Fatal("expected an error") + } + if !strings.Contains(err.Error(), "upload limit") { + t.Errorf("error = %q", err) + } +} + +func TestNewMultipartBodyRequiresAFile(t *testing.T) { + if _, err := NewMultipartBody(nil, map[string]string{"name": "x"}); err == nil { + t.Error("expected an error") + } +}