[jira] [Comment Edited] (CXF-9235) Invocation.Builder.property() settings ignored by HTTP transport for set.content.type.for.empty.request

["Freeman Yue Fang (Jira)" <ji...@apache.org>]

    [ 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)