From ba7845f3acfacdb36464ce0c1bd1de0cf9c862f4 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 10 Jun 2026 10:36:18 -0400 Subject: [PATCH 1/5] Fix Go GET content type header --- go/pkg/basecamp/client.go | 13 ++++++++--- go/pkg/basecamp/client_test.go | 41 ++++++++++++++++++++++++++++++++++ go/pkg/basecamp/search_test.go | 23 +++++++++++++++++++ 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/go/pkg/basecamp/client.go b/go/pkg/basecamp/client.go index dc7b3d4b..f6ed492d 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") == "" { + // Only set Content-Type when a request body is present. GET requests + // carry parameters in the URL, and Content-Type describes a request body. + 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 body != nil { + 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..30d55f25 100644 --- a/go/pkg/basecamp/client_test.go +++ b/go/pkg/basecamp/client_test.go @@ -125,6 +125,47 @@ 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 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 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_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..ffae4077 100644 --- a/go/pkg/basecamp/search_test.go +++ b/go/pkg/basecamp/search_test.go @@ -286,3 +286,26 @@ 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 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(200) + w.Write(fixture) + }) + + _, err := svc.Search(context.Background(), "leto", nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} From 8f3a0966ff4964421a088f4cf13e7d746f5f2382 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 10 Jun 2026 10:53:16 -0400 Subject: [PATCH 2/5] Address GET header review feedback --- go/pkg/basecamp/client.go | 2 +- go/pkg/basecamp/client_test.go | 6 ++++++ go/pkg/basecamp/search_test.go | 5 ++++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/go/pkg/basecamp/client.go b/go/pkg/basecamp/client.go index f6ed492d..658e7fd8 100644 --- a/go/pkg/basecamp/client.go +++ b/go/pkg/basecamp/client.go @@ -688,7 +688,7 @@ func (c *Client) singleRequest(ctx context.Context, method, url string, body any return nil, err } req.Header.Set("User-Agent", c.userAgent) - if body != nil { + if requestHasBody(req) { req.Header.Set("Content-Type", "application/json") } req.Header.Set("Accept", "application/json") diff --git a/go/pkg/basecamp/client_test.go b/go/pkg/basecamp/client_test.go index 30d55f25..299dfcf1 100644 --- a/go/pkg/basecamp/client_test.go +++ b/go/pkg/basecamp/client_test.go @@ -127,6 +127,9 @@ 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.Fatalf("expected GET, got %s", r.Method) + } if got := r.Header.Get("Content-Type"); got != "" { t.Errorf("expected no Content-Type for bodyless GET, got %q", got) } @@ -149,6 +152,9 @@ func TestSingleRequest_GETDoesNotSetContentType(t *testing.T) { func TestSingleRequest_POSTSetsContentTypeForJSONBody(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Fatalf("expected POST, got %s", r.Method) + } if got := r.Header.Get("Content-Type"); got != "application/json" { t.Errorf("expected Content-Type application/json, got %q", got) } diff --git a/go/pkg/basecamp/search_test.go b/go/pkg/basecamp/search_test.go index ffae4077..81ede66f 100644 --- a/go/pkg/basecamp/search_test.go +++ b/go/pkg/basecamp/search_test.go @@ -290,6 +290,9 @@ func TestSearchService_Search_NoSort(t *testing.T) { 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.Fatalf("expected GET, got %s", r.Method) + } if got := r.Header.Get("Content-Type"); got != "" { t.Errorf("expected no Content-Type for bodyless GET, got %q", got) } @@ -300,7 +303,7 @@ func TestSearchService_Search_DoesNotSetContentTypeOnGet(t *testing.T) { t.Errorf("expected q=leto, got %q", got) } w.Header().Set("Content-Type", "application/json") - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) w.Write(fixture) }) From 0d03f1d0a43bd7dc74140b20248ce9f74e1b5628 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 10 Jun 2026 11:11:58 -0400 Subject: [PATCH 3/5] Address PR review feedback --- go/pkg/basecamp/client.go | 2 +- go/pkg/basecamp/client_test.go | 47 ++++++++++++++++++++++++++++++++-- go/pkg/basecamp/search_test.go | 4 ++- 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/go/pkg/basecamp/client.go b/go/pkg/basecamp/client.go index 658e7fd8..99aab4c8 100644 --- a/go/pkg/basecamp/client.go +++ b/go/pkg/basecamp/client.go @@ -688,7 +688,7 @@ func (c *Client) singleRequest(ctx context.Context, method, url string, body any return nil, err } req.Header.Set("User-Agent", c.userAgent) - if requestHasBody(req) { + if requestHasBody(req) && req.Header.Get("Content-Type") == "" { req.Header.Set("Content-Type", "application/json") } req.Header.Set("Accept", "application/json") diff --git a/go/pkg/basecamp/client_test.go b/go/pkg/basecamp/client_test.go index 299dfcf1..20a706c8 100644 --- a/go/pkg/basecamp/client_test.go +++ b/go/pkg/basecamp/client_test.go @@ -9,6 +9,15 @@ import ( "testing" ) +type contentTypeAuthStrategy struct { + contentType string +} + +func (s contentTypeAuthStrategy) Authenticate(_ context.Context, req *http.Request) error { + 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) @@ -128,7 +137,9 @@ 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.Fatalf("expected GET, got %s", r.Method) + 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) @@ -153,7 +164,9 @@ func TestSingleRequest_GETDoesNotSetContentType(t *testing.T) { func TestSingleRequest_POSTSetsContentTypeForJSONBody(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodPost { - t.Fatalf("expected POST, got %s", r.Method) + 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) @@ -172,6 +185,36 @@ func TestSingleRequest_POSTSetsContentTypeForJSONBody(t *testing.T) { } } +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 81ede66f..7441211d 100644 --- a/go/pkg/basecamp/search_test.go +++ b/go/pkg/basecamp/search_test.go @@ -291,7 +291,9 @@ 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.Fatalf("expected GET, got %s", r.Method) + 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) From 1db4805fa57b6c11994a7de4f5c41e1920e74a7f Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 10 Jun 2026 14:40:09 -0400 Subject: [PATCH 4/5] Address follow-up review comments --- go/pkg/basecamp/client.go | 4 ++-- go/pkg/basecamp/search_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go/pkg/basecamp/client.go b/go/pkg/basecamp/client.go index 99aab4c8..620cf39a 100644 --- a/go/pkg/basecamp/client.go +++ b/go/pkg/basecamp/client.go @@ -365,8 +365,8 @@ func (c *Client) initGeneratedClient() { return err } req.Header.Set("User-Agent", c.userAgent) - // Only set Content-Type when a request body is present. GET requests - // carry parameters in the URL, and Content-Type describes a request body. + // 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") } diff --git a/go/pkg/basecamp/search_test.go b/go/pkg/basecamp/search_test.go index 7441211d..639d2e31 100644 --- a/go/pkg/basecamp/search_test.go +++ b/go/pkg/basecamp/search_test.go @@ -306,7 +306,7 @@ func TestSearchService_Search_DoesNotSetContentTypeOnGet(t *testing.T) { } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - w.Write(fixture) + _, _ = w.Write(fixture) }) _, err := svc.Search(context.Background(), "leto", nil) From 767a8ff0fd0b38e6c484ece8c2a09e3fbf2a5cd2 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 10 Jun 2026 14:49:35 -0400 Subject: [PATCH 5/5] Strengthen Content-Type preservation test --- go/pkg/basecamp/client_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/go/pkg/basecamp/client_test.go b/go/pkg/basecamp/client_test.go index 20a706c8..37a4c5fd 100644 --- a/go/pkg/basecamp/client_test.go +++ b/go/pkg/basecamp/client_test.go @@ -3,6 +3,7 @@ package basecamp import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "strings" @@ -14,6 +15,9 @@ type contentTypeAuthStrategy struct { } 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 }