[
https://issues.apache.org/jira/browse/CXF-9235?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18102228#comment-18102228
]
Freeman Yue Fang edited comment on CXF-9235 at 8/5/26 9:55 PM:
---------------------------------------------------------------
Hi Neena,
Thanks for the detailed report and for tracking down the root cause — your
diagnosis is correct IMO. I traced it through the code and confirmed:
InvocationBuilderImpl.property(name, value)
(rt/rs/client/.../spec/InvocationBuilderImpl.java) nests the property under a
private key, "jaxrs.filter.properties", instead of writing it flat into the
request context:
Map<String, Object> filterProps = CastUtils.cast((Map<?, ?>)
contextProps.get(PROPERTY_KEY));
...
filterProps.put(name, value);
Compare that to Client.property() / WebTarget.property(), which go through
ConfigurableImpl and end up flattened directly into
ClientConfiguration.getRequestContext() (ClientImpl.java, around line 285).
That flat write is what lets Message.getContextualProperty() — used by both
Headers.setProtocolHeadersInConnection() and HttpClientHTTPConduit — see the
property at all, since it does a flat merge and never unwraps a value that's
itself a Map.
So I'd suggest fixing this at the source rather than the transport side: your
workaround (teaching the transport to also check jaxrs.filter.properties when a
contextual lookup misses) does resolve your case, but it only fixes this one
property. The nesting bug in InvocationBuilderImpl actually swallows any
property set via Invocation.Builder.property(), and your workaround also pulls
a JAX-RS-client-internal key (jaxrs.filter.properties) into the generic HTTP
transport layer, which we'd rather avoid introducing upstream.
The fix I'd propose instead: make InvocationBuilderImpl.property() write
directly into contextProps (flat), matching how Client/WebTarget already
behave. That's a smaller change, fixes the bug for every property (not just
this one), and needs no changes outside rt/rs/client.
Something like this in my mind for now
{code}
---
a/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/spec/InvocationBuilderImpl.java
+++
b/rt/rs/client/src/main/java/org/apache/cxf/jaxrs/client/spec/InvocationBuilderImpl.java
@@ -288,8 +288,13 @@ public class InvocationBuilderImpl implements
Invocation.Builder {
}
if (value == null) {
filterProps.remove(name);
+ // CXF-9235: properties set here also need to be visible as
top-level
+ // context properties, otherwise code that reads them via
+ // Message.getContextualProperty() (e.g. the HTTP transport) never
sees them.
+ contextProps.remove(name);
} else {
filterProps.put(name, value);
+ contextProps.put(name, value);
}
return this;
{code}
Would you be willing to rework your patch along those lines and open the PR?
Also we should add a regression test exercising
Invocation.Builder.property("set.content.type.for.empty.request", ...)
directly, since that path currently has no coverage.
Thanks again for digging into this!
Freeman
was (Author: ffang):
Hi Neena,
Thanks for the detailed report and for tracking down the root cause — your
diagnosis is correct IMO. I traced it through the code and confirmed:
InvocationBuilderImpl.property(name, value)
(rt/rs/client/.../spec/InvocationBuilderImpl.java) nests the property under a
private key, "jaxrs.filter.properties", instead of writing it flat into the
request context:
Map<String, Object> filterProps = CastUtils.cast((Map<?, ?>)
contextProps.get(PROPERTY_KEY));
...
filterProps.put(name, value);
Compare that to Client.property() / WebTarget.property(), which go through
ConfigurableImpl and end up flattened directly into
ClientConfiguration.getRequestContext() (ClientImpl.java, around line 285).
That flat write is what lets Message.getContextualProperty() — used by both
Headers.setProtocolHeadersInConnection() and HttpClientHTTPConduit — see the
property at all, since it does a flat merge and never unwraps a value that's
itself a Map.
So I'd suggest fixing this at the source rather than the transport side: your
workaround (teaching the transport to also check jaxrs.filter.properties when a
contextual lookup misses) does resolve your case, but it only fixes this one
property. The nesting bug in InvocationBuilderImpl actually swallows any
property set via Invocation.Builder.property(), and your workaround also pulls
a JAX-RS-client-internal key (jaxrs.filter.properties) into the generic HTTP
transport layer, which we'd rather avoid introducing upstream.
The fix I'd propose instead: make InvocationBuilderImpl.property() write
directly into contextProps (flat), matching how Client/WebTarget already
behave. That's a smaller change, fixes the bug for every property (not just
this one), and needs no changes outside rt/rs/client.
Would you be willing to rework your patch along those lines and open the PR?
Also we should add a regression test exercising
Invocation.Builder.property("set.content.type.for.empty.request", ...)
directly, since that path currently has no coverage.
Thanks again for digging into this!
Freeman
> Invocation.Builder.property() settings ignored by HTTP transport for
> set.content.type.for.empty.request
> -------------------------------------------------------------------------------------------------------
>
> Key: CXF-9235
> URL: https://issues.apache.org/jira/browse/CXF-9235
> Project: CXF
> Issue Type: Bug
> Reporter: Neena Jacob
> Priority: Minor
>
> {{InvocationBuilderImpl.property()}} stores request properties in a nested
> map under the key {{jaxrs.filter.properties}} inside the WebClient request
> context. When a client calls:
>
> {{builder.property("set.content.type.for.empty.request", "FALSE")}}
>
> the value is stored inside that nested map, not at the top level of the
> request context.
> However, {{Headers.setProtocolHeadersInConnection()}} retrieves the property
> via
> {{{}message.getContextualProperty("set.content.type.for.empty.request"){}}},
> which builds a merged context cache from Bus → Service → Endpoint → Exchange
> → Message using shallow {{putAll()}} calls. The nested
> {{jaxrs.filter.properties}} map is present in the cache as an opaque {{Map}}
> value under that key, but its contents are never unpacked. Consequently,
> {{getContextualProperty()}} returns {{{}null{}}}, the
> {{set.content.type.for.empty.request}} configuration is silently ignored, and
> the {{Content-Type}} header is always added to non-GET requests with an empty
> entity body.
> *Working scenario (for contrast):*
> Setting the property at the {{Client}} or {{WebTarget}} level works correctly:
>
> {{client.property("set.content.type.for.empty.request", "FALSE")//
> orwebTarget.property("set.content.type.for.empty.request", "FALSE")}}
>
> This goes through {{ClientImpl.WebTargetImpl.request()}} which calls
> {{{}clientCfg.getRequestContext().putAll(configProps){}}}, placing the
> property *directly at the top level* of the request context.
> {{getContextualProperty()}} then finds it correctly.
> *Expected Behaviour:*
> When {{set.content.type.for.empty.request}} is set to {{FALSE}} via
> {{{}Invocation.Builder.property(){}}}, the HTTP transport should honour the
> setting and omit the {{Content-Type}} header for non-GET requests with an
> empty entity body, consistent with the behaviour when the same property is
> set via {{Client.property()}} or {{{}WebTarget.property(){}}}.
> *Affected versions:* Verified with CXF 3.1.18 and 3.5.5. Likely affects other
> maintained branches.
> *Steps to reproduce:*
>
> {{Client client = ClientBuilder.newClient();Invocation.Builder builder =
> client.target("http://example.com/api";).request();}}
> {{builder.property("set.content.type.for.empty.request", "FALSE");// Issue a
> POST/DELETE with no entity body}}
> {{// Observed: Content-Type header is still sent// Expected: Content-Type
> header is omitted}}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
