gnodet-bot commented on code in PR #26842:
URL: https://github.com/apache/camel/pull/26842#discussion_r4092618134
##########
components/camel-aws/camel-aws2-s3/src/main/java/org/apache/camel/component/aws2/s3/utils/AWS2S3Utils.java:
##########
@@ -50,15 +50,16 @@ private AWS2S3Utils() {
public static String determineBucketName(final Exchange exchange,
AWS2S3Configuration configuration) {
String bucketName =
exchange.getIn().getHeader(AWS2S3Constants.OVERRIDE_BUCKET_NAME, String.class);
if (ObjectHelper.isEmpty(bucketName)) {
+ // only the configured bucket name (supplied by the route) may be
a dynamic simple expression;
+ // a bucket name provided through the header is used literally and
never evaluated
bucketName = configuration.getBucketName();
Review Comment:
Stale Javadoc. The method-level Javadoc for determineBucketName() (and its
twin determineKey()) still says nothing about the new contract: header values
are passed through literally and never evaluated as Simple expressions. This is
the entire point of the security fix — anyone maintaining this code in six
months won't know that the asymmetry between configuration.getBucketName()
(evaluated) and the header (literal) is intentional. The Javadoc should state
it explicitly.
##########
components/camel-aws/camel-aws2-s3/src/test/java/org/apache/camel/component/aws2/s3/integration/S3GetObjectDynamicKeyOperationIT.java:
##########
@@ -81,7 +79,9 @@ protected RouteBuilder createRouteBuilder() {
public void configure() {
context.setVariable("myVar", "myCamel");
- String awsEndpoint = "aws2-s3://" + name.get() +
"?autoCreateBucket=true";
+ // the dynamic key is a simple expression supplied through the
endpoint configuration (keyName)
+ String awsEndpoint
+ = "aws2-s3://" + name.get() +
"?autoCreateBucket=true&keyName=RAW(${variable.global:myVar}.txt)";
Review Comment:
Integration coverage gap. This IT now exercises config keyName is evaluated
as Simple (correct) but no longer covers the actual bug path: a header value
containing dollar-curly-brace used literally. That path is covered at unit
level but no IT exercises it.
--
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]
