Fix cloning issue in CodecConfigurer for multipart writers

Backport of b236176

Closes gh-24194

Author: rstoyanchev

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.defaultCodecs.getTypedWriters(forMultipart));
+		result.addAll(this.defaultCodecs.getTypedWriters());
 		result.addAll(this.customCodecs.getTypedWriters().keySet());
-
-		result.addAll(this.defaultCodecs.getObjectWriters(forMultipart));
+		result.addAll(this.defaultCodecs.getObjectWriters());
 		result.addAll(this.customCodecs.getObjectWriters().keySet());
-
 		result.addAll(this.defaultCodecs.getCatchAllWriters());
 		return result;
 	}

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

@@ -354,13 +354,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({"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<?>> getTypedWriters(boolean forMultipart) {
+	final List<HttpMessageWriter<?>> getBaseTypedWriters() {
 		if (!this.registerDefaults) {
 			return Collections.emptyList();
 		}
@@ -370,10 +380,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));
@@ -389,14 +395,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()));
@@ -408,10 +420,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

@@ -106,8 +106,8 @@ public void defaultWriters() {
 		assertEquals(DataBufferEncoder.class, getNextEncoder(writers).getClass());
 		assertEquals(ResourceHttpMessageWriter.class, writers.get(index.getAndIncrement()).getClass());
 		assertStringEncoder(getNextEncoder(writers), true);
-		assertEquals(MultipartHttpMessageWriter.class, writers.get(this.index.getAndIncrement()).getClass());
 		assertEquals(ProtobufHttpMessageWriter.class, writers.get(index.getAndIncrement()).getClass());
+		assertEquals(MultipartHttpMessageWriter.class, writers.get(this.index.getAndIncrement()).getClass());
 		assertEquals(Jackson2JsonEncoder.class, getNextEncoder(writers).getClass());
 		assertEquals(Jackson2SmileEncoder.class, getNextEncoder(writers).getClass());
 		assertEquals(Jaxb2XmlEncoder.class, getNextEncoder(writers).getClass());
@@ -161,7 +161,7 @@ public void enableLoggingRequestDetails() {
 	}
 
 	@Test
-	public void cloneConfigurer() {
+	public void clonedConfigurer() {
 		ClientCodecConfigurer clone = this.configurer.clone();
 
 		Jackson2JsonDecoder jackson2Decoder = new Jackson2JsonDecoder();
@@ -186,6 +186,30 @@ public void cloneConfigurer() {
 		assertEquals(10, writers.size());
 	}
 
+	@Test // gh-24194
+	public void cloneShouldNotDropMultipartCodecs() {
+
+		ClientCodecConfigurer clone = this.configurer.clone();
+		List<HttpMessageWriter<?>> writers =
+				findCodec(clone.getWriters(), MultipartHttpMessageWriter.class).getPartWriters();
+
+		assertEquals(10, writers.size());
+	}
+
+	@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();
+
+		assertEquals(10, writers.size());
+	}
+
 	private Decoder<?> getNextDecoder(List<HttpMessageReader<?>> readers) {
 		HttpMessageReader<?> reader = readers.get(this.index.getAndIncrement());
 		assertEquals(DecoderHttpMessageReader.class, reader.getClass());