From b7da46e28d4cc3abab27a854d52a2a6179ebdd84 Mon Sep 17 00:00:00 2001 From: hutiefang76 <137664623+hutiefang76@users.noreply.github.com> Date: Wed, 7 Oct 2026 18:54:37 +0800 Subject: [PATCH 1/2] Handle primitive array form values without reference-array casts Read primitive and reference array elements through Array, retaining collection formatting and multipart binary writer priority. Add encoder regressions and a changelog entry. Fixes #3607 --- CHANGELOG.md | 6 + .../form/UrlencodedFormContentProcessor.java | 18 +- .../form/multipart/ManyParametersWriter.java | 10 +- .../form/FormEncoderPrimitiveArrayTest.java | 126 ++++++++++ .../multipart/ManyParametersWriterTest.java | 223 ++++++++++++++++++ 5 files changed, 375 insertions(+), 8 deletions(-) create mode 100644 form/src/test/java/feign/form/FormEncoderPrimitiveArrayTest.java create mode 100644 form/src/test/java/feign/form/multipart/ManyParametersWriterTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f3098fba1..9902c7a039 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +### Version 13.17 + +* Form encoders handle primitive array field values without an `Object[]` cast failure. + URL-encoded fields follow the request's `CollectionFormat`; multipart numeric and boolean arrays + produce repeated parts. Multipart `byte[]` values remain a single binary part. + ### Version 13.16 * Path-style expansion repeats the entry name for each collection or array value inside a map, diff --git a/form/src/main/java/feign/form/UrlencodedFormContentProcessor.java b/form/src/main/java/feign/form/UrlencodedFormContentProcessor.java index 6c82a5c9d2..4fdca05288 100644 --- a/form/src/main/java/feign/form/UrlencodedFormContentProcessor.java +++ b/form/src/main/java/feign/form/UrlencodedFormContentProcessor.java @@ -20,15 +20,16 @@ import feign.CollectionFormat; import feign.RequestTemplate; import feign.codec.EncodeException; +import java.lang.reflect.Array; import java.net.URLEncoder; import java.nio.charset.Charset; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Map; import java.util.Map.Entry; import java.util.Objects; import java.util.stream.Collectors; +import java.util.stream.IntStream; import java.util.stream.Stream; import lombok.SneakyThrows; import lombok.val; @@ -90,8 +91,7 @@ private CharSequence createKeyValuePair( if (value == null) { return encodedKey; } else if (value.getClass().isArray()) { - return createKeyValuePair( - collectionFormat, encodedKey, Arrays.stream((Object[]) value), charset); + return createKeyValuePair(collectionFormat, encodedKey, arrayElements(value), charset); } else if (value instanceof Collection) { return createKeyValuePair( collectionFormat, encodedKey, ((Collection) value).stream(), charset); @@ -112,4 +112,16 @@ private CharSequence createKeyValuePair( .collect(Collectors.toList()); return collectionFormat.join(key, stringValues, charset); } + + /** + * Streams the elements of an array, boxing primitive elements. An {@code (Object[])} cast cannot + * be used instead, because it fails with a {@link ClassCastException} on primitive arrays such as + * {@code int[]} or {@code char[]}. + * + * @param array a non-null array of any component type. + * @return a stream over the array's elements, in order. + */ + private static Stream arrayElements(Object array) { + return IntStream.range(0, Array.getLength(array)).mapToObj(index -> Array.get(array, index)); + } } diff --git a/form/src/main/java/feign/form/multipart/ManyParametersWriter.java b/form/src/main/java/feign/form/multipart/ManyParametersWriter.java index 6e743ea21c..2b39603ea6 100644 --- a/form/src/main/java/feign/form/multipart/ManyParametersWriter.java +++ b/form/src/main/java/feign/form/multipart/ManyParametersWriter.java @@ -18,6 +18,7 @@ import static lombok.AccessLevel.PRIVATE; import feign.codec.EncodeException; +import java.lang.reflect.Array; import lombok.experimental.FieldDefaults; import lombok.val; @@ -34,8 +35,7 @@ public class ManyParametersWriter extends AbstractWriter { @Override public boolean isApplicable(Object value) { if (value.getClass().isArray()) { - Object[] values = (Object[]) value; - return values.length > 0 && parameterWriter.isApplicable(values[0]); + return Array.getLength(value) > 0 && parameterWriter.isApplicable(Array.get(value, 0)); } if (!(value instanceof Iterable)) { return false; @@ -49,9 +49,9 @@ public boolean isApplicable(Object value) { public void write(Output output, String boundary, String key, Object value) throws EncodeException { if (value.getClass().isArray()) { - val objects = (Object[]) value; - for (val object : objects) { - parameterWriter.write(output, boundary, key, object); + int length = Array.getLength(value); + for (int index = 0; index < length; index++) { + parameterWriter.write(output, boundary, key, Array.get(value, index)); } } else if (value instanceof Iterable) { val iterable = (Iterable) value; diff --git a/form/src/test/java/feign/form/FormEncoderPrimitiveArrayTest.java b/form/src/test/java/feign/form/FormEncoderPrimitiveArrayTest.java new file mode 100644 index 0000000000..09186249aa --- /dev/null +++ b/form/src/test/java/feign/form/FormEncoderPrimitiveArrayTest.java @@ -0,0 +1,126 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feign.form; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; + +import feign.CollectionFormat; +import feign.RequestTemplate; +import java.time.Duration; +import java.util.Arrays; +import java.util.LinkedHashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +/** + * URL encoded forms expand array values into repeated fields. Primitive arrays have to expand like + * reference arrays, so their elements are read reflectively instead of being cast to {@code + * Object[]}. + */ +class FormEncoderPrimitiveArrayTest { + + private static final String URLENCODED = "application/x-www-form-urlencoded; charset=utf-8"; + + @Test + void booleanArrayIsExplodedIntoRepeatedFields() { + assertThat(encode("flags", new boolean[] {true, false})).isEqualTo("flags=true&flags=false"); + } + + @Test + void byteArrayIsExplodedIntoRepeatedFields() { + assertThat(encode("sizes", new byte[] {1, 2})).isEqualTo("sizes=1&sizes=2"); + } + + @Test + void charArrayIsExplodedIntoRepeatedFields() { + assertThat(encode("letters", new char[] {'a', 'b'})).isEqualTo("letters=a&letters=b"); + } + + @Test + void doubleArrayIsExplodedIntoRepeatedFields() { + assertThat(encode("ratios", new double[] {1.25, 2.5})).isEqualTo("ratios=1.25&ratios=2.5"); + } + + @Test + void floatArrayIsExplodedIntoRepeatedFields() { + assertThat(encode("ratios", new float[] {1.5F, 2.5F})).isEqualTo("ratios=1.5&ratios=2.5"); + } + + @Test + void intArrayIsExplodedIntoRepeatedFields() { + assertThat(encode("sizes", new int[] {1, 2})).isEqualTo("sizes=1&sizes=2"); + } + + @Test + void longArrayIsExplodedIntoRepeatedFields() { + assertThat(encode("times", new long[] {10L, 20L})).isEqualTo("times=10×=20"); + } + + @Test + void shortArrayIsExplodedIntoRepeatedFields() { + assertThat(encode("counts", new short[] {3, 4})).isEqualTo("counts=3&counts=4"); + } + + @Test + void arrayValuesHonourTheCollectionFormatOfTheRequest() { + assertThat(encode("sizes", new int[] {1, 2}, CollectionFormat.CSV)).isEqualTo("sizes=1%2C2"); + } + + @Test + void emptyPrimitiveArrayAddsNoField() { + assertThat(encode("sizes", new int[0])).isEmpty(); + } + + @Test + void referenceArrayIsStillExplodedIntoRepeatedFields() { + assertThat(encode("tags", new String[] {"one", "two"})).isEqualTo("tags=one&tags=two"); + } + + @Test + void iterableIsStillExplodedIntoRepeatedFields() { + assertThat(encode("tags", Arrays.asList("one", "two"))).isEqualTo("tags=one&tags=two"); + } + + @Test + void scalarValueIsEncodedAsASingleField() { + assertThat(encode("timeout", Duration.ofSeconds(5))).isEqualTo("timeout=PT5S"); + } + + private static String encode(String name, Object value) { + return encode(name, value, null); + } + + private static String encode(String name, Object value, CollectionFormat collectionFormat) { + Map data = new LinkedHashMap<>(); + data.put(name, value); + return encode(data, collectionFormat); + } + + private static String encode(Map data, CollectionFormat collectionFormat) { + RequestTemplate template = new RequestTemplate(); + if (collectionFormat != null) { + template.collectionFormat(collectionFormat); + } + template.header("Content-Type", URLENCODED); + + new FormEncoder().encode(data, Map.class, template); + + assertThat(template.headers().get("Content-Type")) + .containsExactly("application/x-www-form-urlencoded; charset=UTF-8"); + return new String(template.body(), UTF_8); + } +} diff --git a/form/src/test/java/feign/form/multipart/ManyParametersWriterTest.java b/form/src/test/java/feign/form/multipart/ManyParametersWriterTest.java new file mode 100644 index 0000000000..1c6ae6faa8 --- /dev/null +++ b/form/src/test/java/feign/form/multipart/ManyParametersWriterTest.java @@ -0,0 +1,223 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feign.form.multipart; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; + +import feign.RequestTemplate; +import feign.form.FormEncoder; +import java.io.File; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +/** + * {@link ManyParametersWriter} writes arrays and {@link Iterable}s of parameters as one part per + * element. Its predicate reads the first element reflectively, so a primitive array such as {@code + * int[]} is recognised instead of failing on an {@code (Object[])} cast. + */ +class ManyParametersWriterTest { + + private static final ManyParametersWriter WRITER = new ManyParametersWriter(); + + private static final String BOUNDARY = "boundary"; + + private static final String TEXT_PART_HEADER = "Content-Type: text/plain; charset=UTF-8\r\n\r\n"; + + @Test + void arrayOfParametersIsApplicable() { + assertThat(WRITER.isApplicable(new int[] {1, 2})).isTrue(); + assertThat(WRITER.isApplicable(new boolean[] {true, false})).isTrue(); + assertThat(WRITER.isApplicable(new String[] {"one", "two"})).isTrue(); + } + + @Test + void byteArrayIsApplicableBecauseByteIsANumber() { + assertThat(WRITER.isApplicable(new byte[] {1, 2})).isTrue(); + } + + @Test + void charArrayIsNotApplicable() { + assertThat(WRITER.isApplicable(new char[] {'a', 'b'})).isFalse(); + } + + @Test + void emptyArrayIsNotApplicable() { + assertThat(WRITER.isApplicable(new int[0])).isFalse(); + assertThat(WRITER.isApplicable(new String[0])).isFalse(); + } + + @Test + void iterableOfParametersIsApplicable() { + assertThat(WRITER.isApplicable(List.of(1, 2))).isTrue(); + assertThat(WRITER.isApplicable(List.of("one", "two"))).isTrue(); + } + + @Test + void emptyIterableIsNotApplicable() { + assertThat(WRITER.isApplicable(List.of())).isFalse(); + } + + @Test + void valuesWithoutParametersAreNotApplicable() { + assertThat(WRITER.isApplicable("one")).isFalse(); + assertThat(WRITER.isApplicable(List.of(new File("file.txt")))).isFalse(); + } + + @Test + void iterableIsWrittenAsOnePartPerElement() { + Output output = new Output(UTF_8); + + WRITER.write(output, BOUNDARY, "tags", List.of("one", "two")); + + assertThat(new String(output.toByteArray(), UTF_8)) + .isEqualTo(expectedPart("tags", "one") + expectedPart("tags", "two")); + } + + @Test + void arrayOfNumbersIsWrittenAsRepeatedParts() { + List parts = textParts(map("ids", new int[] {1, 2}, "times", new long[] {10L, 20L})); + + assertThat(parts).hasSize(4); + assertThat(parts).allSatisfy(part -> assertThat(part).contains(TEXT_PART_HEADER)); + assertThat(payloadsOf(parts, "ids")).containsExactly("1", "2"); + assertThat(payloadsOf(parts, "times")).containsExactly("10", "20"); + } + + @Test + void arrayOfBooleansIsWrittenAsRepeatedParts() { + List parts = textParts(map("flags", new boolean[] {true, false})); + + assertThat(parts).hasSize(2); + assertThat(payloadsOf(parts, "flags")).containsExactly("true", "false"); + } + + @Test + void byteArrayIsWrittenAsOneBinaryPart() { + byte[] payload = {0, 1, 42, (byte) 0x80, (byte) 0xFF}; + + List parts = splitParts(encodeMultipart(map("blob", payload))); + + assertThat(parts).hasSize(1); + assertThat(new String(parts.get(0), UTF_8)) + .contains("Content-Disposition: form-data; name=\"blob\"") + .contains("Content-Type: application/octet-stream") + .contains("Content-Transfer-Encoding: binary"); + assertThat(parts.get(0)).endsWith(payload); + } + + private static String expectedPart(String name, String payload) { + return "--" + + BOUNDARY + + "\r\n" + + "Content-Disposition: form-data; name=\"" + + name + + "\"\r\n" + + TEXT_PART_HEADER + + payload + + "\r\n"; + } + + private static Map map(Object... keysAndValues) { + Map data = new LinkedHashMap<>(); + for (int index = 0; index < keysAndValues.length; index += 2) { + data.put((String) keysAndValues[index], keysAndValues[index + 1]); + } + return data; + } + + private static RequestTemplate encodeMultipart(Map data) { + RequestTemplate template = new RequestTemplate(); + template.header("Content-Type", "multipart/form-data"); + + new FormEncoder().encode(data, Map.class, template); + + return template; + } + + private static List textParts(Map data) { + List parts = new ArrayList<>(); + for (byte[] part : splitParts(encodeMultipart(data))) { + parts.add(new String(part, UTF_8)); + } + return parts; + } + + private static List payloadsOf(List parts, String name) { + List payloads = new ArrayList<>(); + for (String part : parts) { + if (part.contains("name=\"" + name + "\"")) { + payloads.add(part.substring(part.indexOf("\r\n\r\n") + 4)); + } + } + return payloads; + } + + private static List splitParts(RequestTemplate template) { + return splitParts(template.body(), boundaryOf(template)); + } + + /** Reads the boundary the processor announced in the {@code Content-Type} header. */ + private static String boundaryOf(RequestTemplate template) { + String contentType = template.headers().get("Content-Type").iterator().next(); + assertThat(contentType).startsWith("multipart/form-data; charset=UTF-8; boundary="); + return contentType.substring(contentType.indexOf("boundary=") + "boundary=".length()); + } + + private static List splitParts(byte[] body, String boundary) { + byte[] delimiter = ("--" + boundary).getBytes(UTF_8); + + List positions = new ArrayList<>(); + int position = indexOf(body, delimiter, 0); + while (position >= 0) { + positions.add(position); + position = indexOf(body, delimiter, position + delimiter.length); + } + + List parts = new ArrayList<>(); + for (int index = 0; index + 1 < positions.size(); index++) { + int start = positions.get(index) + delimiter.length; + int end = positions.get(index + 1); + if (end - start < 4 || body[start] != '\r' || body[start + 1] != '\n') { + continue; // the closing delimiter, "----", is not a part + } + parts.add(Arrays.copyOfRange(body, start + 2, end - 2)); + } + return parts; + } + + private static int indexOf(byte[] haystack, byte[] needle, int from) { + for (int index = Math.max(from, 0); index <= haystack.length - needle.length; index++) { + if (matchesAt(haystack, needle, index)) { + return index; + } + } + return -1; + } + + private static boolean matchesAt(byte[] haystack, byte[] needle, int offset) { + for (int index = 0; index < needle.length; index++) { + if (haystack[offset + index] != needle[index]) { + return false; + } + } + return true; + } +} From ecbe017c89814af87e870ac529db597698dc0237 Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Wed, 7 Oct 2026 18:03:03 -0300 Subject: [PATCH 2/2] Read form array and iterable values through one Elements helper and fold primitive-array tests into existing suites Signed-off-by: Marvin Froeder --- CHANGELOG.md | 7 +- .../form/UrlencodedFormContentProcessor.java | 25 +-- .../form/multipart/ManyParametersWriter.java | 23 +-- .../main/java/feign/form/util/Elements.java | 41 +++++ .../form/FormEncoderPrimitiveArrayTest.java | 126 -------------- .../UrlencodedFormContentProcessorTest.java | 34 ++++ .../multipart/ManyParametersWriterTest.java | 161 +++++------------- 7 files changed, 129 insertions(+), 288 deletions(-) create mode 100644 form/src/main/java/feign/form/util/Elements.java delete mode 100644 form/src/test/java/feign/form/FormEncoderPrimitiveArrayTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 9902c7a039..9aa994c248 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,9 @@ ### Version 13.17 -* Form encoders handle primitive array field values without an `Object[]` cast failure. - URL-encoded fields follow the request's `CollectionFormat`; multipart numeric and boolean arrays - produce repeated parts. Multipart `byte[]` values remain a single binary part. +* Form encoders can process primitive array fields without failing on an `Object[]` cast. + URL-encoded fields use the request's `CollectionFormat`. In multipart requests, numeric and + boolean arrays create repeated parts, and `byte[]` values stay as one binary part. Multipart + `char[]` values are still passed to the delegate encoder (#3607). ### Version 13.16 diff --git a/form/src/main/java/feign/form/UrlencodedFormContentProcessor.java b/form/src/main/java/feign/form/UrlencodedFormContentProcessor.java index 4fdca05288..db9517bd6e 100644 --- a/form/src/main/java/feign/form/UrlencodedFormContentProcessor.java +++ b/form/src/main/java/feign/form/UrlencodedFormContentProcessor.java @@ -16,11 +16,11 @@ package feign.form; import static feign.form.ContentType.URLENCODED; +import static feign.form.util.Elements.elementsOf; import feign.CollectionFormat; import feign.RequestTemplate; import feign.codec.EncodeException; -import java.lang.reflect.Array; import java.net.URLEncoder; import java.nio.charset.Charset; import java.util.Collection; @@ -29,8 +29,8 @@ import java.util.Map.Entry; import java.util.Objects; import java.util.stream.Collectors; -import java.util.stream.IntStream; import java.util.stream.Stream; +import java.util.stream.StreamSupport; import lombok.SneakyThrows; import lombok.val; @@ -90,11 +90,12 @@ private CharSequence createKeyValuePair( if (value == null) { return encodedKey; - } else if (value.getClass().isArray()) { - return createKeyValuePair(collectionFormat, encodedKey, arrayElements(value), charset); - } else if (value instanceof Collection) { + } else if (value.getClass().isArray() || value instanceof Collection) { return createKeyValuePair( - collectionFormat, encodedKey, ((Collection) value).stream(), charset); + collectionFormat, + encodedKey, + StreamSupport.stream(elementsOf(value).spliterator(), false), + charset); } return new StringBuilder() .append(encodedKey) @@ -112,16 +113,4 @@ private CharSequence createKeyValuePair( .collect(Collectors.toList()); return collectionFormat.join(key, stringValues, charset); } - - /** - * Streams the elements of an array, boxing primitive elements. An {@code (Object[])} cast cannot - * be used instead, because it fails with a {@link ClassCastException} on primitive arrays such as - * {@code int[]} or {@code char[]}. - * - * @param array a non-null array of any component type. - * @return a stream over the array's elements, in order. - */ - private static Stream arrayElements(Object array) { - return IntStream.range(0, Array.getLength(array)).mapToObj(index -> Array.get(array, index)); - } } diff --git a/form/src/main/java/feign/form/multipart/ManyParametersWriter.java b/form/src/main/java/feign/form/multipart/ManyParametersWriter.java index 2b39603ea6..4238ad8135 100644 --- a/form/src/main/java/feign/form/multipart/ManyParametersWriter.java +++ b/form/src/main/java/feign/form/multipart/ManyParametersWriter.java @@ -15,10 +15,10 @@ */ package feign.form.multipart; +import static feign.form.util.Elements.elementsOf; import static lombok.AccessLevel.PRIVATE; import feign.codec.EncodeException; -import java.lang.reflect.Array; import lombok.experimental.FieldDefaults; import lombok.val; @@ -34,30 +34,15 @@ public class ManyParametersWriter extends AbstractWriter { @Override public boolean isApplicable(Object value) { - if (value.getClass().isArray()) { - return Array.getLength(value) > 0 && parameterWriter.isApplicable(Array.get(value, 0)); - } - if (!(value instanceof Iterable)) { - return false; - } - val iterable = (Iterable) value; - val iterator = iterable.iterator(); + val iterator = elementsOf(value).iterator(); return iterator.hasNext() && parameterWriter.isApplicable(iterator.next()); } @Override public void write(Output output, String boundary, String key, Object value) throws EncodeException { - if (value.getClass().isArray()) { - int length = Array.getLength(value); - for (int index = 0; index < length; index++) { - parameterWriter.write(output, boundary, key, Array.get(value, index)); - } - } else if (value instanceof Iterable) { - val iterable = (Iterable) value; - for (val object : iterable) { - parameterWriter.write(output, boundary, key, object); - } + for (val object : elementsOf(value)) { + parameterWriter.write(output, boundary, key, object); } } } diff --git a/form/src/main/java/feign/form/util/Elements.java b/form/src/main/java/feign/form/util/Elements.java new file mode 100644 index 0000000000..1e6fe5576b --- /dev/null +++ b/form/src/main/java/feign/form/util/Elements.java @@ -0,0 +1,41 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package feign.form.util; + +import java.lang.reflect.Array; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +public final class Elements { + + private Elements() {} + + public static Iterable elementsOf(Object value) { + if (value instanceof Iterable) { + return (Iterable) value; + } + if (value == null || !value.getClass().isArray()) { + return Collections.emptyList(); + } + int length = Array.getLength(value); + List elements = new ArrayList<>(length); + for (int index = 0; index < length; index++) { + elements.add(Array.get(value, index)); + } + return elements; + } +} diff --git a/form/src/test/java/feign/form/FormEncoderPrimitiveArrayTest.java b/form/src/test/java/feign/form/FormEncoderPrimitiveArrayTest.java deleted file mode 100644 index 09186249aa..0000000000 --- a/form/src/test/java/feign/form/FormEncoderPrimitiveArrayTest.java +++ /dev/null @@ -1,126 +0,0 @@ -/* - * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package feign.form; - -import static java.nio.charset.StandardCharsets.UTF_8; -import static org.assertj.core.api.Assertions.assertThat; - -import feign.CollectionFormat; -import feign.RequestTemplate; -import java.time.Duration; -import java.util.Arrays; -import java.util.LinkedHashMap; -import java.util.Map; -import org.junit.jupiter.api.Test; - -/** - * URL encoded forms expand array values into repeated fields. Primitive arrays have to expand like - * reference arrays, so their elements are read reflectively instead of being cast to {@code - * Object[]}. - */ -class FormEncoderPrimitiveArrayTest { - - private static final String URLENCODED = "application/x-www-form-urlencoded; charset=utf-8"; - - @Test - void booleanArrayIsExplodedIntoRepeatedFields() { - assertThat(encode("flags", new boolean[] {true, false})).isEqualTo("flags=true&flags=false"); - } - - @Test - void byteArrayIsExplodedIntoRepeatedFields() { - assertThat(encode("sizes", new byte[] {1, 2})).isEqualTo("sizes=1&sizes=2"); - } - - @Test - void charArrayIsExplodedIntoRepeatedFields() { - assertThat(encode("letters", new char[] {'a', 'b'})).isEqualTo("letters=a&letters=b"); - } - - @Test - void doubleArrayIsExplodedIntoRepeatedFields() { - assertThat(encode("ratios", new double[] {1.25, 2.5})).isEqualTo("ratios=1.25&ratios=2.5"); - } - - @Test - void floatArrayIsExplodedIntoRepeatedFields() { - assertThat(encode("ratios", new float[] {1.5F, 2.5F})).isEqualTo("ratios=1.5&ratios=2.5"); - } - - @Test - void intArrayIsExplodedIntoRepeatedFields() { - assertThat(encode("sizes", new int[] {1, 2})).isEqualTo("sizes=1&sizes=2"); - } - - @Test - void longArrayIsExplodedIntoRepeatedFields() { - assertThat(encode("times", new long[] {10L, 20L})).isEqualTo("times=10×=20"); - } - - @Test - void shortArrayIsExplodedIntoRepeatedFields() { - assertThat(encode("counts", new short[] {3, 4})).isEqualTo("counts=3&counts=4"); - } - - @Test - void arrayValuesHonourTheCollectionFormatOfTheRequest() { - assertThat(encode("sizes", new int[] {1, 2}, CollectionFormat.CSV)).isEqualTo("sizes=1%2C2"); - } - - @Test - void emptyPrimitiveArrayAddsNoField() { - assertThat(encode("sizes", new int[0])).isEmpty(); - } - - @Test - void referenceArrayIsStillExplodedIntoRepeatedFields() { - assertThat(encode("tags", new String[] {"one", "two"})).isEqualTo("tags=one&tags=two"); - } - - @Test - void iterableIsStillExplodedIntoRepeatedFields() { - assertThat(encode("tags", Arrays.asList("one", "two"))).isEqualTo("tags=one&tags=two"); - } - - @Test - void scalarValueIsEncodedAsASingleField() { - assertThat(encode("timeout", Duration.ofSeconds(5))).isEqualTo("timeout=PT5S"); - } - - private static String encode(String name, Object value) { - return encode(name, value, null); - } - - private static String encode(String name, Object value, CollectionFormat collectionFormat) { - Map data = new LinkedHashMap<>(); - data.put(name, value); - return encode(data, collectionFormat); - } - - private static String encode(Map data, CollectionFormat collectionFormat) { - RequestTemplate template = new RequestTemplate(); - if (collectionFormat != null) { - template.collectionFormat(collectionFormat); - } - template.header("Content-Type", URLENCODED); - - new FormEncoder().encode(data, Map.class, template); - - assertThat(template.headers().get("Content-Type")) - .containsExactly("application/x-www-form-urlencoded; charset=UTF-8"); - return new String(template.body(), UTF_8); - } -} diff --git a/form/src/test/java/feign/form/UrlencodedFormContentProcessorTest.java b/form/src/test/java/feign/form/UrlencodedFormContentProcessorTest.java index be915fab92..81327533a3 100644 --- a/form/src/test/java/feign/form/UrlencodedFormContentProcessorTest.java +++ b/form/src/test/java/feign/form/UrlencodedFormContentProcessorTest.java @@ -28,7 +28,11 @@ import java.util.LinkedHashMap; import java.util.Map; import java.util.function.BiFunction; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; class UrlencodedFormContentProcessorTest { @@ -116,6 +120,36 @@ void collectionValueUsesPipesCollectionFormat() { Arrays.asList("one", "two"), Client::mapPipes); } + static Stream primitiveArrays() { + return Stream.of( + Arguments.of(new boolean[] {true, false}, "tags=true&tags=false"), + Arguments.of(new byte[] {1, 2}, "tags=1&tags=2"), + Arguments.of(new char[] {'a', 'b'}, "tags=a&tags=b"), + Arguments.of(new double[] {1.25, 2.5}, "tags=1.25&tags=2.5"), + Arguments.of(new float[] {1.5F, 2.5F}, "tags=1.5&tags=2.5"), + Arguments.of(new int[] {1, 2}, "tags=1&tags=2"), + Arguments.of(new long[] {10L, 20L}, "tags=10&tags=20"), + Arguments.of(new short[] {3, 4}, "tags=3&tags=4")); + } + + @ParameterizedTest + @MethodSource("primitiveArrays") + void primitiveArrayValueUsesDefaultExplodedCollectionFormat( + Object tags, String expectedTagFields) { + assertEncodedBody("from=%2B987654321&to=%2B123456789&" + expectedTagFields, tags, Client::map); + } + + @Test + void primitiveArrayIsJoinedIntoOneFieldWithCsvCollectionFormat() { + assertEncodedBody( + "from=%2B987654321&to=%2B123456789&tags=1%2C2", new int[] {1, 2}, Client::mapCsv); + } + + @Test + void emptyPrimitiveArrayValueAddsNoTagsField() { + assertEncodedBody("from=%2B987654321&to=%2B123456789&", new int[0], Client::map); + } + private void assertEncodedBody( String expectedBody, Object tags, diff --git a/form/src/test/java/feign/form/multipart/ManyParametersWriterTest.java b/form/src/test/java/feign/form/multipart/ManyParametersWriterTest.java index 1c6ae6faa8..21f1a61c05 100644 --- a/form/src/test/java/feign/form/multipart/ManyParametersWriterTest.java +++ b/form/src/test/java/feign/form/multipart/ManyParametersWriterTest.java @@ -15,34 +15,29 @@ */ package feign.form.multipart; +import static java.nio.charset.StandardCharsets.ISO_8859_1; import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; import feign.RequestTemplate; import feign.form.FormEncoder; import java.io.File; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.regex.Pattern; import org.junit.jupiter.api.Test; -/** - * {@link ManyParametersWriter} writes arrays and {@link Iterable}s of parameters as one part per - * element. Its predicate reads the first element reflectively, so a primitive array such as {@code - * int[]} is recognised instead of failing on an {@code (Object[])} cast. - */ class ManyParametersWriterTest { private static final ManyParametersWriter WRITER = new ManyParametersWriter(); - private static final String BOUNDARY = "boundary"; + private static final String FIXED_BOUNDARY = "boundary"; - private static final String TEXT_PART_HEADER = "Content-Type: text/plain; charset=UTF-8\r\n\r\n"; + private static final String TEXT_PLAIN_CONTENT_TYPE_HEADER = + "Content-Type: text/plain; charset=UTF-8\r\n\r\n"; @Test - void arrayOfParametersIsApplicable() { + void intBooleanAndStringArraysAreApplicable() { assertThat(WRITER.isApplicable(new int[] {1, 2})).isTrue(); assertThat(WRITER.isApplicable(new boolean[] {true, false})).isTrue(); assertThat(WRITER.isApplicable(new String[] {"one", "two"})).isTrue(); @@ -53,11 +48,6 @@ void byteArrayIsApplicableBecauseByteIsANumber() { assertThat(WRITER.isApplicable(new byte[] {1, 2})).isTrue(); } - @Test - void charArrayIsNotApplicable() { - assertThat(WRITER.isApplicable(new char[] {'a', 'b'})).isFalse(); - } - @Test void emptyArrayIsNotApplicable() { assertThat(WRITER.isApplicable(new int[0])).isFalse(); @@ -76,7 +66,7 @@ void emptyIterableIsNotApplicable() { } @Test - void valuesWithoutParametersAreNotApplicable() { + void scalarAndIterableOfFilesAreNotApplicable() { assertThat(WRITER.isApplicable("one")).isFalse(); assertThat(WRITER.isApplicable(List.of(new File("file.txt")))).isFalse(); } @@ -85,139 +75,66 @@ void valuesWithoutParametersAreNotApplicable() { void iterableIsWrittenAsOnePartPerElement() { Output output = new Output(UTF_8); - WRITER.write(output, BOUNDARY, "tags", List.of("one", "two")); + WRITER.write(output, FIXED_BOUNDARY, "tags", List.of("one", "two")); assertThat(new String(output.toByteArray(), UTF_8)) - .isEqualTo(expectedPart("tags", "one") + expectedPart("tags", "two")); + .isEqualTo(formatDelimitedTextPart("tags", "one") + formatDelimitedTextPart("tags", "two")); } @Test - void arrayOfNumbersIsWrittenAsRepeatedParts() { - List parts = textParts(map("ids", new int[] {1, 2}, "times", new long[] {10L, 20L})); + void intAndLongArraysAreWrittenAsOneTextPartPerElement() { + Output output = new Output(UTF_8); + + WRITER.write(output, FIXED_BOUNDARY, "ids", new int[] {1, 2}); + WRITER.write(output, FIXED_BOUNDARY, "times", new long[] {10L, 20L}); - assertThat(parts).hasSize(4); - assertThat(parts).allSatisfy(part -> assertThat(part).contains(TEXT_PART_HEADER)); - assertThat(payloadsOf(parts, "ids")).containsExactly("1", "2"); - assertThat(payloadsOf(parts, "times")).containsExactly("10", "20"); + assertThat(new String(output.toByteArray(), UTF_8)) + .isEqualTo( + formatDelimitedTextPart("ids", "1") + + formatDelimitedTextPart("ids", "2") + + formatDelimitedTextPart("times", "10") + + formatDelimitedTextPart("times", "20")); } @Test - void arrayOfBooleansIsWrittenAsRepeatedParts() { - List parts = textParts(map("flags", new boolean[] {true, false})); + void primitiveBooleanArrayIsWrittenAsOnePartPerElement() { + Output output = new Output(UTF_8); + + WRITER.write(output, FIXED_BOUNDARY, "flags", new boolean[] {true, false}); - assertThat(parts).hasSize(2); - assertThat(payloadsOf(parts, "flags")).containsExactly("true", "false"); + assertThat(new String(output.toByteArray(), UTF_8)) + .isEqualTo( + formatDelimitedTextPart("flags", "true") + formatDelimitedTextPart("flags", "false")); } @Test - void byteArrayIsWrittenAsOneBinaryPart() { - byte[] payload = {0, 1, 42, (byte) 0x80, (byte) 0xFF}; - - List parts = splitParts(encodeMultipart(map("blob", payload))); + void byteArrayIsWrittenAsOneBinaryPartInsteadOfRepeatedParts() { + String body = + new String( + encodeMultipart(Map.of("blob", new byte[] {0, 1, 42})), ISO_8859_1); - assertThat(parts).hasSize(1); - assertThat(new String(parts.get(0), UTF_8)) - .contains("Content-Disposition: form-data; name=\"blob\"") - .contains("Content-Type: application/octet-stream") - .contains("Content-Transfer-Encoding: binary"); - assertThat(parts.get(0)).endsWith(payload); + assertThat(Pattern.compile("name=\"blob\"").matcher(body).results()).hasSize(1); + assertThat(body).contains("Content-Type: application/octet-stream"); } - private static String expectedPart(String name, String payload) { + private static String formatDelimitedTextPart(String name, String payload) { return "--" - + BOUNDARY + + FIXED_BOUNDARY + "\r\n" + "Content-Disposition: form-data; name=\"" + name + "\"\r\n" - + TEXT_PART_HEADER + + TEXT_PLAIN_CONTENT_TYPE_HEADER + payload + "\r\n"; } - private static Map map(Object... keysAndValues) { - Map data = new LinkedHashMap<>(); - for (int index = 0; index < keysAndValues.length; index += 2) { - data.put((String) keysAndValues[index], keysAndValues[index + 1]); - } - return data; - } - - private static RequestTemplate encodeMultipart(Map data) { + private static byte[] encodeMultipart(Map formFields) { RequestTemplate template = new RequestTemplate(); template.header("Content-Type", "multipart/form-data"); - new FormEncoder().encode(data, Map.class, template); - - return template; - } - - private static List textParts(Map data) { - List parts = new ArrayList<>(); - for (byte[] part : splitParts(encodeMultipart(data))) { - parts.add(new String(part, UTF_8)); - } - return parts; - } - - private static List payloadsOf(List parts, String name) { - List payloads = new ArrayList<>(); - for (String part : parts) { - if (part.contains("name=\"" + name + "\"")) { - payloads.add(part.substring(part.indexOf("\r\n\r\n") + 4)); - } - } - return payloads; - } - - private static List splitParts(RequestTemplate template) { - return splitParts(template.body(), boundaryOf(template)); - } - - /** Reads the boundary the processor announced in the {@code Content-Type} header. */ - private static String boundaryOf(RequestTemplate template) { - String contentType = template.headers().get("Content-Type").iterator().next(); - assertThat(contentType).startsWith("multipart/form-data; charset=UTF-8; boundary="); - return contentType.substring(contentType.indexOf("boundary=") + "boundary=".length()); - } - - private static List splitParts(byte[] body, String boundary) { - byte[] delimiter = ("--" + boundary).getBytes(UTF_8); - - List positions = new ArrayList<>(); - int position = indexOf(body, delimiter, 0); - while (position >= 0) { - positions.add(position); - position = indexOf(body, delimiter, position + delimiter.length); - } - - List parts = new ArrayList<>(); - for (int index = 0; index + 1 < positions.size(); index++) { - int start = positions.get(index) + delimiter.length; - int end = positions.get(index + 1); - if (end - start < 4 || body[start] != '\r' || body[start + 1] != '\n') { - continue; // the closing delimiter, "----", is not a part - } - parts.add(Arrays.copyOfRange(body, start + 2, end - 2)); - } - return parts; - } - - private static int indexOf(byte[] haystack, byte[] needle, int from) { - for (int index = Math.max(from, 0); index <= haystack.length - needle.length; index++) { - if (matchesAt(haystack, needle, index)) { - return index; - } - } - return -1; - } + new FormEncoder().encode(formFields, Map.class, template); - private static boolean matchesAt(byte[] haystack, byte[] needle, int offset) { - for (int index = 0; index < needle.length; index++) { - if (haystack[offset + index] != needle[index]) { - return false; - } - } - return true; + return template.body(); } }