From 634aba4ab63b43a700f95301edb0be0282577783 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 12 Dec 2019 21:55:13 +0000 Subject: [PATCH] Fix cloning issue in CodecConfigurer for multipart writers Backport of b23617637d21f1f49f7f0c1c9c3ccc90c1de5b92 Closes gh-24194 --- .../codec/support/BaseCodecConfigurer.java | 17 +------- .../http/codec/support/BaseDefaultCodecs.java | 42 +++++++++++-------- .../support/ClientDefaultCodecsImpl.java | 10 ++--- .../support/DefaultClientCodecConfigurer.java | 17 +++++++- .../support/ClientCodecConfigurerTests.java | 28 ++++++++++++- 5 files changed, 73 insertions(+), 41 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/codec/support/BaseCodecConfigurer.java b/spring-web/src/main/java/org/springframework/http/codec/support/BaseCodecConfigurer.java index c698d5fba324..b6e351fc8ecd 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/support/BaseCodecConfigurer.java +++ b/spring-web/src/main/java/org/springframework/http/codec/support/BaseCodecConfigurer.java @@ -105,25 +105,12 @@ public List> getReaders() { @Override public List> 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> getWritersInternal(boolean forMultipart) { List> 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; } diff --git a/spring-web/src/main/java/org/springframework/http/codec/support/BaseDefaultCodecs.java b/spring-web/src/main/java/org/springframework/http/codec/support/BaseDefaultCodecs.java index 00b0fdab3e33..e01fb3c29c29 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/support/BaseDefaultCodecs.java +++ b/spring-web/src/main/java/org/springframework/http/codec/support/BaseDefaultCodecs.java @@ -354,13 +354,23 @@ final List> 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> getTypedWriters() { + if (!this.registerDefaults) { + return Collections.emptyList(); + } + List> writers = getBaseTypedWriters(); + extendTypedWriters(writers); + return writers; + } + + /** + * Return "base" typed writers only, i.e. common to client and server. */ @SuppressWarnings("unchecked") - final List> getTypedWriters(boolean forMultipart) { + final List> getBaseTypedWriters() { if (!this.registerDefaults) { return Collections.emptyList(); } @@ -370,10 +380,6 @@ final List> 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> 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> getObjectWriters(boolean forMultipart) { + final List> getObjectWriters() { if (!this.registerDefaults) { return Collections.emptyList(); } + List> writers = getBaseObjectWriters(); + extendObjectWriters(writers); + return writers; + } + + /** + * Return "base" object writers only, i.e. common to client and server. + */ + final List> getBaseObjectWriters() { List> writers = new ArrayList<>(); if (jackson2Present) { writers.add(new EncoderHttpMessageWriter<>(getJackson2JsonEncoder())); @@ -408,10 +420,6 @@ final List> 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; } diff --git a/spring-web/src/main/java/org/springframework/http/codec/support/ClientDefaultCodecsImpl.java b/spring-web/src/main/java/org/springframework/http/codec/support/ClientDefaultCodecsImpl.java index dae7b25704d2..cc1c7f1a439c 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/support/ClientDefaultCodecsImpl.java +++ b/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); } diff --git a/spring-web/src/main/java/org/springframework/http/codec/support/DefaultClientCodecConfigurer.java b/spring-web/src/main/java/org/springframework/http/codec/support/DefaultClientCodecConfigurer.java index 737282eecd5e..382d11bec8c4 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/support/DefaultClientCodecConfigurer.java +++ b/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> getPartWriters() { + List> 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; + } + } diff --git a/spring-web/src/test/java/org/springframework/http/codec/support/ClientCodecConfigurerTests.java b/spring-web/src/test/java/org/springframework/http/codec/support/ClientCodecConfigurerTests.java index 87572c619927..aed7f71db4e2 100644 --- a/spring-web/src/test/java/org/springframework/http/codec/support/ClientCodecConfigurerTests.java +++ b/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> 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> writers = + findCodec(clone.getWriters(), MultipartHttpMessageWriter.class).getPartWriters(); + + assertEquals(10, writers.size()); + } + private Decoder getNextDecoder(List> readers) { HttpMessageReader reader = readers.get(this.index.getAndIncrement()); assertEquals(DecoderHttpMessageReader.class, reader.getClass());