Fix AbstractMessageConverter not delegating to ContentTypeResolver when message header is null

[yoshikawaa]

Versions

• Spring Messaging 6.0.3 / 5.3.24

Problems

DefaultContentTypeResolver#resolve will return defaultMimeType if null is passed as MessageHeaders.

spring-framework/spring-messaging/src/main/java/org/springframework/messaging/converter/DefaultContentTypeResolver.java

Lines 58 to 63 in 0fbc94f

	@Override
	@Nullable
	public MimeType resolve(@Nullable MessageHeaders headers) {
	if (headers == null || headers.get(MessageHeaders.CONTENT_TYPE) == null) {
	return this.defaultMimeType;
	}

However, AbstractMessageConverter#getMimeType does not call the contentTypeResolver if null is passed as MessageHeaders.

spring-framework/spring-messaging/src/main/java/org/springframework/messaging/converter/AbstractMessageConverter.java

Lines 252 to 255 in 0fbc94f

	@Nullable
	protected MimeType getMimeType(@Nullable MessageHeaders headers) {
	return (headers != null && this.contentTypeResolver != null ? this.contentTypeResolver.resolve(headers) : null);
	}

Because of this, even if DefaultContentTypeResolver#defaultMimeType is set properly, it may be ignored.

I've confirmed this issue with Spring Cloud Stream CompositeMessageConverter.
ApplicationJsonMessageMarshallingConverter (child converter of CompositeMessageConverter) has strictContentTypeMatch=true, so it thinks it's not a supported content type.

Fixes

ContentTypeResolver#resolve accepts null, so MessageConverter should always delegate resolution to it.
Fix AbstractMessageConverter#getMimeType not checking the value of MessageHeaders.

[rstoyanchev]

It looks like this was changed in f813712#diff-f79ea4f7cb1d3ef331698d7edd1ae7cb7326a8082f34d1c55c97a29865e57d6dL238-R243 with @Nullable related updates. ContentTypeResolver is declared to accept null, and the default implementation does handle it, but after the change it is no longer called with null headers.

The PR looks good to me, unless I'm missing something, @jhoeller?

Aside from that, I'm wondering if this should be done for 6.1 since it is technically a change in behavior. I don't know how common it is to have null headers passed in. /cc @artembilan

[artembilan]

Right, Rossen, it is not common to pass a null for header value.
Even all our messaging API does not let do that: as long as we provide a null, that is a signal to remove that header.
See a MessageHeaderAccessor for example:

	/**
	 * Set the value for the given header name.
	 * <p>If the provided value is {@code null}, the header will be removed.
	 */
	public void setHeader(String name, @Nullable Object value) {
		if (isReadOnly(name)) {
			throw new IllegalArgumentException("'" + name + "' header is read-only");
		}
		verifyType(name, value);
		if (value != null) {
			// Modify header if necessary
			if (!ObjectUtils.nullSafeEquals(value, getHeader(name))) {
				this.modified = true;
				this.headers.getRawHeaders().put(name, value);
			}
		}
		else {
			// Remove header if available
			if (this.headers.containsKey(name)) {
				this.modified = true;
				this.headers.getRawHeaders().remove(name);
			}
		}
	}

Perhaps for consistency our MessageHeaders must be fixed respectively - to ignore entries of the input Map which has null values...