Handle parsing of form attributes with no value (#10601)

* Handle parsing of form attributes with no value

MicronautHttpData.AttributeImpl is updated to provide an implementation of the setValue method.
This implementation is needed in the corner cases where Netty's HttpPostStandardRequestDecoder
successfully parses an attribute with no value such as in the body "a&b=2".

Tests are added to more thoroughly test the various forms of x-www-form-urlencoded bodies as
specified by https://url.spec.whatwg.org/#application/x-www-form-urlencoded. These tests
include scenarios where we know and expect that HttpPostStandardRequestDecoder will currently
fail to parse an attribute with a name but no value - namely when that attribute is the last
one in a given POST body. If HttpPostStandardRequestDecoder is patched in the future, then
these expected parsing failures can be moved into the success scenario instead.

This partially resolves #10446.

* Use CharSequence version of Unpooled.copiedBuffer

* use BlockingHttpClient

---------

Co-authored-by: Sergio del Amo <sergio.delamo@softamo.com>

http-server-netty/src/main/java/io/micronaut/http/server/netty/MicronautHttpData.java

@@ -513,7 +513,7 @@ public String getValue() throws IOException {
 
         @Override
         public void setValue(String value) throws IOException {
-            throw new UnsupportedOperationException();
+            setContent(Unpooled.copiedBuffer(value, factory.characterEncoding));
         }
 
         @Override

http-server-netty/src/test/groovy/io/micronaut/http/server/netty/AbstractMicronautSpec.groovy

@@ -16,6 +16,7 @@
 package io.micronaut.http.server.netty
 
 import io.micronaut.context.ApplicationContext
+import io.micronaut.http.client.BlockingHttpClient
 import io.micronaut.http.client.HttpClient
 import io.micronaut.runtime.server.EmbeddedServer
 import spock.lang.AutoCleanup
@@ -37,8 +38,8 @@ abstract class AbstractMicronautSpec extends Specification {
     @Shared int serverPort = embeddedServer.getPort()
     @Shared URL server = embeddedServer.getURL()
     @Shared @AutoCleanup ApplicationContext applicationContext = embeddedServer.applicationContext
-    @Shared @AutoCleanup HttpClient rxClient = applicationContext.createBean(HttpClient, server)
-
+    @Shared @AutoCleanup HttpClient httpClient = applicationContext.createBean(HttpClient, server)
+    @Shared @AutoCleanup BlockingHttpClient client = httpClient.toBlocking()
 
     Collection<String> configurationNames() {
         ['io.micronaut.configuration.jackson','io.micronaut.web.router']

http-server-netty/src/test/groovy/io/micronaut/http/server/netty/RequestCertificateSpec.groovy

@@ -20,7 +20,7 @@ class RequestCertificateSpec extends AbstractMicronautSpec {
 
     void "test certificate extraction"() {
         when:
-        def response = Flux.from(rxClient
+        def response = Flux.from(httpClient
                 .exchange('/ssl', String))
                 .blockFirst()
         then:

http-server-netty/src/test/groovy/io/micronaut/http/server/netty/binding/CookieBindingSpec.groovy

@@ -22,7 +22,6 @@ import io.micronaut.http.annotation.Get
 import io.micronaut.http.client.annotation.Client
 import io.micronaut.http.cookie.Cookies
 import io.micronaut.http.server.netty.AbstractMicronautSpec
-import reactor.core.publisher.Flux
 import spock.lang.Unroll
 
 /**
@@ -37,7 +36,7 @@ class CookieBindingSpec extends AbstractMicronautSpec {
         for (header in headers) {
             request = request.header(header.key, header.value)
         }
-        rxClient.toBlocking().retrieve(request) == result
+        httpClient.toBlocking().retrieve(request) == result
 
         where:
         uri                | result              | headers

http-server-netty/src/test/groovy/io/micronaut/http/server/netty/binding/CustomParameterBindingSpec.groovy

@@ -20,7 +20,7 @@ class CustomParameterBindingSpec extends AbstractMicronautSpec {
     void "test bind HTTP parameters for URI #httpMethod #uri"() {
         given:
         HttpRequest<?> req = HttpRequest.create(HttpMethod.CUSTOM, uri, httpMethod)
-        Publisher<HttpResponse<?>> exchange = rxClient.exchange(req, String)
+        Publisher<HttpResponse<?>> exchange = httpClient.exchange(req, String)
         HttpResponse<?> response = Flux.from(exchange).onErrorResume(t -> {
             if (t instanceof HttpClientResponseException) {
                 return Flux.just(((HttpClientResponseException) t).response)
@@ -74,7 +74,7 @@ class CustomParameterBindingSpec extends AbstractMicronautSpec {
 
     void "test exploded with no default constructor"() {
         when:
-        Flux<HttpResponse<String>> exchange = rxClient.exchange(HttpRequest.create(HttpMethod.CUSTOM, "/parameter/exploded?title=The%20Stand", "REPORT"), String)
+        Flux<HttpResponse<String>> exchange = httpClient.exchange(HttpRequest.create(HttpMethod.CUSTOM, "/parameter/exploded?title=The%20Stand", "REPORT"), String)
         HttpResponse<String> response = exchange.blockFirst()
 
         then:

http-server-netty/src/test/groovy/io/micronaut/http/server/netty/binding/DefaultJsonErrorHandlingSpec.groovy

@@ -16,7 +16,7 @@ class DefaultJsonErrorHandlingSpec extends AbstractMicronautSpec {
 
         when:
         def json = '{"title":"The Stand"'
-        Flux.from(rxClient.exchange(
+        Flux.from(httpClient.exchange(
                 HttpRequest.POST('/errors/map', json), String
         )).blockFirst()
 

http-server-netty/src/test/groovy/io/micronaut/http/server/netty/binding/FormDataBindingSpec.groovy

@@ -31,6 +31,7 @@ import io.micronaut.http.server.netty.AbstractMicronautSpec
 import org.reactivestreams.Publisher
 import reactor.core.publisher.Flux
 import spock.lang.Issue
+import spock.lang.Unroll
 
 /**
  * @author Graeme Rocher
@@ -40,10 +41,10 @@ class FormDataBindingSpec extends AbstractMicronautSpec {
 
     void "test simple string-based body parsing"() {
         when:
-        HttpResponse<?> response = Flux.from(rxClient.exchange(HttpRequest.POST('/form/simple', [
+        HttpResponse<?> response = client.exchange(HttpRequest.POST('/form/simple', [
                 name:"Fred",
                 age:"10"
-        ]).contentType(MediaType.APPLICATION_FORM_URLENCODED_TYPE), String)).blockFirst()
+        ]).contentType(MediaType.APPLICATION_FORM_URLENCODED_TYPE), String)
 
         then:
         response.status == HttpStatus.OK
@@ -53,11 +54,11 @@ class FormDataBindingSpec extends AbstractMicronautSpec {
 
     void "test pojo body parsing"() {
         when:
-        HttpResponse<?> response = Flux.from(rxClient.exchange(HttpRequest.POST('/form/pojo', [
+        HttpResponse<?> response = client.exchange(HttpRequest.POST('/form/pojo', [
                 name:"Fred",
                 age:"10",
                 something: "else"
-        ]).contentType(MediaType.APPLICATION_FORM_URLENCODED_TYPE), String)).blockFirst()
+        ]).contentType(MediaType.APPLICATION_FORM_URLENCODED_TYPE), String)
 
         then:
         response.status == HttpStatus.OK
@@ -67,15 +68,121 @@ class FormDataBindingSpec extends AbstractMicronautSpec {
 
     void "test simple string-based body parsing with missing data"() {
         when:
-        Flux.from(rxClient.exchange(HttpRequest.POST('/form/simple', [
+        client.exchange(HttpRequest.POST('/form/simple', [
                 name:"Fred"
-        ]).contentType(MediaType.APPLICATION_FORM_URLENCODED_TYPE), String)).blockFirst()
+        ]).contentType(MediaType.APPLICATION_FORM_URLENCODED_TYPE), String)
 
         then:
         HttpClientResponseException e = thrown()
         e.response.status == HttpStatus.BAD_REQUEST
     }
 
+    @Issue("https://github.com/micronaut-projects/micronaut-core/issues/10446")
+    @Unroll
+    void "test application/x-www-form-urlencoded String #body is parsed to #parsedMapString"() {
+        when:
+        String result = client.exchange(HttpRequest.POST('/form/map', body)
+                .contentType(MediaType.APPLICATION_FORM_URLENCODED_TYPE), String).getBody(String.class).get()
+
+        then:
+        result == parsedMapString
+
+        where:
+        body            | parsedMapString
+        "a=1"           | "[a:1]"
+        "a=1&b=2"       | "[a:1, b:2]"
+
+        //nothing
+        "&a=1&b=2"      | "[a:1, b:2]"
+        "a=1&&b=2"      | "[a:1, b:2]"
+        "a=1&b=2&"      | "[a:1, b:2]"
+
+        //key equals empty value
+        "z=&a=1&b=2"    | "[z:, a:1, b:2]"
+        "a=1&z=&b=2"    | "[a:1, z:, b:2]"
+        "a=1&b=2&z="    | "[a:1, b:2, z:]"
+
+        //empty key equals value
+        "=0&a=1&b=2"    | "[:0, a:1, b:2]"
+        "a=1&=0&b=2"    | "[a:1, :0, b:2]"
+        "a=1&b=2&=0"    | "[a:1, b:2, :0]"
+
+        //empty key equals empty value
+        "=&a=1&b=2"     | "[:, a:1, b:2]"
+        "a=1&=&b=2"     | "[a:1, :, b:2]"
+        "a=1&b=2&="     | "[a:1, b:2, :]"
+
+        //just key
+        "z&a=1&b=2"     | "[z:, a:1, b:2]"
+        "a=1&z&b=2"     | "[a:1, z:, b:2]"
+    }
+
+    @Issue("https://github.com/micronaut-projects/micronaut-core/issues/10446")
+    @Unroll
+    void "test application/x-www-form-urlencoded String #body with single key fails to parse"() {
+        // NOTE - Parsing is expected to fail here unless HttpPostStandardRequestDecoder is corrected
+        when:
+        String result = client.exchange(HttpRequest.POST('/form/map', body)
+                .contentType(MediaType.APPLICATION_FORM_URLENCODED_TYPE), String).getBody(String.class).get()
+
+        then:
+        def ex = thrown(HttpClientResponseException)
+        ex.status == HttpStatus.BAD_REQUEST
+
+        where:
+        body            | parsedMapString
+        //just key
+        "a"             | "[a:]"
+        "&a"            | "[a:]"
+    }
+
+    @Issue("https://github.com/micronaut-projects/micronaut-core/issues/10446")
+    @Unroll
+    void "test application/x-www-form-urlencoded String #body with single key as the last attribute fails to parse to #parsedMapString"() {
+        // NOTE - Parsing is expected to fail here unless HttpPostStandardRequestDecoder is corrected
+        when:
+        String result = client.exchange(HttpRequest.POST('/form/map', body)
+                .contentType(MediaType.APPLICATION_FORM_URLENCODED_TYPE), String).getBody(String.class).get()
+
+        then:
+        result != parsedMapString
+
+        where:
+        body            | parsedMapString
+        //just key
+        "a=1&b=2&z"     | "[a:1, b:2, z:]"
+    }
+
+    @Issue("https://github.com/micronaut-projects/micronaut-core/issues/10446")
+    @Unroll
+    void "test application/x-www-form-urlencoded Map #body is parsed to #parsedMapString"() {
+        //NOTE - Netty does not allow setting an http data attribute with an empty string key, which seems reasonable, thus fewer
+        // scenarios are exercised than the above that uses a raw String body. The key with empty value works here because it is
+        // sent as key= instead of just key, and Netty parses that correctly on the server end.
+        when:
+        String result = client.exchange(HttpRequest.POST('/form/map', body)
+                .contentType(MediaType.APPLICATION_FORM_URLENCODED_TYPE), String).getBody(String.class).get()
+
+        then:
+        result == parsedMapString
+
+        where:
+        body                    | parsedMapString
+        [a:"1"]                 | "[a:1]"
+        [a:"1", b:"2"]          | "[a:1, b:2]"
+
+        //key equals empty value
+        [z:"",a:"1",b:"2"]      | "[z:, a:1, b:2]"
+        [a:"1",z:"",b:"2"]      | "[a:1, z:, b:2]"
+        [a:"1",b:"2",z:""]      | "[a:1, b:2, z:]"
+
+        //just key
+        [z:"",a:"1",b:"2"]      | "[z:, a:1, b:2]"
+        [a:"1",z:"",b:"2"]      | "[a:1, z:, b:2]"
+        [a:""]                  | "[a:]"
+        [a:"1",b:"2",z:""]      | "[a:1, b:2, z:]"
+    }
+
     @Issue('https://github.com/micronaut-projects/micronaut-core/issues/1032')
     void "test POST SAML form url encoded"() {
         given:
@@ -89,8 +196,8 @@ class FormDataBindingSpec extends AbstractMicronautSpec {
     void "test POST SAML form multipart form data"() {
         given:
         MultipartBody body = MultipartBody.builder().addPart("SAMLResponse", SAML_DATA).build()
-        String data = Flux.from(rxClient.retrieve(HttpRequest.POST("/form/saml/test/form-data", body)
-                .contentType(MediaType.MULTIPART_FORM_DATA_TYPE), String)).blockFirst()
+        String data = client.retrieve(HttpRequest.POST("/form/saml/test/form-data", body)
+                .contentType(MediaType.MULTIPART_FORM_DATA_TYPE), String)
 
         expect:
         data == SAML_DATA
@@ -131,10 +238,10 @@ class FormDataBindingSpec extends AbstractMicronautSpec {
     @Issue("https://github.com/micronaut-projects/micronaut-core/issues/2263")
     void "test binding directly to a string"() {
         when:
-        HttpResponse<String> response = Flux.from(rxClient.exchange(HttpRequest.POST('/form/string', [
+        HttpResponse<String> response = client.exchange(HttpRequest.POST('/form/string', [
                 name:"Fred",
                 age:"10"
-        ]).contentType(MediaType.APPLICATION_FORM_URLENCODED_TYPE), String)).blockFirst()
+        ]).contentType(MediaType.APPLICATION_FORM_URLENCODED_TYPE), String)
 
         then:
         response.status == HttpStatus.OK
@@ -144,10 +251,10 @@ class FormDataBindingSpec extends AbstractMicronautSpec {
 
     void "test binding directly to a reactive string"() {
         when:
-        HttpResponse<String> response = Flux.from(rxClient.exchange(HttpRequest.POST('/form/maybe-string', [
+        HttpResponse<String> response = client.exchange(HttpRequest.POST('/form/maybe-string', [
                 name:"Fred",
                 age:"10"
-        ]).contentType(MediaType.APPLICATION_FORM_URLENCODED_TYPE), String)).blockFirst()
+        ]).contentType(MediaType.APPLICATION_FORM_URLENCODED_TYPE), String)
 
         then:
         response.status == HttpStatus.OK
@@ -179,6 +286,11 @@ class FormDataBindingSpec extends AbstractMicronautSpec {
             "name: $person.name, age: $person.age"
         }
 
+        @Post('/map')
+        String map(@Body Map<String, String> formData) {
+            formData.toMapString()
+        }
+
         @EqualsAndHashCode
         static class Person {
             String name

http-server-netty/src/test/groovy/io/micronaut/http/server/netty/binding/HeaderBindingSpec.groovy

@@ -23,7 +23,6 @@ import io.micronaut.http.annotation.Controller
 import io.micronaut.http.annotation.Get
 import io.micronaut.http.annotation.Header
 import io.micronaut.http.server.netty.AbstractMicronautSpec
-import reactor.core.publisher.Flux
 import spock.lang.Shared
 import spock.lang.Unroll
 
@@ -53,7 +52,7 @@ class HeaderBindingSpec extends AbstractMicronautSpec {
         for (header in headers) {
             request = request.header(header.key, header.value)
         }
-        rxClient.toBlocking().retrieve(request) == result
+        httpClient.toBlocking().retrieve(request) == result
 
         where:
         uri                       | result                       | headers

http-server-netty/src/test/groovy/io/micronaut/http/server/netty/binding/HttpResponseSpec.groovy

@@ -43,7 +43,7 @@ class HttpResponseSpec extends AbstractMicronautSpec {
     void "test custom HTTP response for java action #action"() {
 
         when:
-        HttpResponse<?> response = Flux.from(rxClient.exchange("/java/response/$action", String))
+        HttpResponse<?> response = Flux.from(httpClient.exchange("/java/response/$action", String))
                 .onErrorResume(t -> {
             if (t instanceof HttpClientResponseException) {
                 return Flux.just(((HttpClientResponseException) t).response)
@@ -85,7 +85,7 @@ class HttpResponseSpec extends AbstractMicronautSpec {
     @Unroll
     void "test custom HTTP response for action #action"() {
         when:
-        HttpResponse<?> response = Flux.from(rxClient.exchange("/java/response/$action", String))
+        HttpResponse<?> response = Flux.from(httpClient.exchange("/java/response/$action", String))
                 .onErrorResume(t -> {
                     if (t instanceof HttpClientResponseException) {
                         return Flux.just(((HttpClientResponseException) t).response)
@@ -121,7 +121,7 @@ class HttpResponseSpec extends AbstractMicronautSpec {
 
     void "test content encoding"() {
         when:
-        HttpResponse<String> response = Flux.from(rxClient.exchange(HttpRequest.GET("/java/response/ok-with-body").header("Accept-Encoding", "gzip"), String))
+        HttpResponse<String> response = Flux.from(httpClient.exchange(HttpRequest.GET("/java/response/ok-with-body").header("Accept-Encoding", "gzip"), String))
                 .onErrorResume(t -> {
                     if (t instanceof HttpClientResponseException) {
                         return Flux.just(((HttpClientResponseException) t).response)
@@ -138,7 +138,7 @@ class HttpResponseSpec extends AbstractMicronautSpec {
 
     void "test custom headers"() {
         when:
-        HttpResponse<?> response = Flux.from(rxClient.exchange(HttpRequest.GET("/java/response/custom-headers")))
+        HttpResponse<?> response = Flux.from(httpClient.exchange(HttpRequest.GET("/java/response/custom-headers")))
                 .onErrorResume(t -> {
                     if (t instanceof HttpClientResponseException) {
                         return Flux.just(((HttpClientResponseException) t).response)