Don't modify input map in UriTemplate (#8364)

Fixes #8338
micronaut-projects · Nov 18, 2022 · 0717eb9 · 0717eb9
1 parent fab0008
commit 0717eb9
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 4 deletions.
diff --git a/http-client/src/test/groovy/io/micronaut/http/client/aop/QueryParametersSpec.groovy b/http-client/src/test/groovy/io/micronaut/http/client/aop/QueryParametersSpec.groovy
@@ -27,8 +27,8 @@ import io.micronaut.http.client.HttpClient
 import io.micronaut.http.client.annotation.Client
 import io.micronaut.http.client.exceptions.HttpClientResponseException
 import io.micronaut.runtime.server.EmbeddedServer
-import reactor.core.publisher.Flux
 import spock.lang.AutoCleanup
+import spock.lang.Issue
 import spock.lang.Shared
 import spock.lang.Specification
 import spock.lang.Unroll
@@ -62,6 +62,18 @@ class QueryParametersSpec extends Specification {
         flavour << [ "pojo", "singlePojo", "list", "map" ]
     }
 
+    @Issue('https://github.com/micronaut-projects/micronaut-core/issues/8338')
+    void "test client mappping URL parameters appended through a Map does not modify the Map"() {
+        when:
+        // this modification is relatively benign, but if the user passed a Map.of, then trying to remove null leads to
+        // an exception. Unfortunately we can't test with Map.of.
+        def map = [term: "Riverside", foo: null]
+        def result = client.searchExplodedMap("map", map)
+        then:
+        result.albums.size() == 2
+        map.containsValue(null)
+    }
+
     @Unroll
     void "test client mappping multiple URL parameters appended through a Map (served through #flavour)"() {
         expect:

diff --git a/http/src/main/java/io/micronaut/http/uri/UriTemplate.java b/http/src/main/java/io/micronaut/http/uri/UriTemplate.java
@@ -27,7 +27,6 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.StringJoiner;
 import java.util.function.Predicate;
@@ -995,7 +994,6 @@ public String expand(Map<String, Object> parameters, boolean previousHasContent,
                         result = joiner.toString();
                     } else if (found instanceof Map) {
                         Map<Object, Object> map = (Map<Object, Object>) found;
-                        map.values().removeIf(Objects::isNull);
                         if (map.isEmpty()) {
                             return "";
                         }
@@ -1020,6 +1018,9 @@ public String expand(Map<String, Object> parameters, boolean previousHasContent,
                         }
 
                         map.forEach((key, some) -> {
+                            if (some == null) {
+                                return;
+                            }
                             String ks = key.toString();
                             Iterable<?> values = (some instanceof Iterable) ? (Iterable) some : Collections.singletonList(some);
                             for (Object value: values) {
@@ -1038,7 +1039,12 @@ public String expand(Map<String, Object> parameters, boolean previousHasContent,
                                 }
                             }
                         });
-                        result = joiner.toString();
+                        if (joiner.length() == 0) {
+                            // only null entries
+                            return "";
+                        } else {
+                            result = joiner.toString();
+                        }
                     } else {
                         String str = found.toString();
                         str = applyModifier(modifierStr, modifierChar, str, str.length());