diff --git a/go/pkg/basecamp/client.go b/go/pkg/basecamp/client.go index dc7b3d4b..620cf39a 100644 --- a/go/pkg/basecamp/client.go +++ b/go/pkg/basecamp/client.go @@ -365,8 +365,9 @@ func (c *Client) initGeneratedClient() { return err } req.Header.Set("User-Agent", c.userAgent) - // Only set Content-Type if not already set (preserves binary upload content types) - if req.Header.Get("Content-Type") == "" { + // Content-Type describes a request body, so set the JSON default only + // when a body is present and the caller has not already set one. + if requestHasBody(req) && req.Header.Get("Content-Type") == "" { req.Header.Set("Content-Type", "application/json") } req.Header.Set("Accept", "application/json") @@ -382,6 +383,10 @@ func (c *Client) initGeneratedClient() { }) } +func requestHasBody(req *http.Request) bool { + return req != nil && req.Body != nil && req.Body != http.NoBody +} + // discardHandler is a slog.Handler that discards all log records. type discardHandler struct{} @@ -683,7 +688,9 @@ func (c *Client) singleRequest(ctx context.Context, method, url string, body any return nil, err } req.Header.Set("User-Agent", c.userAgent) - req.Header.Set("Content-Type", "application/json") + if requestHasBody(req) && req.Header.Get("Content-Type") == "" { + req.Header.Set("Content-Type", "application/json") + } req.Header.Set("Accept", "application/json") // Add ETag for cached GET requests. Derive cache key from the Authorization diff --git a/go/pkg/basecamp/client_test.go b/go/pkg/basecamp/client_test.go index 155bbc79..37a4c5fd 100644 --- a/go/pkg/basecamp/client_test.go +++ b/go/pkg/basecamp/client_test.go @@ -3,12 +3,25 @@ package basecamp import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "strings" "testing" ) +type contentTypeAuthStrategy struct { + contentType string +} + +func (s contentTypeAuthStrategy) Authenticate(_ context.Context, req *http.Request) error { + if req.Header.Get("Content-Type") != "" { + return errors.New("expected Content-Type to be empty before auth strategy sets it") + } + req.Header.Set("Content-Type", s.contentType) + return nil +} + func TestSingleRequest_204ReturnsValidJSON(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) @@ -125,6 +138,87 @@ func TestSingleRequest_200WithBody(t *testing.T) { } } +func TestSingleRequest_GETDoesNotSetContentType(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + t.Errorf("expected GET, got %s", r.Method) + http.Error(w, "wrong method", http.StatusBadRequest) + return + } + if got := r.Header.Get("Content-Type"); got != "" { + t.Errorf("expected no Content-Type for bodyless GET, got %q", got) + } + if got := r.Header.Get("Accept"); got != "application/json" { + t.Errorf("expected Accept application/json, got %q", got) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"ok":true}`)) + })) + defer server.Close() + + cfg := &Config{BaseURL: server.URL, CacheEnabled: false} + client := NewClient(cfg, &StaticTokenProvider{Token: "test-token"}) + + if _, err := client.Get(context.Background(), "/test.json"); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestSingleRequest_POSTSetsContentTypeForJSONBody(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Errorf("expected POST, got %s", r.Method) + http.Error(w, "wrong method", http.StatusBadRequest) + return + } + if got := r.Header.Get("Content-Type"); got != "application/json" { + t.Errorf("expected Content-Type application/json, got %q", got) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"ok":true}`)) + })) + defer server.Close() + + cfg := &Config{BaseURL: server.URL, CacheEnabled: false} + client := NewClient(cfg, &StaticTokenProvider{Token: "test-token"}) + + if _, err := client.Post(context.Background(), "/test.json", map[string]any{"ok": true}); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestSingleRequest_POSTPreservesExistingContentType(t *testing.T) { + const customContentType = "application/merge-patch+json" + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Errorf("expected POST, got %s", r.Method) + http.Error(w, "wrong method", http.StatusBadRequest) + return + } + if got := r.Header.Get("Content-Type"); got != customContentType { + t.Errorf("expected Content-Type %q, got %q", customContentType, got) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"ok":true}`)) + })) + defer server.Close() + + cfg := &Config{BaseURL: server.URL, CacheEnabled: false} + client := NewClient( + cfg, + &StaticTokenProvider{Token: "test-token"}, + WithAuthStrategy(contentTypeAuthStrategy{contentType: customContentType}), + ) + + if _, err := client.Post(context.Background(), "/test.json", map[string]any{"ok": true}); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + func TestSingleRequest_204Delete(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodDelete { diff --git a/go/pkg/basecamp/search_test.go b/go/pkg/basecamp/search_test.go index 6e4a9208..639d2e31 100644 --- a/go/pkg/basecamp/search_test.go +++ b/go/pkg/basecamp/search_test.go @@ -286,3 +286,31 @@ func TestSearchService_Search_NoSort(t *testing.T) { t.Fatalf("unexpected error: %v", err) } } + +func TestSearchService_Search_DoesNotSetContentTypeOnGet(t *testing.T) { + fixture := loadSearchFixture(t, "results.json") + svc := testSearchServer(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + t.Errorf("expected GET, got %s", r.Method) + http.Error(w, "wrong method", http.StatusBadRequest) + return + } + if got := r.Header.Get("Content-Type"); got != "" { + t.Errorf("expected no Content-Type for bodyless GET, got %q", got) + } + if got := r.Header.Get("Accept"); got != "application/json" { + t.Errorf("expected Accept application/json, got %q", got) + } + if got := r.URL.Query().Get("q"); got != "leto" { + t.Errorf("expected q=leto, got %q", got) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write(fixture) + }) + + _, err := svc.Search(context.Background(), "leto", nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +}