Revert optimization in StringDecoder

This commit reverts the first optimizations listed in
fa096dc, as the default delimiters
do vary, namely by the charset given in the message mime type.
The mimetype charset might not be compatible with ASCII (i.e. anything
but UTF-8 or ISO-8859-1, for instance it might be UTF-16), and will not
successfully find the default delimiters as a consequence.

Added test to indicate the bug.

Author: poutsma

spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java

@@ -23,6 +23,8 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 import java.util.stream.Collectors;
 
 import org.reactivestreams.Publisher;
@@ -35,6 +37,7 @@
 import org.springframework.core.io.buffer.PooledDataBuffer;
 import org.springframework.core.log.LogFormatUtils;
 import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
 import org.springframework.util.MimeType;
 import org.springframework.util.MimeTypeUtils;
 
@@ -63,20 +66,18 @@ public final class StringDecoder extends AbstractDataBufferDecoder<String> {
 	/** The default delimiter strings to use, i.e. {@code \r\n} and {@code \n}. */
 	public static final List<String> DEFAULT_DELIMITERS = Arrays.asList("\r\n", "\n");
 
-	private static final List<byte[]> DEFAULT_DELIMITER_BYTES = DEFAULT_DELIMITERS.stream()
-			.map(str -> str.getBytes(StandardCharsets.UTF_8))
-			.collect(Collectors.toList());
 
-
-	@Nullable
 	private final List<String> delimiters;
 
 	private final boolean stripDelimiter;
 
+	private final ConcurrentMap<Charset, List<byte[]>> delimitersCache = new ConcurrentHashMap<>();
+
 
-	private StringDecoder(@Nullable List<String> delimiters, boolean stripDelimiter, MimeType... mimeTypes) {
+	private StringDecoder(List<String> delimiters, boolean stripDelimiter, MimeType... mimeTypes) {
 		super(mimeTypes);
-		this.delimiters = delimiters != null ? new ArrayList<>(delimiters) : null;
+		Assert.notEmpty(delimiters, "'delimiters' must not be empty");
+		this.delimiters = new ArrayList<>(delimiters);
 		this.stripDelimiter = stripDelimiter;
 	}
 
@@ -90,9 +91,7 @@ public boolean canDecode(ResolvableType elementType, @Nullable MimeType mimeType
 	public Flux<String> decode(Publisher<DataBuffer> inputStream, ResolvableType elementType,
 			@Nullable MimeType mimeType, @Nullable Map<String, Object> hints) {
 
-		List<byte[]> delimiterBytes = this.delimiters != null ?
-				this.delimiters.stream().map(s -> s.getBytes(getCharset(mimeType))).collect(Collectors.toList()) :
-				DEFAULT_DELIMITER_BYTES;
+		List<byte[]> delimiterBytes = getDelimiterBytes(mimeType);
 
 		Flux<DataBuffer> inputFlux = Flux.from(inputStream)
 				.flatMapIterable(dataBuffer -> splitOnDelimiter(dataBuffer, delimiterBytes))
@@ -103,6 +102,13 @@ public Flux<String> decode(Publisher<DataBuffer> inputStream, ResolvableType ele
 		return super.decode(inputFlux, elementType, mimeType, hints);
 	}
 
+	private List<byte[]> getDelimiterBytes(@Nullable MimeType mimeType) {
+		return this.delimitersCache.computeIfAbsent(getCharset(mimeType),
+				charset -> this.delimiters.stream()
+						.map(s -> s.getBytes(charset))
+						.collect(Collectors.toList()));
+	}
+
 	/**
 	 * Splits the given data buffer on delimiter boundaries. The returned Flux contains a
 	 * {@link #END_FRAME} buffer after each delimiter.
@@ -234,17 +240,16 @@ public static StringDecoder textPlainOnly(boolean ignored) {
 	 * Create a {@code StringDecoder} for {@code "text/plain"}.
 	 */
 	public static StringDecoder textPlainOnly() {
-		return textPlainOnly(null, true);
+		return textPlainOnly(DEFAULT_DELIMITERS, true);
 	}
 
 	/**
 	 * Create a {@code StringDecoder} for {@code "text/plain"}.
-	 * @param delimiters delimiter strings to use to split the input stream, if
-	 * {@code null} by default {@link #DEFAULT_DELIMITERS} is used.
+	 * @param delimiters delimiter strings to use to split the input stream
 	 * @param stripDelimiter whether to remove delimiters from the resulting
 	 * input strings.
 	 */
-	public static StringDecoder textPlainOnly(@Nullable List<String> delimiters, boolean stripDelimiter) {
+	public static StringDecoder textPlainOnly(List<String> delimiters, boolean stripDelimiter) {
 		return new StringDecoder(delimiters, stripDelimiter, new MimeType("text", "plain", DEFAULT_CHARSET));
 	}
 
@@ -263,17 +268,16 @@ public static StringDecoder allMimeTypes(boolean ignored) {
 	 * Create a {@code StringDecoder} that supports all MIME types.
 	 */
 	public static StringDecoder allMimeTypes() {
-		return allMimeTypes(null, true);
+		return allMimeTypes(DEFAULT_DELIMITERS, true);
 	}
 
 	/**
 	 * Create a {@code StringDecoder} that supports all MIME types.
-	 * @param delimiters delimiter strings to use to split the input stream, if
-	 * {@code null} by default {@link #DEFAULT_DELIMITERS} is used.
+	 * @param delimiters delimiter strings to use to split the input stream
 	 * @param stripDelimiter whether to remove delimiters from the resulting
 	 * input strings.
 	 */
-	public static StringDecoder allMimeTypes(@Nullable List<String> delimiters, boolean stripDelimiter) {
+	public static StringDecoder allMimeTypes(List<String> delimiters, boolean stripDelimiter) {
 		return new StringDecoder(delimiters, stripDelimiter,
 				new MimeType("text", "plain", DEFAULT_CHARSET), MimeTypeUtils.ALL);
 	}

spring-core/src/test/java/org/springframework/core/codec/StringDecoderTests.java

@@ -16,7 +16,7 @@
 
 package org.springframework.core.codec;
 
-import java.nio.charset.StandardCharsets;
+import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -29,8 +29,11 @@
 import org.springframework.core.ResolvableType;
 import org.springframework.core.io.buffer.AbstractDataBufferAllocatingTestCase;
 import org.springframework.core.io.buffer.DataBuffer;
+import org.springframework.util.MimeType;
 import org.springframework.util.MimeTypeUtils;
 
+import static java.nio.charset.StandardCharsets.UTF_16BE;
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.Assert.*;
 
 /**
@@ -69,22 +72,43 @@ public void canDecode() {
 
 	@Test
 	public void decodeMultibyteCharacter() {
-		String s = "üéø";
-		Flux<DataBuffer> source = toSingleByteDataBuffers(s);
+		String u = "ü";
+		String e = "é";
+		String o = "ø";
+		String s = String.format("%s\n%s\n%s", u, e, o);
+		Flux<DataBuffer> source = toDataBuffers(s, 1, UTF_8);
 
 		Flux<String> output = this.decoder.decode(source, ResolvableType.forClass(String.class),
 				null, Collections.emptyMap());
 		StepVerifier.create(output)
-				.expectNext(s)
+				.expectNext(u, e, o)
 				.verifyComplete();
 	}
 
-	private Flux<DataBuffer> toSingleByteDataBuffers(String s) {
-		byte[] bytes = s.getBytes(StandardCharsets.UTF_8);
+	@Test
+	public void decodeMultibyteCharacterUtf16() {
+		String u = "ü";
+		String e = "é";
+		String o = "ø";
+		String s = String.format("%s\n%s\n%s", u, e, o);
+		Flux<DataBuffer> source = toDataBuffers(s, 2, UTF_16BE);
+
+		MimeType mimeType = MimeTypeUtils.parseMimeType("text/plain;charset=utf-16be");
+		Flux<String> output = this.decoder.decode(source, ResolvableType.forClass(String.class),
+				mimeType, Collections.emptyMap());
+		StepVerifier.create(output)
+				.expectNext(u, e, o)
+				.verifyComplete();
+	}
+
+	private Flux<DataBuffer> toDataBuffers(String s, int length, Charset charset) {
+		byte[] bytes = s.getBytes(charset);
 
 		List<DataBuffer> dataBuffers = new ArrayList<>();
-		for (byte b : bytes) {
-			dataBuffers.add(this.bufferFactory.wrap(new byte[]{b}));
+		for (int i = 0; i < bytes.length; i += length) {
+			DataBuffer dataBuffer = this.bufferFactory.allocateBuffer(length);
+			dataBuffer.write(bytes, i, length);
+			dataBuffers.add(dataBuffer);
 		}
 		return Flux.fromIterable(dataBuffers);
 	}