[jira] [Commented] (CAMEL-24579) DataFormat.marshal graph parameter should be @Nullable

["Claus Ibsen (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CAMEL-24579?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18110203#comment-18110203 ] 

Claus Ibsen commented on CAMEL-24579:
-------------------------------------

PR: https://github.com/apache/camel/pull/25994

> DataFormat.marshal graph parameter should be @Nullable
> ------------------------------------------------------
>
>                 Key: CAMEL-24579
>                 URL: https://issues.apache.org/jira/browse/CAMEL-24579
>             Project: Camel
>          Issue Type: Bug
>          Components: camel-core
>            Reporter: Claus Ibsen
>            Assignee: Claus Ibsen
>            Priority: Major
>
> org.apache.camel.spi is @NullMarked (since 4.21, CAMEL-22640) and DataFormat.marshal has no per-parameter @Nullable on its graph parameter, so it is treated as non-null by contract:
>     void marshal(Exchange exchange, Object graph, OutputStream stream) throws Exception;
> However, MarshalProcessor (camel-support) passes the message body straight through with no null guard:
>     // MarshalProcessor.java
>     final Object originalBody = in.getBody();
>     Object body = originalBody;
>     ...
>     dataFormat.marshal(exchange, body, osb);
> and Message.getBody() is itself explicitly @Nullable. So Camel can hand a value it declares nullable to a parameter it declares non-null. This is invisible for Java implementations (null just arrives), but for Kotlin implementations of DataFormat, the compiler emits a non-null assertion at the start of the overriding method, which throws NPE before user code runs, whenever the body is legitimately null (e.g. on an error route, or for a DataFormat that marshals from Exchange state rather than from graph).
> This was not caught by NullAway because org.apache.camel.support.processor is not @NullMarked.
> DataFormat.java was not touched by the original CAMEL-22640 sweep (67f6a37d473c) -- it inherited non-null-by-default purely from the package-level @NullMarked, without anyone reviewing whether graph can actually be null at its real call site. This is the same category of gap as the still-open CAMEL-24460 (ProducerTemplate body parameters should be nullable).
> Before 4.21.0, org.apache.camel.spi was not @NullMarked, so graph was a platform type and null was harmless.
> Fix: annotate the parameter to match actual runtime behavior:
>     void marshal(Exchange exchange, @Nullable Object graph, OutputStream stream) throws Exception;
> This is a Javadoc/contract-only change -- MarshalProcessor's runtime behavior is unchanged, so it does not affect existing Java implementations, and does not risk breaking DataFormat implementations (such as Kotlin ones) that intentionally marshal based on Exchange state rather than the graph parameter.
> Reported by Petr H. on the dev list.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)