Skip to content

Cxf 9233 fix response context #3373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
vp340 wants to merge 9 commits into from
Closed

Cxf 9233 fix response context #3373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
12f1beb
move LIVE_LOGGING_PROP from AbstractLoggingInterceptor to Message
vp340 Aug 13, 2026
622e240
remove the LIVE_LOGGING_PROPERTY from the ResponseContext
vp340 Aug 13, 2026
c8c2aec
Merge branch 'apache:main' into CXF-9233-Fix-Response-Context
vp340 Aug 18, 2026
66886ed
restore previous situation
vp340 Aug 18, 2026
08c8e80
restore previous situation
vp340 Aug 18, 2026
6dc70e4
add in the constructor the directive not to propagate the LIVE_LOGGI…
vp340 Aug 18, 2026
00e09ab
add test
vp340 Aug 18, 2026
2a65368
add methods to manage the propagation of ResponseContext properties c…
vp340 Aug 18, 2026
6d619a7
add some test
vp340 Aug 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 31 additions & 10 deletions core/src/main/java/org/apache/cxf/endpoint/ClientImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,7 @@
import java.io.Serializable;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.WeakHashMap;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Executor;
import java.util.logging.Level;
Expand Down Expand Up @@ -107,6 +100,35 @@ public class ClientImpl
protected Map<Thread, ResponseContext> responseContext
= Collections.synchronizedMap(new WeakHashMap<Thread, ResponseContext>());

/**
* Set of properties that should not be propagated into the ResponseContext from IN Message
*/
private static final Set<String> RESPONSE_CONTEXT_EXCLUDED_IN_PROPERTIES = new HashSet<>(
List.of(
// remove the recursive reference if present
Message.INVOCATION_CONTEXT
)
);

/**
* Method that a cxf submodule can call to add a property not to propagate from IN Message into the ResponseContext
* @param property to exclude
*/
public static void addResponseContextExcludedInProperty(String property) {
RESPONSE_CONTEXT_EXCLUDED_IN_PROPERTIES.add(property);
}
public static void addAllResponseContextExcludedInProperties(Set<String> properties) {
RESPONSE_CONTEXT_EXCLUDED_IN_PROPERTIES.addAll(properties);
}

/**
* Method to remove RESPONSE_CONTEXT_EXCLUDED_IN_PROPERTIES from ResponseContext Map
* @param context ResponseContext Map
*/
protected void filterResponseContextProperties(Map<String, Object> context) {
RESPONSE_CONTEXT_EXCLUDED_IN_PROPERTIES.forEach(context::remove);
}

protected Executor executor;

public ClientImpl(Bus b, Endpoint e) {
Expand Down Expand Up @@ -648,8 +670,7 @@ protected Object[] processResult(Message message,
if (inMsg != null) {
if (null != resContext) {
resContext.putAll(inMsg);
// remove the recursive reference if present
resContext.remove(Message.INVOCATION_CONTEXT);
filterResponseContextProperties(resContext);
setResponseContext(resContext);
}
resList = CastUtils.cast(inMsg.getContent(List.class));
Expand Down
124 changes: 124 additions & 0 deletions core/src/test/java/org/apache/cxf/endpoint/ClientImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
package org.apache.cxf.endpoint;


import java.lang.reflect.Field;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

import org.apache.cxf.BusFactory;
import org.apache.cxf.message.Message;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;

import static org.junit.Assert.*;

public class ClientImplTest {

private static class TestClientImpl extends ClientImpl {

public TestClientImpl() {
super(BusFactory.newInstance().createBus(), null);
}

void filter(Map<String, Object> context) {
filterResponseContextProperties(context);
}
}

private final TestClientImpl testClientImpl = new TestClientImpl();

private static Set<String> getExcludedProperties() throws Exception {
Field field = ClientImpl.class
.getDeclaredField("RESPONSE_CONTEXT_EXCLUDED_IN_PROPERTIES");

field.setAccessible(true);

@SuppressWarnings("unchecked")
Set<String> properties = (Set<String>) field.get(null);

return properties;
}

private static Set<String> defaultExcludedProperties;

@BeforeClass
public static void initDefaults() throws Exception {
defaultExcludedProperties =
new HashSet<>(getExcludedProperties());
}

@Before
public void setUp() throws Exception {
Set<String> properties = getExcludedProperties();

properties.clear();
properties.addAll(defaultExcludedProperties);
}

@Test
public void shouldFilterDefaultExcludedProperty() {

Map<String, Object> context = new HashMap<>();
context.put(Message.INVOCATION_CONTEXT, "invocation-context");
context.put("property.to.keep", "value");

testClientImpl.filter(context);

assertFalse(context.containsKey(Message.INVOCATION_CONTEXT));
assertEquals("value", context.get("property.to.keep"));
}

@Test
public void shouldFilterPropertyAddedWithAdd() {
String property = "my.custom.property";

ClientImpl.addResponseContextExcludedInProperty(property);

Map<String, Object> context = new HashMap<>();
context.put(property, "custom-value");
context.put("property.to.keep", "value");

testClientImpl.filter(context);

assertFalse(context.containsKey(property));
assertEquals("value", context.get("property.to.keep"));
}

@Test
public void shouldFilterPropertiesAddedWithAddAll() {
Set<String> properties = Set.of(
"custom.property.1",
"custom.property.2"
);

ClientImpl.addAllResponseContextExcludedInProperties(properties);

Map<String, Object> context = new HashMap<>();
context.put("custom.property.1", "value1");
context.put("custom.property.2", "value2");
context.put("property.to.keep", "keep");

testClientImpl.filter(context);

assertFalse(context.containsKey("custom.property.1"));
assertFalse(context.containsKey("custom.property.2"));
assertEquals("keep", context.get("property.to.keep"));
}

@Test
public void shouldKeepPropertiesNotExcluded() {

Map<String, Object> context = new HashMap<>();
context.put("property.1", "value1");
context.put("property.2", "value2");

testClientImpl.filter(context);

assertEquals(2, context.size());
assertEquals("value1", context.get("property.1"));
assertEquals("value2", context.get("property.2"));
}
}
5 changes: 5 additions & 0 deletions rt/features/logging/src/main/java/org/apache/cxf/ext/logging/LoggingInInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import org.apache.cxf.phase.Phase;
import org.apache.cxf.phase.PhaseInterceptor;

import static org.apache.cxf.endpoint.ClientImpl.addResponseContextExcludedInProperty;

/**
*
*/
Expand Down Expand Up @@ -71,6 +73,9 @@ public LoggingInInterceptor(PrintWriter writer) {

public LoggingInInterceptor(LogEventSender sender) {
super(Phase.PRE_INVOKE, sender);

//Make sure that the LIVE_LOGGING_PROP won't be propagated into the ResponseContext from IN Messages
addResponseContextExcludedInProperty(LIVE_LOGGING_PROP);

@reta reta Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vp340 thanks for another alternative, the problem I see with this one is two fold: LoggingXxxInteceptor has to be aware about the ClientImpl specifics/internals but this is generic feature that works for client (we have many) or/and server (same, many).

@vp340 vp340 Aug 22, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @reta , I understand your point.
All module must be discern (I thought wrongly that the logging module could be aware of the core one ... having his dependency).
Thanks a lot for all the lessons about design pattern in such big project like cxf :) and for guiding me through all these solutions .

I'm glad at least we have discover the full picture about the problem!
So the only solutions remains acting on the single modules.

For me your solution in the logging module (change LIVE_LOGGING_PROP adding true/false based on client/server) works fine and giving the results of the test solve the ghost RESP_OUT.
If I were U, I would only consider to add the string "client/server" instead of "true/false" in order to be more understandable for the posterity ( but is up to U ... U are the pro one :) ).

In the core module...the other solution that I thought right now (to be taken with a grain of salt) is to change approach in the ClientImpl ... and add a sort of white-list of the properties that needs to be propagated in the ResponseContext. But this will change completely the actual policy from... let pass all and remove one ....to ... let pass only the needed.
I don't have the knowledge to know what are the properties needed and I don't even know if this is a suitable idea...
If U find it ok and want to try to implement it let me know ...I could prepare at least the skeleton where to add the white-list properties if U want.
((In the last message where U said the you have many clients I wondered if this 'problem' of propagating all props into ResponseContext is common for other Client-s other than the ClientImpl... if so this last idea is less appetizing and would need to change all ...or find some common point to put all the props if they are the same)).

Let me know what U think.
If the last idea isn't suitable in my opinion we can merge your #3372 .

Have a great job!

Valentino Porta

@reta reta Aug 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me your solution in the logging module (change LIVE_LOGGING_PROP adding true/false based on client/server) works fine and giving the results of the test solve the ghost RESP_OUT.
If I were U, I would only consider to add the string "client/server" instead of "true/false" in order to be more understandable for the posterity ( but is up to U ... U are the pro one :) ).

Thank you @vp340 , yes, I think it is good idea to make the fix more understandable, I will work on it

In the core module...the other solution that I thought right now (to be taken with a grain of salt) is to change approach in the ClientImpl ... and add a sort of white-list of the properties that needs to be propagated in the ResponseContext. But this will change completely the actual policy from... let pass all and remove one ....to ... let pass only the needed.

The issue to be fair has nothing to do with the CXF but the way Camel does pass the context from in- message to out- message (see please [1]) inside its CXF wrappers, so I think the fix within CXF is not even needed (but we could probably try the one we already have to help). So in my opinion, going with the simple solution on CXF side is more than enough, we could not (and should not) introduce the complexity here, thank you.

[1] https://github.com/apache/camel/blob/main/components/camel-cxf/camel-cxf-soap/src/main/java/org/apache/camel/component/cxf/jaxws/DefaultCxfBinding.java#L530

@vp340 vp340 Aug 23, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @reta ,
Yeah the camel CXF wrapper does :
// make sure the "requestor role" property does not get propagated as we do switch role
responseContext.remove(Message.REQUESTOR_ROLE);
outMessage.putAll(responseContext);
// Do we still need to put the response context back like this
outMessage.put(CxfConstants.RESPONSE_CONTEXT, responseContext);
...
but as we said for the cxf core module, being the LIVE_LOGGING_PROP protected the camel module is not aware of it so it can't remove it like it does with Message.REQUESTOR_ROLE .
The only way would be adding a white-list there, but as U said (and I fully agree) for this side log feature your simple fix is more than enough as long as we see the RESP_OUT log. :)
I'll leave it up to U whether it's worth pursuing a Camel-side fix as well, but as long as it doesn't emerge a more tricky problem with the propagation of the response context ...I don't find it really necessary and could be also risky.

Thanks for all the time U dedicated to this problem.
See U in the next one (hopefully not ;) ... )

Keep up the great work!

Valentino Porta

}

public Collection<PhaseInterceptor<? extends Message>> getAdditionalInterceptors() {
Expand Down
17 changes: 17 additions & 0 deletions rt/features/logging/src/test/java/org/apache/cxf/ext/logging/LoggingInInterceptorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.io.IOException;
import java.io.OutputStream;
import java.lang.reflect.Field;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -29,6 +30,7 @@
import java.util.Map;
import java.util.Set;

import org.apache.cxf.endpoint.ClientImpl;
import org.apache.cxf.ext.logging.event.LogEvent;
import org.apache.cxf.io.CachedOutputStream;
import org.apache.cxf.message.ExchangeImpl;
Expand All @@ -38,11 +40,13 @@
import org.junit.Before;
import org.junit.Test;

import static org.apache.cxf.ext.logging.AbstractLoggingInterceptor.LIVE_LOGGING_PROP;
import static org.apache.cxf.ext.logging.event.DefaultLogEventMapper.MASKED_HEADER_VALUE;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalToIgnoringCase;
import static org.hamcrest.Matchers.hasSize;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

public class LoggingInInterceptorTest {
private static final String TEST_HEADER_VALUE = "TestValue";
Expand Down Expand Up @@ -234,4 +238,17 @@ public void shouldLogMultipartPayloadNoHeaders() throws IOException {

assertThat(event.getPayload(), equalToIgnoringCase(buf.toString()));
}

@Test
public void shouldAddResponseContextInExcludedProperty() throws NoSuchFieldException, IllegalAccessException {
Field field = ClientImpl.class
.getDeclaredField("RESPONSE_CONTEXT_EXCLUDED_IN_PROPERTIES");

field.setAccessible(true);

@SuppressWarnings("unchecked")
Set<String> actual = (Set<String>) field.get(null);

assertTrue(actual.contains(LIVE_LOGGING_PROP));
}
}