gnodet-bot commented on code in PR #28991:
URL: https://github.com/apache/camel/pull/28991#discussion_r4448670230


##########
components/camel-netty/src/main/java/org/apache/camel/component/netty/NettyConfiguration.java:
##########
@@ -97,7 +97,10 @@ public class NettyConfiguration extends 
NettyServerBootstrapConfiguration implem
               description = "Only used for TCP. You can transfer the exchange 
over the wire instead of just the body. The following fields are"
                             + " transferred: In body, Out body, fault body, In 
headers, Out headers, fault headers, exchange properties, exchange"
                             + " exception. This requires that the objects are 
serializable. Camel will exclude any non-serializable objects and"
-                            + " log it at WARN level.")
+                            + " log it at WARN level. The connection headers 
(CamelNettyRemoteAddress, CamelNettyLocalAddress,"
+                            + " CamelNettyChannelHandlerContext, 
CamelNettySSLSession and the CamelNettySSLClientCert headers) are always"

Review Comment:
   ⚠️ **`CamelNettyChannel` not mentioned here.** `NETTY_CHANNEL` is in 
`CONNECTION_KEYS` and is actively set as an exchange property by 
`ServerChannelHandler` — it belongs in this list alongside the other 
connection-describing values. The upgrade guide already mentions it; the 
`@UriParam` description should too.



##########
components/camel-netty/src/main/java/org/apache/camel/component/netty/NettyPayloadHelper.java:
##########
@@ -151,4 +153,47 @@ public static void setOut(Exchange exchange, Object 
payload) {
         }
     }
 
+    /**
+     * The headers and exchange properties that describe the Netty connection 
of this side.
+     */
+    private static final String[] CONNECTION_KEYS = {
+            NettyConstants.NETTY_CHANNEL_HANDLER_CONTEXT, 
NettyConstants.NETTY_MESSAGE_EVENT,
+            NettyConstants.NETTY_REMOTE_ADDRESS, 
NettyConstants.NETTY_LOCAL_ADDRESS, NettyConstants.NETTY_CHANNEL,
+            NettyConstants.NETTY_SSL_SESSION, 
NettyConstants.NETTY_SSL_CLIENT_CERT_SUBJECT_NAME,
+            NettyConstants.NETTY_SSL_CLIENT_CERT_ISSUER_NAME, 
NettyConstants.NETTY_SSL_CLIENT_CERT_SERIAL_NO,
+            NettyConstants.NETTY_SSL_CLIENT_CERT_NOT_BEFORE, 
NettyConstants.NETTY_SSL_CLIENT_CERT_NOT_AFTER };
+
+    /**
+     * Unmarshals the transferred exchange into the exchange, but keeps the 
connection headers and exchange properties
+     * of this side: the connection this side accepted (or opened) is the 
source of truth for them. A connection header
+     * or property in the transferred exchange is never used, also when this 
side has no value for it (for example the
+     * SSL client certificate headers when the connection has no client 
certificate).

Review Comment:
   💡 `NETTY_MESSAGE_EVENT` (`CamelNettyMessageEvent`) is never set as a header 
or exchange property anywhere in camel-netty — only declared in 
`NettyConstants` (no `@Metadata`). Including it in `CONNECTION_KEYS` is 
harmless but misleading given the Javadoc describes these as keys that 
'describe the Netty connection'. Consider removing it or adding an explanatory 
comment.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to