Skip to content

ObjectStringMessageConverter.toMessage() can't be used with arbitrary payload #8757

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mrpiggi opened this issue Oct 10, 2023 · 1 comment
Closed

Comments

@mrpiggi
Copy link
Contributor

mrpiggi commented Oct 10, 2023

I am not quite sure, what the original intention of ObjectStringMessageConverter was, but currently using

ObjectStringMessageConverter converter = new ObjectStringMessageConverter();
Integer payload = 1234;
Message<?> converted = converter.toMessage(payload, null);
assertThat(converted).isNotNull();

fails, as Message<?> converted is always null when payload is anything else than String.class as AbstractMessageConverter.canConvertTo()

https://github.com/spring-projects/spring-framework/blob/80646591367b93d220b9dc0b0a126b203edc140c/spring-messaging/src/main/java/org/springframework/messaging/converter/AbstractMessageConverter.java#L231-L235

always returns null due to StringMessageConverter.supports()
https://github.com/spring-projects/spring-framework/blob/80646591367b93d220b9dc0b0a126b203edc140c/spring-messaging/src/main/java/org/springframework/messaging/converter/StringMessageConverter.java#L50-L55
returns false in this case.

The question arises, whether this is intentional. If ObjectStringMessageConverter.toMessage() should not be used with anything else than String payload explicitly throwing an IllegalArgumentException would probably be a better option. For the opposite case I will provide a PR.

@mrpiggi
Copy link
Contributor Author

mrpiggi commented Oct 10, 2023

Once I am on it:
package org.springframework.messaging.converter is marked with both @NonNullApi and @NonNullFields. Should this hold for package org.springframework.integration.support.converter as well? If confirmed, I would prepare a PR.

@artembilan artembilan added this to the 6.2.0-RC1 milestone Oct 11, 2023
@artembilan artembilan added type: enhancement in: core and removed type: bug status: waiting-for-triage The issue need to be evaluated and its future decided labels Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants