Fix cloning issue in CodecConfigurer for multipart writers

Closes gh-24194

spring-web/src/main/java/org/springframework/http/codec/support/BaseCodecConfigurer.java

@@ -105,25 +105,12 @@ public List<HttpMessageReader<?>> getReaders() {
 	@Override
 	public List<HttpMessageWriter<?>> getWriters() {
 		this.defaultCodecs.applyDefaultConfig(this.customCodecs);
-		return getWritersInternal(false);
-	}
-
 
-	/**
-	 * Internal method that returns the configured writers.
-	 * @param forMultipart whether to returns writers for general use ("false"),
-	 * or for multipart requests only ("true"). Generally the two sets are the
-	 * same except for the multipart writer itself.
-	 */
-	protected List<HttpMessageWriter<?>> getWritersInternal(boolean forMultipart) {
 		List<HttpMessageWriter<?>> result = new ArrayList<>();
-
 		result.addAll(this.customCodecs.getTypedWriters().keySet());
-		result.addAll(this.defaultCodecs.getTypedWriters(forMultipart));
-
+		result.addAll(this.defaultCodecs.getTypedWriters());
 		result.addAll(this.customCodecs.getObjectWriters().keySet());
-		result.addAll(this.defaultCodecs.getObjectWriters(forMultipart));
-
+		result.addAll(this.defaultCodecs.getObjectWriters());
 		result.addAll(this.defaultCodecs.getCatchAllWriters());
 		return result;
 	}

spring-web/src/main/java/org/springframework/http/codec/support/BaseDefaultCodecs.java

@@ -355,13 +355,23 @@ final List<HttpMessageReader<?>> getCatchAllReaders() {
 	}
 
 	/**
-	 * Return writers that support specific types.
-	 * @param forMultipart whether to returns writers for general use ("false"),
-	 * or for multipart requests only ("true"). Generally the two sets are the
-	 * same except for the multipart writer itself.
+	 * Return all writers that support specific types.
 	 */
-	@SuppressWarnings({ "unchecked", "rawtypes" })
-	final List<HttpMessageWriter<?>> getTypedWriters(boolean forMultipart) {
+	@SuppressWarnings({"rawtypes" })
+	final List<HttpMessageWriter<?>> getTypedWriters() {
+		if (!this.registerDefaults) {
+			return Collections.emptyList();
+		}
+		List<HttpMessageWriter<?>> writers = getBaseTypedWriters();
+		extendTypedWriters(writers);
+		return writers;
+	}
+
+	/**
+	 * Return "base" typed writers only, i.e. common to client and server.
+	 */
+	@SuppressWarnings("unchecked")
+	final List<HttpMessageWriter<?>> getBaseTypedWriters() {
 		if (!this.registerDefaults) {
 			return Collections.emptyList();
 		}
@@ -371,10 +381,6 @@ final List<HttpMessageWriter<?>> getTypedWriters(boolean forMultipart) {
 		writers.add(new EncoderHttpMessageWriter<>(new DataBufferEncoder()));
 		writers.add(new ResourceHttpMessageWriter());
 		writers.add(new EncoderHttpMessageWriter<>(CharSequenceEncoder.textPlainOnly()));
-		// No client or server specific multipart writers currently..
-		if (!forMultipart) {
-			extendTypedWriters(writers);
-		}
 		if (protobufPresent) {
 			Encoder<?> encoder = this.protobufEncoder != null ? this.protobufEncoder : new ProtobufEncoder();
 			writers.add(new ProtobufHttpMessageWriter((Encoder) encoder));
@@ -390,14 +396,20 @@ protected void extendTypedWriters(List<HttpMessageWriter<?>> typedWriters) {
 
 	/**
 	 * Return Object writers (JSON, XML, SSE).
-	 * @param forMultipart whether to returns writers for general use ("false"),
-	 * or for multipart requests only ("true"). Generally the two sets are the
-	 * same except for the multipart writer itself.
 	 */
-	final List<HttpMessageWriter<?>> getObjectWriters(boolean forMultipart) {
+	final List<HttpMessageWriter<?>> getObjectWriters() {
 		if (!this.registerDefaults) {
 			return Collections.emptyList();
 		}
+		List<HttpMessageWriter<?>> writers = getBaseObjectWriters();
+		extendObjectWriters(writers);
+		return writers;
+	}
+
+	/**
+	 * Return "base" object writers only, i.e. common to client and server.
+	 */
+	final List<HttpMessageWriter<?>> getBaseObjectWriters() {
 		List<HttpMessageWriter<?>> writers = new ArrayList<>();
 		if (jackson2Present) {
 			writers.add(new EncoderHttpMessageWriter<>(getJackson2JsonEncoder()));
@@ -409,10 +421,6 @@ final List<HttpMessageWriter<?>> getObjectWriters(boolean forMultipart) {
 			Encoder<?> encoder = this.jaxb2Encoder != null ? this.jaxb2Encoder : new Jaxb2XmlEncoder();
 			writers.add(new EncoderHttpMessageWriter<>(encoder));
 		}
-		// No client or server specific multipart writers currently..
-		if (!forMultipart) {
-			extendObjectWriters(writers);
-		}
 		return writers;
 	}
 

spring-web/src/main/java/org/springframework/http/codec/support/ClientDefaultCodecsImpl.java

@@ -54,9 +54,9 @@ class ClientDefaultCodecsImpl extends BaseDefaultCodecs implements ClientCodecCo
 
 	ClientDefaultCodecsImpl(ClientDefaultCodecsImpl other) {
 		super(other);
-		this.multipartCodecs = new DefaultMultipartCodecs(other.multipartCodecs);
+		this.multipartCodecs = (other.multipartCodecs != null ?
+				new DefaultMultipartCodecs(other.multipartCodecs) : null);
 		this.sseDecoder = other.sseDecoder;
-		this.partWritersSupplier = other.partWritersSupplier;
 	}
 
 
@@ -132,10 +132,8 @@ private static class DefaultMultipartCodecs implements ClientCodecConfigurer.Mul
 		DefaultMultipartCodecs() {
 		}
 
-		DefaultMultipartCodecs(@Nullable DefaultMultipartCodecs other) {
-			if (other != null) {
-				this.writers.addAll(other.writers);
-			}
+		DefaultMultipartCodecs(DefaultMultipartCodecs other) {
+			this.writers.addAll(other.writers);
 		}
 
 

spring-web/src/main/java/org/springframework/http/codec/support/DefaultClientCodecConfigurer.java

@@ -16,7 +16,11 @@
 
 package org.springframework.http.codec.support;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import org.springframework.http.codec.ClientCodecConfigurer;
+import org.springframework.http.codec.HttpMessageWriter;
 
 /**
  * Default implementation of {@link ClientCodecConfigurer}.
@@ -29,11 +33,12 @@ public class DefaultClientCodecConfigurer extends BaseCodecConfigurer implements
 
 	public DefaultClientCodecConfigurer() {
 		super(new ClientDefaultCodecsImpl());
-		((ClientDefaultCodecsImpl) defaultCodecs()).setPartWritersSupplier(() -> getWritersInternal(true));
+		((ClientDefaultCodecsImpl) defaultCodecs()).setPartWritersSupplier(this::getPartWriters);
 	}
 
 	private DefaultClientCodecConfigurer(DefaultClientCodecConfigurer other) {
 		super(other);
+		((ClientDefaultCodecsImpl) defaultCodecs()).setPartWritersSupplier(this::getPartWriters);
 	}
 
 
@@ -52,4 +57,14 @@ protected BaseDefaultCodecs cloneDefaultCodecs() {
 		return new ClientDefaultCodecsImpl((ClientDefaultCodecsImpl) defaultCodecs());
 	}
 
+	private List<HttpMessageWriter<?>> getPartWriters() {
+		List<HttpMessageWriter<?>> result = new ArrayList<>();
+		result.addAll(this.customCodecs.getTypedWriters().keySet());
+		result.addAll(this.defaultCodecs.getBaseTypedWriters());
+		result.addAll(this.customCodecs.getObjectWriters().keySet());
+		result.addAll(this.defaultCodecs.getBaseObjectWriters());
+		result.addAll(this.defaultCodecs.getCatchAllWriters());
+		return result;
+	}
+
 }

spring-web/src/test/java/org/springframework/http/codec/support/ClientCodecConfigurerTests.java

@@ -104,8 +104,8 @@ public void defaultWriters() {
 		assertThat(getNextEncoder(writers).getClass()).isEqualTo(DataBufferEncoder.class);
 		assertThat(writers.get(index.getAndIncrement()).getClass()).isEqualTo(ResourceHttpMessageWriter.class);
 		assertStringEncoder(getNextEncoder(writers), true);
-		assertThat(writers.get(this.index.getAndIncrement()).getClass()).isEqualTo(MultipartHttpMessageWriter.class);
 		assertThat(writers.get(index.getAndIncrement()).getClass()).isEqualTo(ProtobufHttpMessageWriter.class);
+		assertThat(writers.get(this.index.getAndIncrement()).getClass()).isEqualTo(MultipartHttpMessageWriter.class);
 		assertThat(getNextEncoder(writers).getClass()).isEqualTo(Jackson2JsonEncoder.class);
 		assertThat(getNextEncoder(writers).getClass()).isEqualTo(Jackson2SmileEncoder.class);
 		assertThat(getNextEncoder(writers).getClass()).isEqualTo(Jaxb2XmlEncoder.class);
@@ -159,7 +159,7 @@ public void enableLoggingRequestDetails() {
 	}
 
 	@Test
-	public void cloneConfigurer() {
+	public void clonedConfigurer() {
 		ClientCodecConfigurer clone = this.configurer.clone();
 
 		Jackson2JsonDecoder jackson2Decoder = new Jackson2JsonDecoder();
@@ -184,6 +184,30 @@ public void cloneConfigurer() {
 		assertThat(writers).hasSize(10);
 	}
 
+	@Test // gh-24194
+	public void cloneShouldNotDropMultipartCodecs() {
+
+		ClientCodecConfigurer clone = this.configurer.clone();
+		List<HttpMessageWriter<?>> writers =
+				findCodec(clone.getWriters(), MultipartHttpMessageWriter.class).getPartWriters();
+
+		assertThat(writers).hasSize(10);
+	}
+
+	@Test
+	public void cloneShouldNotBeImpactedByChangesToOriginal() {
+
+		ClientCodecConfigurer clone = this.configurer.clone();
+
+		this.configurer.registerDefaults(false);
+		this.configurer.customCodecs().register(new Jackson2JsonEncoder());
+
+		List<HttpMessageWriter<?>> writers =
+				findCodec(clone.getWriters(), MultipartHttpMessageWriter.class).getPartWriters();
+
+		assertThat(writers).hasSize(10);
+	}
+
 	private Decoder<?> getNextDecoder(List<HttpMessageReader<?>> readers) {
 		HttpMessageReader<?> reader = readers.get(this.index.getAndIncrement());
 		assertThat(reader).isInstanceOf(DecoderHttpMessageReader.class);