davsclaus commented on code in PR #26746:
URL: https://github.com/apache/camel/pull/26746#discussion_r4072425635
##########
components/camel-aws/camel-aws2-s3/src/main/java/org/apache/camel/component/aws2/s3/utils/AWS2S3Utils.java:
##########
@@ -137,16 +138,17 @@ public static byte[] toByteArray(InputStream is, final
int size) throws IOExcept
public static String determineKey(final Exchange exchange,
AWS2S3Configuration configuration) {
String key = exchange.getIn().getHeader(AWS2S3Constants.KEY,
String.class);
if (ObjectHelper.isEmpty(key)) {
+ // only the configured key (supplied by the route) may be a
dynamic simple expression;
+ // a key provided through the header is used literally and never
evaluated
key = configuration.getKeyName();
+ if (key != null && hasSimpleFunction(key)) {
Review Comment:
🟡 Moving the evaluation inside the `isEmpty` block also moves it *before*
the `key == null` check, which changes one more behaviour that isn't mentioned
in the description or the upgrade guide:
a configured `keyName` that contains a Simple function which evaluates to
`null` previously passed the null check (the raw expression string was
non-null), then evaluated to `null`, and `determineKey` returned `null` —
pushing the failure into the AWS SDK call. It now throws
`IllegalArgumentException("AWS S3 Key header missing.")` instead.
That's an improvement — failing at the producer with a clear message beats a
null key reaching S3 — so I'm not suggesting reverting it. But it's a second
behaviour change riding along with the first, and worth either a line in the
upgrade guide or a test, since nothing currently covers it. The same applies to
`determineBucketName` above.
(The message text is also now slightly off for this path — it says "Key
header missing" when the configured `keyName` was present but evaluated to
nothing.)
--
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]
