Do not put a null message attribute when contentType is unsupported - #1703
Open
anjeongkyun wants to merge 1 commit into
Open
Do not put a null message attribute when contentType is unsupported#1703anjeongkyun wants to merge 1 commit into
anjeongkyun wants to merge 1 commit into
Conversation
getContentTypeMessageAttribute returns null for a contentType header that is neither a MimeType nor a String, and the caller put that null straight into the attribute map. The SDK then fails marshalling the whole publish request. Fall back to the same warn-and-skip the loop already uses for header types SNS cannot carry.
anjeongkyun
requested review from
MatejNedic,
maciejwalkowiak and
tomazfernandes
as code owners
September 13, 2026 06:58
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
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.
📢 Type of change
📜 Description
SnsHeaderConverterUtil.getContentTypeMessageAttributereturnsnullwhen thecontentTypeheader is neither aMimeTypenor aString, and the caller put that return value straight into the attribute map:The map then carries
contentType -> null.PublishRequest.builder().messageAttributes(map).build()accepts it, so nothing fails until the request is marshalled:That happens before the call leaves the client, so the publish fails entirely rather than just dropping the header.
The loop already has a branch for header types SNS cannot carry: it logs a warning and leaves the header out. This change routes the unsupported
contentTypedown that same path, and pulls the warning into a small private method so both places use it.The SQS side handles the same case:
SqsHeaderMapper.getContentTypeMessageAttributeis not@Nullableand falls back to an empty string attribute. I kept SNS consistent with its own loop rather than copying that, since skipping the header matches what this class already does for values it cannot convert. Happy to switch it to the empty-string fallback if you would rather the two modules line up.This is not a regression. The same code was in
TopicMessageChannelbefore it was extracted intoSnsHeaderConverterUtilin #1574.💡 Motivation and Context
toSnsMessageAttributesis used by the sync (TopicMessageChannel), batch (DefaultSnsMessageConverter) and async (DefaultSnsPublishMessageConverter) publish paths, so any application setting acontentTypeheader that is not aStringorMimeTypeloses the message on all three.💚 How did you test it?
Added
sendMessage_withUnsupportedContentTypeHeader_shouldNotSetItAsMessageAttributetoTopicMessageChannelTest, alongside the existing header conversion tests. It fails on currentmain(the attribute map contains the key with a null value) and passes with the change.I also confirmed the marshalling failure end to end before writing the fix, by sending a request built from the converted attributes through a real
SnsClientpointed at an unreachable endpoint, which produced theSdkClientExceptionabove.spring-cloud-aws-snspasses in full (119 tests). I ran the repo-wide unit tests too: the only failure isS3InboundChannelAdapterTests.s3InboundChannelAdapter, which fails the same way on an unmodifiedmainhere.📝 Checklist
Reference docs are unchanged because the documented behaviour is unaffected: supported
contentTypevalues are converted exactly as before.On breaking changes: an application that today sends an unsupported
contentTypegets a failed publish, and after this it gets a successful publish with a warning and nocontentTypeattribute. Nothing that works today changes.🔮 Next steps
None.