CAMEL-24157: camel-aws2 - Fix medium-severity findings from July 2026 review#24864
Merged
Conversation
…validation, and health check Fix subscribeSNStoSQS queueArn validation that was dead code due to nested isNotEmpty(isNotEmpty()) always returning true. Also update stale error message. Require kmsMasterKeyId when serverSideEncryptionEnabled is true instead of silently creating unencrypted topics. Fix health check NPE when region is unset and replace listSubscriptions (requires broad IAM) with getTopicAttributes (matches producer permissions). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…tRead, and table-creation retry Map empty JSON arrays to DynamoDB List type (L) instead of empty StringSet which DynamoDB rejects. Use Number type check for list elements to match prior CAMEL-24156 fix on scalar values. Add missing consistentRead to ScanCommand matching GetItem/Query/ExecuteStatement. Fix table-creation wait loop to catch ResourceNotFoundException by type instead of unreliable string matching against SDK v2 messages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
… to send operations Move the messageGroupIdStrategy validation from the constructor to processSingleMessage and sendBatchMessage where it is actually needed. This allows purgeQueue, deleteQueue, listQueues, and deleteMessage operations to work on FIFO queues without requiring a messageGroupIdStrategy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…, presigner, and deprecate maxConnections Fix ListObjectsV2 pagination fallback to use startAfter instead of continuationToken when nextContinuationToken is absent (S3-compatible stores). Apply SSE-C customer key headers on getObject, getObjectRange, and consumer fileName path (read operations were missing SSE-C unlike write operations). Fix removePrefixOnMove to use substring instead of regex replaceFirst which caused PatternSyntaxException on prefixes with special characters and skipped deleteAfterRead on failure. Fix presigner to support session credentials, profile credentials, and null region. Deprecate maxConnections option that was never wired to the S3 client. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
gnodet
approved these changes
Jul 17, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Claude Code review on behalf of gnodet
All 12 medium-severity findings reviewed. Every fix is correct.
Highlights
| Component | Fix | Verdict |
|---|---|---|
| SNS | isNotEmpty(isNotEmpty(queueArn)) — double-wrap always returned true, making validation dead code. Fixed to single call + corrected error message |
✅ Good catch — subtle bug |
| SNS | SSE without kmsMasterKeyId now throws instead of silently creating unencrypted topic |
✅ Correct fail-fast |
| SNS health | Null-guard on Region.of(); getTopicAttributes() replaces listSubscriptions() (narrower IAM scope) |
✅ Correct |
| DDB | Empty array → L type (List) instead of SS (StringSet) — DynamoDB rejects empty sets |
✅ Correct |
| DDB | catch (ResourceNotFoundException) by type instead of ase.getMessage().contains(...) |
✅ Correct for SDK v2 |
| DDB | consistentRead added to ScanCommand — was the only read command missing it |
✅ |
| SQS | FIFO messageGroupIdStrategy check deferred to send-time — unblocks purge/delete/list ops |
✅ Correct scope reduction |
| S3 | V2 pagination fallback uses startAfter instead of continuationToken with object key |
✅ Correct — key is not a continuation token; fixes MinIO/Ceph |
| S3 | SSE-C headers on getObject, getObjectRange, and consumer fileName path |
✅ Correct — read ops were missing them |
| S3 | replaceFirst → substring for prefix removal — prevents PatternSyntaxException on prefixes with .+* |
✅ Also adds null/startsWith guards |
| S3 | Presigner now supports session/profile credentials + null region | ✅ |
| S3 | maxConnections deprecated (dead option, never wired) |
✅ |
Minor notes
-
DDB transformer overlap with PR #24863: Both PRs modify
Ddb2JsonDataTypeTransformer.javawith theNumberinstance check. #24863 also changes theInteger/Double→Numbercheck above this line. Whoever merges second will need a trivial rebase. -
SSE-C code duplication: The SSE-C header block is repeated 3 times (consumer
fileNamepath, producergetObject, producergetObjectRange). A private helper likeapplySSECustomerKey(GetObjectRequest.Builder)would reduce duplication. Not blocking — just a future cleanup opportunity.
LGTM 👍
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
… deprecation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 15 tested, 27 compile-only — current: 13 all testedMaveniverse Scalpel detected 42 affected modules (current approach: 13).
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes 12 medium-severity findings from the July 2026 deep code review of camel-aws2 components (S3, SQS, SNS, DDB), covering the "quick wins" and "medium effort" groups from the CAMEL-24157 umbrella.
Claude Code on behalf of davsclaus
camel-aws2-sns (3 fixes)
isNotEmpty(isNotEmpty(queueArn))always returned true, making the validation unreachable. Fixed to singleisNotEmpty()and updated stale error message.serverSideEncryptionEnabled=truewithoutkmsMasterKeyIdsilently created unencrypted topics. Now throwsIllegalArgumentException.Region.of(null)NPE when region relies on SDK defaults; replacedlistSubscriptions()(requires broad IAM) withgetTopicAttributes()(matches producer permissions).camel-aws2-ddb (3 fixes)
Number(matching prior CAMEL-24156 fix)..consistentRead(determineConsistentRead()).ase.getMessage().contains("ResourceNotFoundException")never matched SDK v2 messages. Changed tocatch (ResourceNotFoundException)by type.camel-aws2-sqs (1 fix)
messageGroupIdStrategy, blockingpurgeQueue/deleteQueue/listQueuesoperations that don't need it. Moved check toprocessSingleMessage/sendBatchMessage.camel-aws2-s3 (5 fixes)
continuationToken(V1 leftover); now usesstartAfterwhich is the correct V2 equivalent. Fixes S3-compatible stores (MinIO, Ceph).getObject,getObjectRange, and consumerfileNamepath were missing SSE-C headers (write operations had them). SSE-C encrypted objects returned HTTP 400 on read.String.replaceFirst()treated prefix as regex causingPatternSyntaxExceptionon prefixes with.+*. Failure also skippeddeleteAfterRead→ endless reprocessing. Changed tosubstring().getOrCreatePresigner()only supported basic + default credentials; now handles session credentials, profile credentials, and null region.@UriParamexisted with getter/setter but was never wired to the S3 HTTP client.Test plan
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com