[
https://issues.apache.org/jira/browse/CAMEL-24001?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18095617#comment-18095617
]
Claus Ibsen commented on CAMEL-24001:
-------------------------------------
Fixed via https://github.com/apache/camel/pull/24597 (merged to main)
> RestBindingAdvice.ensureHeaderContentType() sets Content-Type on body-less
> (e.g. 204) responses, and falls back to a raw multi-value "produces" list
> instead of a single media type
> -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-24001
> URL: https://issues.apache.org/jira/browse/CAMEL-24001
> Project: Camel
> Issue Type: Bug
> Components: camel-core, camel-rest-openapi
> Affects Versions: 4.18.2
> Reporter: Martin Lopatář
> Assignee: Claus Ibsen
> Priority: Major
> Fix For: 4.22.0
>
>
> h3. Summary
> RestBindingAdvice.after() -> marshal() calls ensureHeaderContentType()
> unconditionally, before checking whether the outgoing message actually has
> a body. This produces a nonsensical Content-Type header on responses that
> have no body at all (e.g. a 204 No Content response from a contract-first
> `rest: openApi:` operation), unless the route explicitly works around it
> by pre-setting Content-Type to a non-null value.
> h3. Root cause
> In RestBindingAdvice.marshal() (camel-support):
> {code:java}
> // need to prepare exchange first
> ExchangeHelper.prepareOutToIn(exchange);
> // ensure there is a content type header (even if binding is off)
> ensureHeaderContentType(produces, isXml, isJson, exchange); // <-- runs
> unconditionally
> if (bindingMode == null || "off".equals(bindingMode)) {
> return;
> }
> if (jsonMarshal == null && xmlMarshal == null) {
> return;
> }
> // is the body empty
> if (exchange.getMessage().getBody() == null) {
> return; // <--
> empty-body check, but too late
> }
> {code}
> The empty-body check exists in this same method, but is applied only to
> gate the marshalling step, not the Content-Type defaulting step that
> precedes it. Content-Type is meant to describe a response's body; with no
> body there is nothing to describe, so defaulting it here is not meaningful.
> Additionally, ensureHeaderContentType()'s "favor given content type" branch:
> {code:java}
> private void ensureHeaderContentType(String contentType, boolean isXml,
> boolean isJson, Exchange exchange) {
> // favor given content type
> if (contentType != null) {
> String type = ExchangeHelper.getContentType(exchange);
> if (type == null) {
> exchange.getIn().setHeader(Exchange.CONTENT_TYPE, contentType);
> }
> }
> ...
> {code}
> sets Content-Type verbatim to the `contentType` parameter, which for a
> contract-first OpenAPI operation is the *operation-level* `produces` string
> computed by OpenApiUtils.getProduces() – a comma-joined union of every
> media type across *all* declared responses for that operation (e.g. every
> content type from the 200 response plus every content type from a shared
> default/error response schema), not the content type of the specific
> response actually being returned. A Content-Type header is expected to be
> a single media type describing the current body, not an enumeration of
> everything the operation could ever produce across all status codes. This
> means even on a populated response, if no step in the route already set
> Content-Type explicitly, the header ends up with a value like
> `{*}/{*},application/json,text/plain` rather than one concrete media type.
> h3. Reproduction
> 1. Define an OpenAPI operation, contract-first, with a `rest: openApi:
> specification:` binding, where one response code has no body (e.g.
> `204: description: no content`) and another response (e.g. a shared
> default/error response) declares one or more `content` media types.
> 2. Implement the route so that, for the no-content branch, no Content-Type
> header is set at all (rely on the framework rather than the route to
> decide).
> 3. Invoke the operation so it takes the no-content branch.
> 4. Observe the response's Content-Type header: it is the full joined
> produces list for the operation (e.g. `{*}/{*},application/json,text/plain`),
> not absent, and not a single meaningful value.
> Only workaround found: have the route explicitly `setHeader(Content-Type,
> "")` (an empty string, not removeHeader) immediately before the response is
> returned, so that ExchangeHelper.getContentType(exchange) returns non-null
> and short-circuits both branches of ensureHeaderContentType(). This works
> but requires every route author to know this internal detail and apply the
> workaround manually per no-content branch.
> h3. Expected behavior
> - No Content-Type header should be defaulted/added on a response with no
> body.
> - When a Content-Type *is* defaulted (body present, none set explicitly),
> it should be a single concrete media type relevant to the response
> actually produced, not a raw join of every media type declared across
> all of the operation's responses.
> h3. Suggested fix
> - Move (or duplicate) the "is body empty" check in marshal() to before the
> call to ensureHeaderContentType(), so no header is added when there's no
> body.
> - Reconsider what value ensureHeaderContentType()'s "favor given content
> type" branch should default to – e.g. pick the content type declared for
> the actual outgoing response/status code, rather than the operation-wide
> produces union.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
