Handle primitive array form values without reference-array casts

CHANGELOG.md

@@ -4,6 +4,10 @@
   `NullPointerException` from `body()`, `charset()`, `isBinary()` and `length()`. A request with no
   body reports a `null` body and charset, is binary, and has length 0. Its `toString()` prints the
   request line and headers without a body (#1210).
+* 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
 

form/src/main/java/feign/form/UrlencodedFormContentProcessor.java

@@ -16,20 +16,21 @@
 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.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.Stream;
+import java.util.stream.StreamSupport;
 import lombok.SneakyThrows;
 import lombok.val;
 
@@ -89,12 +90,12 @@ private CharSequence createKeyValuePair(
 
     if (value == null) {
       return encodedKey;
-    } else if (value.getClass().isArray()) {
+    } else if (value.getClass().isArray() || value instanceof Collection) {
       return createKeyValuePair(
-          collectionFormat, encodedKey, Arrays.stream((Object[]) value), charset);
-    } else if (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)

form/src/main/java/feign/form/multipart/ManyParametersWriter.java

@@ -15,6 +15,7 @@
  */
 package feign.form.multipart;
 
+import static feign.form.util.Elements.elementsOf;
 import static lombok.AccessLevel.PRIVATE;
 
 import feign.codec.EncodeException;
@@ -33,31 +34,15 @@ 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]);
-    }
-    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()) {
-      val objects = (Object[]) value;
-      for (val object : objects) {
-        parameterWriter.write(output, boundary, key, object);
-      }
-    } 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);
     }
   }
 }

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<Object> elements = new ArrayList<>(length);
+    for (int index = 0; index < length; index++) {
+      elements.add(Array.get(value, index));
+    }
+    return elements;
+  }
+}

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<Arguments> 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,

form/src/test/java/feign/form/multipart/ManyParametersWriterTest.java

@@ -0,0 +1,140 @@
+/*
+ * 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.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.List;
+import java.util.Map;
+import java.util.regex.Pattern;
+import org.junit.jupiter.api.Test;
+
+class ManyParametersWriterTest {
+
+  private static final ManyParametersWriter WRITER = new ManyParametersWriter();
+
+  private static final String FIXED_BOUNDARY = "boundary";
+
+  private static final String TEXT_PLAIN_CONTENT_TYPE_HEADER =
+      "Content-Type: text/plain; charset=UTF-8\r\n\r\n";
+
+  @Test
+  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();
+  }
+
+  @Test
+  void byteArrayIsApplicableBecauseByteIsANumber() {
+    assertThat(WRITER.isApplicable(new byte[] {1, 2})).isTrue();
+  }
+
+  @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 scalarAndIterableOfFilesAreNotApplicable() {
+    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, FIXED_BOUNDARY, "tags", List.of("one", "two"));
+
+    assertThat(new String(output.toByteArray(), UTF_8))
+        .isEqualTo(formatDelimitedTextPart("tags", "one") + formatDelimitedTextPart("tags", "two"));
+  }
+
+  @Test
+  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(new String(output.toByteArray(), UTF_8))
+        .isEqualTo(
+            formatDelimitedTextPart("ids", "1")
+                + formatDelimitedTextPart("ids", "2")
+                + formatDelimitedTextPart("times", "10")
+                + formatDelimitedTextPart("times", "20"));
+  }
+
+  @Test
+  void primitiveBooleanArrayIsWrittenAsOnePartPerElement() {
+    Output output = new Output(UTF_8);
+
+    WRITER.write(output, FIXED_BOUNDARY, "flags", new boolean[] {true, false});
+
+    assertThat(new String(output.toByteArray(), UTF_8))
+        .isEqualTo(
+            formatDelimitedTextPart("flags", "true") + formatDelimitedTextPart("flags", "false"));
+  }
+
+  @Test
+  void byteArrayIsWrittenAsOneBinaryPartInsteadOfRepeatedParts() {
+    String body =
+        new String(
+            encodeMultipart(Map.<String, Object>of("blob", new byte[] {0, 1, 42})), ISO_8859_1);
+
+    assertThat(Pattern.compile("name=\"blob\"").matcher(body).results()).hasSize(1);
+    assertThat(body).contains("Content-Type: application/octet-stream");
+  }
+
+  private static String formatDelimitedTextPart(String name, String payload) {
+    return "--"
+        + FIXED_BOUNDARY
+        + "\r\n"
+        + "Content-Disposition: form-data; name=\""
+        + name
+        + "\"\r\n"
+        + TEXT_PLAIN_CONTENT_TYPE_HEADER
+        + payload
+        + "\r\n";
+  }
+
+  private static byte[] encodeMultipart(Map<String, Object> formFields) {
+    RequestTemplate template = new RequestTemplate();
+    template.header("Content-Type", "multipart/form-data");
+
+    new FormEncoder().encode(formFields, Map.class, template);
+
+    return template.body();
+  }
+}