From 89648ae034716a3fb85fb72663117d40b42c8997 Mon Sep 17 00:00:00 2001 From: Eduardo Radieske Date: Sat, 26 Sep 2026 23:26:36 -0300 Subject: [PATCH 1/5] fix(form): correctly handle Map and POJO types - improve POJO detection for parameterized types - prevent Map and collection types from being treated as POJOs - add PojoUtil unit tests - update WildCardMapTest --- .../src/main/java/feign/form/FormEncoder.java | 11 +- .../main/java/feign/form/util/PojoUtil.java | 23 +- .../test/java/feign/form/WildCardMapTest.java | 18 +- .../java/feign/form/utils/PojoUtilTest.java | 231 ++++++++++++++++++ 4 files changed, 271 insertions(+), 12 deletions(-) create mode 100644 form/src/test/java/feign/form/utils/PojoUtilTest.java diff --git a/form/src/main/java/feign/form/FormEncoder.java b/form/src/main/java/feign/form/FormEncoder.java index 19551b0fb0..d031ffd1ba 100644 --- a/form/src/main/java/feign/form/FormEncoder.java +++ b/form/src/main/java/feign/form/FormEncoder.java @@ -104,8 +104,7 @@ public FormEncoder(Encoder delegate) { * alongside it, instead of being swallowed by a fallback of its own. * * 
-   * Feign.builder()
-   *     .encoders(FormEncoder.createPredicatedFormEncoder(), new JacksonEncoder());
+   * Feign.builder().encoders(FormEncoder.createPredicatedFormEncoder(), new JacksonEncoder());
    *
* * @return a form encoder guarded by {@link #formRequests()} @@ -125,7 +124,7 @@ public static EncoderPredicate formRequests() { "Content-Type is a form type and the body is a map or a user pojo", (object, bodyType, template) -> ContentType.of(getContentTypeValue(template.headers())) != ContentType.UNDEFINED - && (object instanceof Map || (bodyType != null && isUserPojo(bodyType)))); + && (isMap(object) || (bodyType != null && isUserPojo(bodyType)))); } @Override @@ -140,7 +139,7 @@ public void encode(Object object, Type bodyType, RequestTemplate template) } Map data; - if (object instanceof Map) { + if (isMap(object)) { data = (Map) object; } else if (isUserPojo(bodyType)) { data = toMap(object); @@ -190,4 +189,8 @@ private Charset getCharset(String contentTypeValue) { return UTF_8; } } + + private static boolean isMap(Object object) { + return object instanceof Map; + } } diff --git a/form/src/main/java/feign/form/util/PojoUtil.java b/form/src/main/java/feign/form/util/PojoUtil.java index 195d176704..192db9bfbe 100644 --- a/form/src/main/java/feign/form/util/PojoUtil.java +++ b/form/src/main/java/feign/form/util/PojoUtil.java @@ -21,6 +21,7 @@ import feign.form.FormProperty; import java.lang.reflect.Field; +import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.rmi.UnexpectedException; import java.security.PrivilegedAction; @@ -41,14 +42,26 @@ public final class PojoUtil { public static boolean isUserPojo(@NonNull Object object) { - val type = object.getClass(); - val packageName = type.getPackage().getName(); - return !packageName.startsWith("java."); + return isUserPojo(object.getClass()); } public static boolean isUserPojo(@NonNull Type type) { - val typeName = type.toString(); - return !typeName.startsWith("class java."); + if (type instanceof Class) { + return isUserPojo((Class) type); + } + + if (type instanceof ParameterizedType) { + ParameterizedType parameterizedType = (ParameterizedType) type; + return isUserPojo(parameterizedType.getRawType()); + } + + return false; + } + + private static boolean isUserPojo(@NonNull Class type) { + val packageName = type.getPackage().getName(); + + return !packageName.startsWith("java."); } @SneakyThrows diff --git a/form/src/test/java/feign/form/WildCardMapTest.java b/form/src/test/java/feign/form/WildCardMapTest.java index 3dca096c8f..e764692814 100644 --- a/form/src/test/java/feign/form/WildCardMapTest.java +++ b/form/src/test/java/feign/form/WildCardMapTest.java @@ -41,12 +41,10 @@ class WildCardMapTest { @BeforeAll static void configureClient() { - var logFile = logDir.resolve("log.txt").toString(); - api = Feign.builder() .encoder(new FormEncoder()) - .logger(new JavaLogger(WildCardMapTest.class).appendToFile(logFile)) + .logger(new JavaLogger(WildCardMapTest.class)) .logLevel(FULL) .target(FormUrlEncodedApi.class, "http://localhost:8080"); } @@ -83,10 +81,24 @@ void testBadRequest() { assertThat(api.wildCardMap(param)).isNotNull().extracting(Response::status).isEqualTo(418); } + @Test + void testMapStringString() { + Map param = new HashMap<>(); + + param.put("key1", "1"); + param.put("key2", "1"); + + assertThat(api.mapStringString(param)).isNotNull().extracting(Response::status).isEqualTo(200); + } + interface FormUrlEncodedApi { @RequestLine("POST /wild-card-map") @Headers("Content-Type: application/x-www-form-urlencoded") Response wildCardMap(Map param); + + @RequestLine("POST /wild-card-map") + @Headers("Content-Type: application/x-www-form-urlencoded") + Response mapStringString(Map param); } } diff --git a/form/src/test/java/feign/form/utils/PojoUtilTest.java b/form/src/test/java/feign/form/utils/PojoUtilTest.java new file mode 100644 index 0000000000..de9e75131c --- /dev/null +++ b/form/src/test/java/feign/form/utils/PojoUtilTest.java @@ -0,0 +1,231 @@ +/* + * 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.utils; + +import static org.assertj.core.api.Assertions.assertThat; + +import feign.form.FormProperty; +import feign.form.util.PojoUtil; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class PojoUtilTest { + + @Test + void shouldIdentifyUserPojoFromObject() { + var pojo = new UserPojo(); + + assertThat(PojoUtil.isUserPojo(pojo)).isTrue(); + } + + @Test + void shouldNotIdentifyJavaObjectAsUserPojo() { + var object = new HashMap(); + + assertThat(PojoUtil.isUserPojo(object)).isFalse(); + } + + @Test + void shouldIdentifyUserPojoFromClassType() { + Type type = UserPojo.class; + + assertThat(PojoUtil.isUserPojo(type)).isTrue(); + } + + @Test + void shouldNotIdentifyJavaClassAsUserPojo() { + Type type = HashMap.class; + + assertThat(PojoUtil.isUserPojo(type)).isFalse(); + } + + @Test + void shouldConvertPojoFieldsToMap() { + var pojo = new UserPojo(); + pojo.name = "Eduardo"; + pojo.age = 30; + + assertThat(PojoUtil.toMap(pojo)) + .containsExactlyInAnyOrderEntriesOf(Map.of("custom_name", "Eduardo", "age", 30)); + } + + @Test + void shouldIgnoreNullFields() { + var pojo = new UserPojo(); + pojo.name = "Eduardo"; + + assertThat(PojoUtil.toMap(pojo)).containsExactly(Map.entry("custom_name", "Eduardo")); + } + + @Test + void shouldIgnoreStaticFields() { + var pojo = new UserPojo(); + pojo.name = "Eduardo"; + + assertThat(PojoUtil.toMap(pojo)).doesNotContainKey("staticField"); + } + + @Test + void shouldIgnoreFinalFields() { + var pojo = new UserPojo(); + pojo.name = "Eduardo"; + + assertThat(PojoUtil.toMap(pojo)).doesNotContainKey("finalField"); + } + + @Test + void shouldUseFormPropertyAsMapKey() { + var pojo = new UserPojo(); + pojo.name = "Eduardo"; + + assertThat(PojoUtil.toMap(pojo)) + .containsEntry("custom_name", "Eduardo") + .doesNotContainKey("name"); + } + + @Test + void shouldReadPrivateFields() { + var pojo = new PrivateFieldsPojo("Eduardo"); + + assertThat(PojoUtil.toMap(pojo)).containsEntry("name", "Eduardo"); + } + + @Test + void shouldNotConvertInheritedFields() { + var pojo = new ChildPojo(); + pojo.child = "child"; + + assertThat(PojoUtil.toMap(pojo)).containsEntry("child", "child").doesNotContainKey("parent"); + } + + @Test + void shouldReturnEmptyMapWhenPojoHasNoEligibleFields() { + var pojo = new EmptyPojo(); + + assertThat(PojoUtil.toMap(pojo)).isEmpty(); + } + + @Test + void shouldNotIdentifyParameterizedMapAsUserPojo() { + Type type = new TypeReference>() {}.getType(); + + assertThat(PojoUtil.isUserPojo(type)).isFalse(); + } + + @Test + void shouldNotIdentifyParameterizedListAsUserPojo() { + Type type = new TypeReference>() {}.getType(); + + assertThat(PojoUtil.isUserPojo(type)).isFalse(); + } + + @Test + void shouldNotIdentifyParameterizedHashMapAsUserPojo() { + Type type = new TypeReference>() {}.getType(); + + assertThat(PojoUtil.isUserPojo(type)).isFalse(); + } + + @Test + void shouldNotIdentifyParameterizedCollectionAsUserPojo() { + Type type = new TypeReference>() {}.getType(); + + assertThat(PojoUtil.isUserPojo(type)).isFalse(); + } + + @Test + void shouldIdentifyParameterizedUserPojoAsUserPojo() { + Type type = new TypeReference>() {}.getType(); + + assertThat(PojoUtil.isUserPojo(type)).isTrue(); + } + + @Test + void shouldNotIdentifyTypeVariableAsUserPojo() { + Type type = UserPojo.class.getTypeParameters()[0]; + + assertThat(PojoUtil.isUserPojo(type)).isFalse(); + } + + @Test + void shouldNotIdentifyWildcardTypeAsUserPojo() { + Type type = new TypeReference>() {}.getType(); + + Type wildcard = ((ParameterizedType) type).getActualTypeArguments()[0]; + + assertThat(PojoUtil.isUserPojo(wildcard)).isFalse(); + } + + static class UserPojo { + + @FormProperty("custom_name") + private String name; + + private Integer age; + + private T value; + + private static String staticField; + + private final String finalField = "ignored"; + } + + private static class PrivateFieldsPojo { + + private String name; + + private PrivateFieldsPojo(String name) { + this.name = name; + } + } + + static class ParentPojo { + + private String parent; + } + + static class ChildPojo extends ParentPojo { + + private String child; + } + + static class EmptyPojo { + + private static final String STATIC = "ignored"; + + private final String FINAL = "ignored"; + } + + private abstract static class TypeReference { + + private final Type type; + + protected TypeReference() { + type = + ((java.lang.reflect.ParameterizedType) getClass().getGenericSuperclass()) + .getActualTypeArguments()[0]; + } + + Type getType() { + return type; + } + } +} From 252fe6b1334bb394d390d03f2f96f823e1c9e5e9 Mon Sep 17 00:00:00 2001 From: Eduardo Radieske Date: Sat, 3 Oct 2026 13:00:44 -0300 Subject: [PATCH 2/5] fix(form): handle primitive and array types in PojoUtil --- form/src/main/java/feign/form/util/PojoUtil.java | 8 ++++++-- .../test/java/feign/form/utils/PojoUtilTest.java | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/form/src/main/java/feign/form/util/PojoUtil.java b/form/src/main/java/feign/form/util/PojoUtil.java index 192db9bfbe..d805946a94 100644 --- a/form/src/main/java/feign/form/util/PojoUtil.java +++ b/form/src/main/java/feign/form/util/PojoUtil.java @@ -59,9 +59,13 @@ public static boolean isUserPojo(@NonNull Type type) { } private static boolean isUserPojo(@NonNull Class type) { - val packageName = type.getPackage().getName(); + if (type.isPrimitive() || type.isArray()) { + return false; + } - return !packageName.startsWith("java."); + Package pkg = type.getPackage(); + + return pkg != null && !pkg.getName().startsWith("java."); } @SneakyThrows diff --git a/form/src/test/java/feign/form/utils/PojoUtilTest.java b/form/src/test/java/feign/form/utils/PojoUtilTest.java index de9e75131c..bc6bfd2f1f 100644 --- a/form/src/test/java/feign/form/utils/PojoUtilTest.java +++ b/form/src/test/java/feign/form/utils/PojoUtilTest.java @@ -173,6 +173,21 @@ void shouldNotIdentifyWildcardTypeAsUserPojo() { assertThat(PojoUtil.isUserPojo(wildcard)).isFalse(); } + + @Test + void shouldNotIdentifyPrimitiveAsUserPojo() { + assertThat(PojoUtil.isUserPojo(int.class)).isFalse(); + } + + @Test + void shouldNotIdentifyArrayAsUserPojo() { + assertThat(PojoUtil.isUserPojo(byte[].class)).isFalse(); + } + + @Test + void shouldNotIdentifyObjectArrayAsUserPojo() { + assertThat(PojoUtil.isUserPojo(String[].class)).isFalse(); + } static class UserPojo { From c2a0a41c9134a703502219529212dd8603e051c6 Mon Sep 17 00:00:00 2001 From: Eduardo Radieske Date: Sat, 3 Oct 2026 13:39:56 -0300 Subject: [PATCH 3/5] test(form): verify JDK types are not treated as POJOs --- .../test/java/feign/form/WildCardMapTest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/form/src/test/java/feign/form/WildCardMapTest.java b/form/src/test/java/feign/form/WildCardMapTest.java index e764692814..60688cc5f9 100644 --- a/form/src/test/java/feign/form/WildCardMapTest.java +++ b/form/src/test/java/feign/form/WildCardMapTest.java @@ -17,6 +17,7 @@ import static feign.Logger.Level.FULL; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.DEFINED_PORT; import feign.Feign; @@ -24,8 +25,12 @@ import feign.Logger.JavaLogger; import feign.RequestLine; import feign.Response; +import feign.codec.EncodeException; + import java.nio.file.Path; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -90,6 +95,17 @@ void testMapStringString() { assertThat(api.mapStringString(param)).isNotNull().extracting(Response::status).isEqualTo(200); } + + @Test + void testListIsDelegatedToDefaultEncoder() { + List param = new ArrayList<>(); + param.add("key1"); + param.add("key2"); + + assertThatThrownBy(() -> api.list(param)) + .isInstanceOf(EncodeException.class) + .hasMessageContaining("ArrayList is not a type supported by this encoder."); + } interface FormUrlEncodedApi { @@ -100,5 +116,9 @@ interface FormUrlEncodedApi { @RequestLine("POST /wild-card-map") @Headers("Content-Type: application/x-www-form-urlencoded") Response mapStringString(Map param); + + @RequestLine("POST /wild-card-map") + @Headers("Content-Type: application/x-www-form-urlencoded") + Response list(List param); } } From 2ea2579c3a3717b10e6aabdfe27c3ab4d860f11f Mon Sep 17 00:00:00 2001 From: Eduardo Radieske Date: Sat, 3 Oct 2026 13:51:51 -0300 Subject: [PATCH 4/5] style(form): address review feedback --- form/src/main/java/feign/form/FormEncoder.java | 11 ++++------- form/src/test/java/feign/form/WildCardMapTest.java | 6 +----- .../java/feign/form/{utils => util}/PojoUtilTest.java | 4 ++-- 3 files changed, 7 insertions(+), 14 deletions(-) rename form/src/test/java/feign/form/{utils => util}/PojoUtilTest.java (98%) diff --git a/form/src/main/java/feign/form/FormEncoder.java b/form/src/main/java/feign/form/FormEncoder.java index d031ffd1ba..19551b0fb0 100644 --- a/form/src/main/java/feign/form/FormEncoder.java +++ b/form/src/main/java/feign/form/FormEncoder.java @@ -104,7 +104,8 @@ public FormEncoder(Encoder delegate) { * alongside it, instead of being swallowed by a fallback of its own. * * 
-   * Feign.builder().encoders(FormEncoder.createPredicatedFormEncoder(), new JacksonEncoder());
+   * Feign.builder()
+   *     .encoders(FormEncoder.createPredicatedFormEncoder(), new JacksonEncoder());
    *
* * @return a form encoder guarded by {@link #formRequests()} @@ -124,7 +125,7 @@ public static EncoderPredicate formRequests() { "Content-Type is a form type and the body is a map or a user pojo", (object, bodyType, template) -> ContentType.of(getContentTypeValue(template.headers())) != ContentType.UNDEFINED - && (isMap(object) || (bodyType != null && isUserPojo(bodyType)))); + && (object instanceof Map || (bodyType != null && isUserPojo(bodyType)))); } @Override @@ -139,7 +140,7 @@ public void encode(Object object, Type bodyType, RequestTemplate template) } Map data; - if (isMap(object)) { + if (object instanceof Map) { data = (Map) object; } else if (isUserPojo(bodyType)) { data = toMap(object); @@ -189,8 +190,4 @@ private Charset getCharset(String contentTypeValue) { return UTF_8; } } - - private static boolean isMap(Object object) { - return object instanceof Map; - } } diff --git a/form/src/test/java/feign/form/WildCardMapTest.java b/form/src/test/java/feign/form/WildCardMapTest.java index 60688cc5f9..b792aa987e 100644 --- a/form/src/test/java/feign/form/WildCardMapTest.java +++ b/form/src/test/java/feign/form/WildCardMapTest.java @@ -27,14 +27,12 @@ import feign.Response; import feign.codec.EncodeException; -import java.nio.file.Path; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; import org.springframework.boot.test.context.SpringBootTest; @SpringBootTest(webEnvironment = DEFINED_PORT, classes = Server.class) @@ -42,11 +40,9 @@ class WildCardMapTest { private static FormUrlEncodedApi api; - @TempDir static Path logDir; - @BeforeAll static void configureClient() { - api = + api = Feign.builder() .encoder(new FormEncoder()) .logger(new JavaLogger(WildCardMapTest.class)) diff --git a/form/src/test/java/feign/form/utils/PojoUtilTest.java b/form/src/test/java/feign/form/util/PojoUtilTest.java similarity index 98% rename from form/src/test/java/feign/form/utils/PojoUtilTest.java rename to form/src/test/java/feign/form/util/PojoUtilTest.java index bc6bfd2f1f..e0617f506e 100644 --- a/form/src/test/java/feign/form/utils/PojoUtilTest.java +++ b/form/src/test/java/feign/form/util/PojoUtilTest.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package feign.form.utils; +package feign.form.util; import static org.assertj.core.api.Assertions.assertThat; @@ -235,7 +235,7 @@ private abstract static class TypeReference { protected TypeReference() { type = - ((java.lang.reflect.ParameterizedType) getClass().getGenericSuperclass()) + ((ParameterizedType) getClass().getGenericSuperclass()) .getActualTypeArguments()[0]; } From e44cfeb00a079f9cbe32f85e85e3a476cea27a45 Mon Sep 17 00:00:00 2001 From: Eduardo Radieske Date: Sat, 3 Oct 2026 13:59:54 -0300 Subject: [PATCH 5/5] style(form): apply code formatting --- form/src/main/java/feign/form/util/PojoUtil.java | 8 ++++---- form/src/test/java/feign/form/WildCardMapTest.java | 7 +++---- form/src/test/java/feign/form/util/PojoUtilTest.java | 11 ++++------- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/form/src/main/java/feign/form/util/PojoUtil.java b/form/src/main/java/feign/form/util/PojoUtil.java index d805946a94..4167287be8 100644 --- a/form/src/main/java/feign/form/util/PojoUtil.java +++ b/form/src/main/java/feign/form/util/PojoUtil.java @@ -59,12 +59,12 @@ public static boolean isUserPojo(@NonNull Type type) { } private static boolean isUserPojo(@NonNull Class type) { - if (type.isPrimitive() || type.isArray()) { + if (type.isPrimitive() || type.isArray()) { return false; - } + } + + Package pkg = type.getPackage(); - Package pkg = type.getPackage(); - return pkg != null && !pkg.getName().startsWith("java."); } diff --git a/form/src/test/java/feign/form/WildCardMapTest.java b/form/src/test/java/feign/form/WildCardMapTest.java index b792aa987e..1024d56b90 100644 --- a/form/src/test/java/feign/form/WildCardMapTest.java +++ b/form/src/test/java/feign/form/WildCardMapTest.java @@ -26,7 +26,6 @@ import feign.RequestLine; import feign.Response; import feign.codec.EncodeException; - import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -42,7 +41,7 @@ class WildCardMapTest { @BeforeAll static void configureClient() { - api = + api = Feign.builder() .encoder(new FormEncoder()) .logger(new JavaLogger(WildCardMapTest.class)) @@ -91,7 +90,7 @@ void testMapStringString() { assertThat(api.mapStringString(param)).isNotNull().extracting(Response::status).isEqualTo(200); } - + @Test void testListIsDelegatedToDefaultEncoder() { List param = new ArrayList<>(); @@ -112,7 +111,7 @@ interface FormUrlEncodedApi { @RequestLine("POST /wild-card-map") @Headers("Content-Type: application/x-www-form-urlencoded") Response mapStringString(Map param); - + @RequestLine("POST /wild-card-map") @Headers("Content-Type: application/x-www-form-urlencoded") Response list(List param); diff --git a/form/src/test/java/feign/form/util/PojoUtilTest.java b/form/src/test/java/feign/form/util/PojoUtilTest.java index e0617f506e..7994a945ae 100644 --- a/form/src/test/java/feign/form/util/PojoUtilTest.java +++ b/form/src/test/java/feign/form/util/PojoUtilTest.java @@ -18,7 +18,6 @@ import static org.assertj.core.api.Assertions.assertThat; import feign.form.FormProperty; -import feign.form.util.PojoUtil; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.util.Collection; @@ -173,17 +172,17 @@ void shouldNotIdentifyWildcardTypeAsUserPojo() { assertThat(PojoUtil.isUserPojo(wildcard)).isFalse(); } - + @Test void shouldNotIdentifyPrimitiveAsUserPojo() { assertThat(PojoUtil.isUserPojo(int.class)).isFalse(); } - + @Test void shouldNotIdentifyArrayAsUserPojo() { assertThat(PojoUtil.isUserPojo(byte[].class)).isFalse(); } - + @Test void shouldNotIdentifyObjectArrayAsUserPojo() { assertThat(PojoUtil.isUserPojo(String[].class)).isFalse(); @@ -234,9 +233,7 @@ private abstract static class TypeReference { private final Type type; protected TypeReference() { - type = - ((ParameterizedType) getClass().getGenericSuperclass()) - .getActualTypeArguments()[0]; + type = ((ParameterizedType) getClass().getGenericSuperclass()).getActualTypeArguments()[0]; } Type getType() {