Skip to content

EncoderHttpMessageWriter adds a Content-Type header even if there's no body #32620

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
NadChel opened this issue Apr 11, 2024 · 4 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another

Comments

@NadChel
Copy link
Contributor

NadChel commented Apr 11, 2024

I was examining this Spring Cloud Gateway issue and discovered that its resolution is blocked by the fact that org.springframework.http.codec.EncoderHttpMessageWriter adds a default Content-Type header anyway, regardless of whether the body Publisher is empty or not. Here's an MRE

package org.springframework.cloud.gateway._misc;

import java.util.Collections;

import org.junit.jupiter.api.Test;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;

import org.springframework.cloud.gateway.filter.factory.rewrite.CachedBodyOutputMessage;
import org.springframework.core.ResolvableType;
import org.springframework.core.codec.CharSequenceEncoder;
import org.springframework.core.codec.Encoder;
import org.springframework.core.io.buffer.DefaultDataBufferFactory;
import org.springframework.http.HttpHeaders;
import org.springframework.http.ReactiveHttpOutputMessage;
import org.springframework.http.codec.EncoderHttpMessageWriter;
import org.springframework.web.server.ServerWebExchange;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Answers.RETURNS_DEEP_STUBS;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;

public class GenericTest {
	@Test
	public void test() {
		Encoder<CharSequence> encoder = CharSequenceEncoder.textPlainOnly();
		EncoderHttpMessageWriter<CharSequence> writer = new EncoderHttpMessageWriter<>(encoder);
		ServerWebExchange exchangeMock = mock(ServerWebExchange.class, RETURNS_DEEP_STUBS);
		given(exchangeMock.getResponse().bufferFactory()).willReturn(DefaultDataBufferFactory.sharedInstance);
		ReactiveHttpOutputMessage outputMessage = new CachedBodyOutputMessage(exchangeMock,
				HttpHeaders.writableHttpHeaders(HttpHeaders.EMPTY));
		Mono<Void> mono = writer.write(Mono.empty(), ResolvableType.forClass(String.class),
				null, outputMessage, Collections.emptyMap());
		StepVerifier.create(mono)
				.verifyComplete();
		assertThat(outputMessage.getHeaders()).doesNotContainKey(HttpHeaders.CONTENT_TYPE);
	}
}

To better match the original issue, I used org.springframework.cloud.gateway.filter.factory.rewrite.CachedBodyOutputMessage, but I believe it doesn't really matter. You can replace it with e.g. org.springframework.mock.http.client.reactive.MockClientHttpRequest, and it will still be reproducible

		ReactiveHttpOutputMessage outputMessage = new MockClientHttpRequest(HttpMethod.POST, "/");

The problem is updateContentType(..) which sets a Content-Type header

	@Override
	public Mono<Void> write(Publisher<? extends T> inputStream, ResolvableType elementType,
			@Nullable MediaType mediaType, ReactiveHttpOutputMessage message, Map<String, Object> hints) {

		MediaType contentType = updateContentType(message, mediaType);

		Flux<DataBuffer> body = this.encoder.encode(
				inputStream, message.bufferFactory(), elementType, contentType, hints);
	@Nullable
	private MediaType updateContentType(ReactiveHttpOutputMessage message, @Nullable MediaType mediaType) {
		MediaType result = message.getHeaders().getContentType();
		if (result != null) {
			return result;
		}
		MediaType fallback = this.defaultMediaType;
		result = (useFallback(mediaType, fallback) ? fallback : mediaType);
		if (result != null) {
			result = addDefaultCharset(result, fallback);
			message.getHeaders().setContentType(result);
		}
		return result;
	}

Spring Cloud Gateway currently uses Spring Web 6.1.5

You may assign me to this issue if you want. I wrote a straightforward fix and added an extra test (will formally submit a PR soon)

@NadChel

This comment was marked as off-topic.

@poutsma poutsma self-assigned this Apr 22, 2024
@poutsma poutsma added status: superseded An issue that has been superseded by another in: web Issues in web modules (web, webmvc, webflux, websocket) and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Apr 23, 2024
@poutsma
Copy link
Contributor

poutsma commented Apr 23, 2024

Superseded in favor of #32622.

@poutsma poutsma closed this as not planned Won't fix, can't repro, duplicate, stale Apr 23, 2024
poutsma pushed a commit that referenced this issue Apr 23, 2024
@NadChel
Copy link
Contributor Author

NadChel commented Apr 24, 2024

Superseded in favor of #32622.

Are issues superseded by PRs? Aren't they closed by PRs and superseded by other issues?

@poutsma
Copy link
Contributor

poutsma commented Apr 24, 2024

Thank you for your suggestion on how we should handle our issue tracker. We will take it into consideration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another
Projects
None yet
Development

No branches or pull requests

3 participants