Stop the payload content type from clobbering a modeled Accept header

CHANGELOG.md

@@ -239,6 +239,19 @@ policy in [docs/versioning.md](docs/versioning.md).
 
 ### Fixed
 
+- **A modeled `@httpHeader("Accept")` input member reaches the wire.** An
+  operation with a response `@httpPayload` emits that payload's content type
+  as the request's Accept header — but it did so with an unconditional `Set`,
+  after the `@httpHeader` bindings had already written the caller's own
+  modeled Accept, so the modeled member was overwritten on every call and a
+  consumer had to put it back with an interceptor's `ModifyBeforeTransmit`.
+  The payload content type is now a default, applied only when nothing has
+  already set the header — the same guard the generated `Send` helper has
+  always applied to its document-response default, and the one the request
+  payload's `Content-Type` already had. Goldens move for every operation with
+  a response payload; behavior changes only for a model that binds Accept (or
+  an `@httpPrefixHeaders` map carrying it).
+
 - **Health probes are distinguishable from dispatch failures in observability
   hooks.** `HealthEndpoint` built its response without stamping
   `HttpResponse::operation`, so every probe reached `Observe` (and so any

codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/HttpJsonClientGenerator.java

@@ -173,8 +173,16 @@ void writeOperationBody(
     }
 
     if (responsePayload != null) {
+      // The payload's content type is this operation's *default* Accept, not
+      // an override: a modeled @httpHeader("Accept") member (or an
+      // @httpPrefixHeaders map carrying one) has already been written above,
+      // and an unconditional Set would discard it on every call, leaving the
+      // member dead and the caller reaching for an interceptor to put back
+      // what the client just took away. Same guard the generated Send helper
+      // applies to its own document-response default, one level down.
       w.write(
-          "request.headers.Set(\"accept\", $S);",
+          "if (!request.headers.Get(\"accept\").has_value()) "
+              + "request.headers.Set(\"accept\", $S);",
           HttpBindingCodeGen.payloadContentType(context, operation, false));
     }
     ProtocolSupport.writeRequestCompression(w, operation);

codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/GeneratedCodeShapeTest.java

@@ -395,6 +395,63 @@ void typedErrorListingNameCollisionFailsWithContext() {
         model, "test.shape#Svc", "test::shape", "cpp-codegen", "PingErrors");
   }
 
+  @Test
+  void aModeledAcceptHeaderSurvivesThePayloadContentType() {
+    // Found in use against a generated client. An operation with a response
+    // @httpPayload emits its payload's content type as the Accept header — but
+    // it did so with an unconditional Set, after the @httpHeader bindings had
+    // already written the caller's own modeled Accept. The modeled member was
+    // therefore dead on arrival: set, then overwritten, every call. The
+    // workaround a consumer is left with is an interceptor re-setting the
+    // header in ModifyBeforeTransmit, which is the client's own job.
+    //
+    // The generated Send helper has always guarded its document-response
+    // default the same way ("Operations with a non-document response payload
+    // set their own accept"); this is that rule applied one level down.
+    String model =
+        """
+        $version: "2.0"
+        namespace test.shape
+        use alloy#simpleRestJson
+
+        @simpleRestJson
+        service Svc { version: "1", operations: [Fetch] }
+
+        @readonly
+        @http(method: "GET", uri: "/fetch/{id}")
+        operation Fetch {
+            input := {
+                @required
+                @httpLabel
+                id: String
+
+                @httpHeader("Accept")
+                accept: String
+            }
+            output := {
+                @httpPayload
+                content: Blob
+            }
+        }
+        """;
+    var manifest = PluginTestHarness.generate(model, "test.shape#Svc", "test::shape");
+    String source = manifest.expectFileString("/src/client.cc");
+
+    assertTrue(
+        source.contains(
+            "if (!request.headers.Get(\"accept\").has_value()) "
+                + "request.headers.Set(\"accept\", \"application/octet-stream\");"),
+        source);
+    // The unguarded form is what clobbered it, so its absence is the fix —
+    // checked per line, since the guarded statement ends with those same
+    // characters and a substring search would always find them.
+    for (String line : source.split("\n", -1)) {
+      assertFalse(
+          line.strip().equals("request.headers.Set(\"accept\", \"application/octet-stream\");"),
+          source);
+    }
+  }
+
   @Test
   void streamingBlobsStayPlainBufferedBlobs() {
     // A @streaming blob in the *request* payload is still an ordinary, fully

examples/bazel-consumer/model/redirector.smithy

@@ -152,6 +152,13 @@ operation Download {
         @required
         @httpLabel
         slug: String
+
+        /// A caller that wants a narrower media type than the payload's own
+        /// says so here. Modeled deliberately: the generated client emits the
+        /// payload's content type as a *default* Accept, and this member is
+        /// what proves it stays a default rather than an override.
+        @httpHeader("Accept")
+        accept: String
     }
 
     output := {

examples/bazel-consumer/response_sink_acceptance_test.cc

@@ -206,6 +206,65 @@ TEST_F(ResponseSinkAcceptanceTest, AModeledErrorIsLeftWhereTheClientLooksForIt)
   EXPECT_NE(response->body.find("no slug: missing"), std::string::npos) << response->body;
 }
 
+// Records the request a generated client produced and answers with a canned
+// payload. The point is the request, not the response: what a caller cannot
+// see from the outside is which headers the client decided to send.
+class RecordingClient final : public opal::http::HttpClient {
+ public:
+  explicit RecordingClient(std::string payload) : payload_(std::move(payload)) {}
+
+  opal::Outcome<opal::http::HttpResponse> Send(const opal::http::HttpRequest& request) override {
+    seen = request;
+    opal::http::HttpResponse response;
+    response.status = 200;
+    response.headers.Set("content-type", "application/octet-stream");
+    response.headers.Set("etag", "\"big\"");
+    response.body = payload_;
+    return response;
+  }
+
+  opal::http::HttpRequest seen;
+
+ private:
+  std::string payload_;
+};
+
+TEST(ModeledAcceptHeaderTest, ACallersModeledAcceptIsNotReplacedByThePayloadContentType) {
+  // The bug this pins: the client emitted the response payload's content type
+  // as Accept with an unconditional Set, after writing the modeled @httpHeader
+  // member — so the member never reached the wire and a consumer had to put it
+  // back with an interceptor.
+  auto transport = std::make_shared<RecordingClient>("payload");
+  opal::ClientConfig config;
+  config.endpoint = "http://127.0.0.1:1";  // unused: the transport is injected
+  config.http_client = transport;
+  auto client = RedirectorClient::Create(config);
+  ASSERT_TRUE(client.ok()) << client.error().message();
+
+  const auto downloaded =
+      client->Download(DownloadInput{.slug = "big", .accept = "application/x-tar"});
+  ASSERT_TRUE(downloaded.ok()) << downloaded.error().message();
+  EXPECT_EQ(transport->seen.headers.Get("accept").value_or(""), "application/x-tar");
+  // Exactly one: Set replaces, but a future Add would send both and let the
+  // server pick, which is not what the model asked for either.
+  EXPECT_EQ(transport->seen.headers.GetAll("accept").size(), 1u);
+}
+
+TEST(ModeledAcceptHeaderTest, AnUnsetModeledAcceptLeavesThePayloadContentTypeInPlace) {
+  // The other half: the payload's content type is still the default, so
+  // guarding it did not simply delete the behavior.
+  auto transport = std::make_shared<RecordingClient>("payload");
+  opal::ClientConfig config;
+  config.endpoint = "http://127.0.0.1:1";
+  config.http_client = transport;
+  auto client = RedirectorClient::Create(config);
+  ASSERT_TRUE(client.ok()) << client.error().message();
+
+  const auto downloaded = client->Download(DownloadInput{.slug = "big"});
+  ASSERT_TRUE(downloaded.ok()) << downloaded.error().message();
+  EXPECT_EQ(transport->seen.headers.Get("accept").value_or(""), "application/octet-stream");
+}
+
 // The generated level (slice 2). The client is built from an endpoint alone —
 // no transport injected, no sink assembled by hand — which is the whole point:
 // a @streaming blob payload is streamed by the operation the generator wrote.

examples/roundtrip/rest/generated/src/client.cc

@@ -308,7 +308,7 @@ opal::Outcome<UploadAttachmentOutput> RoundTripRestClient::UploadAttachment(cons
     request.body = (*input.data).ToString();
     if (!request.headers.Get("content-type").has_value()) request.headers.Set("content-type", "application/octet-stream");
   }
-  request.headers.Set("accept", "application/json");
+  if (!request.headers.Get("accept").has_value()) request.headers.Set("accept", "application/json");
   auto response = Send(std::move(request));
   if (!response) return std::move(response).error();
   if (response->status != 200) return helpers::ParseUploadAttachmentError(*response);

protocol-tests/simplerestjson/generated/src/client.cc

@@ -358,7 +358,7 @@ opal::Outcome<AddMenuItemOutput> PizzaAdminServiceClient::AddMenuItem(const AddM
   request.target = std::move(target);
   request.body = opal::json::Encode(SerializeMenuItem(input.menuItem));
   if (!request.headers.Get("content-type").has_value()) request.headers.Set("content-type", "application/json");
-  request.headers.Set("accept", "application/json");
+  if (!request.headers.Get("accept").has_value()) request.headers.Set("accept", "application/json");
   auto response = Send(std::move(request));
   if (!response) return std::move(response).error();
   if (response->status != 201) return helpers::ParseAddMenuItemError(*response);
@@ -436,7 +436,7 @@ opal::Outcome<GetMenuOutput> PizzaAdminServiceClient::GetMenu(const GetMenuInput
   opal::http::HttpRequest request;
   request.method = "GET";
   request.target = std::move(target);
-  request.headers.Set("accept", "application/json");
+  if (!request.headers.Get("accept").has_value()) request.headers.Set("accept", "application/json");
   auto response = Send(std::move(request));
   if (!response) return std::move(response).error();
   if (response->status != 200) return helpers::ParseGetMenuError(*response);
@@ -518,7 +518,7 @@ opal::Outcome<HttpPayloadRequiredWithDefaultOutput> PizzaAdminServiceClient::Htt
   request.target = std::move(target);
   request.body = opal::json::Encode(opal::Document(input.body));
   if (!request.headers.Get("content-type").has_value()) request.headers.Set("content-type", "application/json");
-  request.headers.Set("accept", "application/json");
+  if (!request.headers.Get("accept").has_value()) request.headers.Set("accept", "application/json");
   auto response = Send(std::move(request));
   if (!response) return std::move(response).error();
   if (response->status != 200) return helpers::ParseHttpPayloadRequiredWithDefaultError(*response);
@@ -542,7 +542,7 @@ opal::Outcome<HttpPayloadWithDefaultOutput> PizzaAdminServiceClient::HttpPayload
     request.body = opal::json::Encode(opal::Document((*input.body)));
     if (!request.headers.Get("content-type").has_value()) request.headers.Set("content-type", "application/json");
   }
-  request.headers.Set("accept", "application/json");
+  if (!request.headers.Get("accept").has_value()) request.headers.Set("accept", "application/json");
   auto response = Send(std::move(request));
   if (!response) return std::move(response).error();
   if (response->status != 200) return helpers::ParseHttpPayloadWithDefaultError(*response);
@@ -564,7 +564,7 @@ opal::Outcome<OpenUnionsOutput> PizzaAdminServiceClient::OpenUnions(const OpenUn
   request.target = std::move(target);
   request.body = opal::json::Encode(SerializeOpenUnionsPayload(input.data));
   if (!request.headers.Get("content-type").has_value()) request.headers.Set("content-type", "application/json");
-  request.headers.Set("accept", "application/json");
+  if (!request.headers.Get("accept").has_value()) request.headers.Set("accept", "application/json");
   auto response = Send(std::move(request));
   if (!response) return std::move(response).error();
   if (response->status != 200) return helpers::ParseOpenUnionsError(*response);
@@ -650,7 +650,7 @@ opal::Outcome<VersionOutput> PizzaAdminServiceClient::Version(const VersionInput
   opal::http::HttpRequest request;
   request.method = "GET";
   request.target = std::move(target);
-  request.headers.Set("accept", "application/json");
+  if (!request.headers.Get("accept").has_value()) request.headers.Set("accept", "application/json");
   auto response = Send(std::move(request));
   if (!response) return std::move(response).error();
   if (response->status != 200) return helpers::ParseVersionError(*response);