WebClient and RestClient's defaultRequest(..) is not invoked early enough

[injae-kim]

Background

/**
 * Global option to specify a header to be added to every request,
 * if the request does not already contain such a header.
 */
Builder defaultHeader(String header, String... values);

WebClient and RestClient have default[Header|Cookie|Request|..] that useful to set default value on request.

spring-framework/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java

Lines 502 to 508 in 899de4f

	private void initHeaders(HttpHeaders out) {
	if (!CollectionUtils.isEmpty(defaultHeaders)) {
	out.putAll(defaultHeaders);
	}
	if (!CollectionUtils.isEmpty(this.headers)) {
	out.putAll(this.headers);
	}

As you can see above code, defaultHeader value is set only when it's not contained in headers.

Question

/**
 * Provide a consumer to customize every request being built.
 * @param defaultRequest the consumer to use for modifying requests
 */
Builder defaultRequest(Consumer<RequestHeadersSpec<?>> defaultRequest);

spring-framework/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java

Lines 481 to 484 in 899de4f

	private ClientRequest.Builder initRequestBuilder() {
	if (defaultRequest != null) {
	defaultRequest.accept(this);
	}

But on defaultRequest(..), it can override request's values unlike defaultHeader cause it's called on every request being built.

Reproduce

// DefaultWebClientTests.java
@Test
void defaultRequest_override() {
	ThreadLocal<String> context = new NamedThreadLocal<>("foo");

	WebClient client = this.builder
			.defaultRequest(spec -> spec.accept(MediaType.APPLICATION_JSON)) // default
			.build();

	client.get().uri("/path")
			.accept(MediaType.IMAGE_PNG) // set
			.retrieve().bodyToMono(Void.class).block(Duration.ofSeconds(10));

	ClientRequest request = verifyAndGetRequest();
	assertThat(request.headers().getAccept()).isEqualTo(MediaType.IMAGE_PNG); // ❌Expected: <image/png> but actual: <application/json>
}

I think defaultRequest(spec -> spec.accept(MEDIA_TYPE)) will override all media types that set by accept(..) on this request.

Q. is above behavior of defaultRequest(..) is intended? thanks!

[injae-kim]

FYI, I found this issue while investigating #32028 😃

If this behavior of defaultRequest(..) is intended, feel free to close this issue~!
(I simply think that defaultRequest works similarly with defaultHeader, setting default value)

[poutsma]

Thanks for spotting this odd behavior, which is indeed not intentional. However, the fact remains that the current behavior has been present since 5.0, and changing it, to do effectively the opposite of what it is currently doing, can affect many users.

Fixing this issue is trivial—I have a fix locally—but that fix won't be merged until 6.2.

[poutsma]

Note that—even though this is a bug—it will not be backported to the 6.1, 6.0, or 5.3 branches, as it fixing it will break backward compatibility.

[injae-kim]

even though this is a bug—it will not be backported to the 6.1, 6.0, or 5.3 branches, as it fixing it will break backward compatibility.

Aha I understood! I agree with your opinion. I just wondered it's bug or intended behavior.

Anyway I'm glad to found this trivial bug, and please share if there's something I can help~ thanks! 🙇