aziubin commented on PR #1033:
URL: https://github.com/apache/cxf/pull/1033#issuecomment-1326759798
> Thanks @aziubin , the `MessageContentsList` is very widely used with CXF
and may be outside (since this is a public class), so none of the "isolated,
safe and will not have negative impact on existing codebase" could be asserted
for sure. Also, the change would introduce a very tricky class of errors, where
`REMOVED_MARKER` will still appear in `toArray(T[])`, `subList(...)`,
`sort(...)` ... unless we override everything (probably not a good idea).
Thank you @reta, @dkulp, I agree. MessageContentsList is core functionality,
which is used everywhere in CXF, so we should be careful. Unfortunately,
`REMOVED_MARKER` was made publicly accessible, although its meaning (the
indication of missing value in the list) points that it should be private or
protected and not leave the methods of the class. As long as `REMOVED_MARKER`
is not referenced in the code of other classes it is I think safe to stop
returning it outside in the inherited toArray method. If there is a suspicion
that other inherited methods of the ArrayList might be used too, we may address
them as well to prevent potential defects. Below is another example of
misbehavior because `REMOVED_MARKER` is passed outside of MessageContentsList
class:
```
import javax.validation.constraints.NotNull;
import javax.validation.Valid;
. . .
public boolean allocate(Integer projectId,
@Valid @NotNull Object targetIds,
@Valid @NotNull Integer[] parameterIds) { }
```
When targetIds parameter is omitted, the NotNull rule will not work because
`REMOVED_MARKER` instance is passed as argument to the targetIds parameter,
although `null` should have been passed instead. This case is covered with this
fix too.
> If we have to cover the omitted operation input, we should cover such
cases as well, we should not be asking users for workarounds.
I might not completely understand this. Do you mean for existing code, or
when developing new one assuming that parameter can be omitted? When
`REMOVED_MARKER` argument is passed to primitive type it is not compatible, as
well as when `null` is passed instead, so this fix as I can see does not change
the behavior with primitive types for existing code, but only the error
message. I do not have CXF expertise as yours, so let me know if I am missing
something. Thanks!
--
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]
