Fix controller @Body Object parameters (#11267)

There's two bugs here.

Let's consider the following cases:

1. `application/x-www-form-urlencoded`, `@Body Object` param (tested by the old FormDataDiskSpec)
2. `application/json`, `@Body Object` param (prev. no test, tested by a new TCK test)
3. `application/x-weird`, `@Body Object` param (prev. no test, tested by a new netty-specific test)
4. `application/x-weird`, `@Body MyRecord` param (prev. no test, tested by a new netty-specific test)

Prior to 4.6, case 2 has a registered "normal" reader (the JSON reader) that can handle this case fine, the other three cases do not have readers and rely on fallback conversion logic. Forms have some special handling here.

In 4.6, the way raw type readers (e.g. that for String) were resolved changed. This means that for `Object` params, all raw type readers were now eligible, since they can read a subtype of Object (e.g. String). For cases 1, 3, and 4, this is a breaking change, since the reading is now done by a (random) raw type reader, instead of falling back to conversion logic.

This change broke the FormDataDiskSpec (case 1). To fix this, 1d09d95 introduced a check that would blanket-ignore the reader for all Object-typed params, falling back to the conversion logic. This fixed cases 1 and 3, but inadvertently broke case 2, since the JSON reader was also ignored.

Case 2 is apparently common enough that there have been multiple reports about it. It also exposed the second bug, a double-claim of the body, which is a fairly simple fix. But it shows that the fallback conversion logic had no test coverage, since this bug should trigger reliably. That is what `BodyConversionSpec` is there to improve.

My proposed fix is to make the raw message readers not apply to Object parameters, and to remove the band-aid fix from 1d09d95 that broke case 2.

Fixes #11266

.github/workflows/gradle.yml

@@ -45,7 +45,7 @@ jobs:
           fetch-depth: 0
 
       - name: "🔧 Setup GraalVM CE"
-        uses: graalvm/[email protected].3
+        uses: graalvm/[email protected].4
         with:
           distribution: 'graalvm'
           java-version: ${{ matrix.java }}

http-server-netty/src/main/java/io/micronaut/http/server/netty/binders/NettyBodyAnnotationBinder.java

@@ -157,10 +157,6 @@ Optional<T> transform(NettyHttpRequest<?> nhr, ArgumentConversionContext<T> cont
         if (mediaType != null && (reader == null || !reader.isReadable(context.getArgument(), mediaType))) {
             reader = bodyHandlerRegistry.findReader(context.getArgument(), List.of(mediaType)).orElse(null);
         }
-        if (reader != null && context.getArgument().getType().equals(Object.class)) {
-            // Prevent random object convertors
-            reader = null;
-        }
         if (reader == null && nhr.isFormOrMultipartData()) {
             FormDataHttpContentProcessor processor = new FormDataHttpContentProcessor(nhr, httpServerConfiguration);
             ByteBuf buf = AvailableNettyByteBody.toByteBuf(imm);
@@ -202,9 +198,8 @@ Optional<T> transform(NettyHttpRequest<?> nhr, ArgumentConversionContext<T> cont
             nhr.setLegacyBody(converted.orElse(null));
             return converted;
         }
-        ByteBuffer<?> byteBuffer = imm.toByteBuffer();
         if (reader != null) {
-            T result = read(context, reader, nhr.getHeaders(), mediaType, byteBuffer);
+            T result = read(context, reader, nhr.getHeaders(), mediaType, imm.toByteBuffer());
             nhr.setLegacyBody(result);
             return Optional.ofNullable(result);
         }

http-server-netty/src/test/groovy/io/micronaut/http/server/netty/body/BodyConversionSpec.groovy

@@ -0,0 +1,90 @@
+package io.micronaut.http.server.netty.body
+
+import io.micronaut.context.ApplicationContext
+import io.micronaut.context.annotation.Requires
+import io.micronaut.core.convert.MutableConversionService
+import io.micronaut.core.convert.TypeConverterRegistrar
+import io.micronaut.http.HttpRequest
+import io.micronaut.http.annotation.Body
+import io.micronaut.http.annotation.Consumes
+import io.micronaut.http.annotation.Controller
+import io.micronaut.http.annotation.Post
+import io.micronaut.http.client.HttpClient
+import io.micronaut.runtime.server.EmbeddedServer
+import io.netty.buffer.ByteBuf
+import jakarta.inject.Singleton
+import spock.lang.Specification
+
+import java.nio.charset.StandardCharsets
+
+class BodyConversionSpec extends Specification {
+    static final String WEIRD_CONTENT_TYPE = "application/x-weird"
+
+    def 'weird content type, object param'() {
+        given:
+        def ctx = ApplicationContext.run(['spec.name': 'BodyConversionSpec'])
+        def server = ctx.getBean(EmbeddedServer)
+        server.start()
+        def client = ctx.createBean(HttpClient, server.URI).toBlocking()
+
+        when:
+        def response = client.retrieve(HttpRequest.POST('/body-conversion/weird-object', "foo")
+                .contentType(WEIRD_CONTENT_TYPE), String)
+        then:
+        response == 'body: foo'
+
+        cleanup:
+        client.close()
+        server.stop()
+    }
+
+    def 'weird content type, converted param'() {
+        given:
+        def ctx = ApplicationContext.run(['spec.name': 'BodyConversionSpec'])
+        def server = ctx.getBean(EmbeddedServer)
+        server.start()
+        def client = ctx.createBean(HttpClient, server.URI).toBlocking()
+
+        when:
+        def response = client.retrieve(HttpRequest.POST('/body-conversion/weird-converted', "foo")
+                .contentType(WEIRD_CONTENT_TYPE), String)
+        then:
+        response == 'body: MyRecord[s=foo]'
+
+        cleanup:
+        client.close()
+        server.stop()
+    }
+
+    @Controller("/body-conversion")
+    @Requires(property = "spec.name", value = "BodyConversionSpec")
+    static class MyCtrl {
+        @Consumes(WEIRD_CONTENT_TYPE)
+        @Post("/weird-object")
+        String object(@Body Object o) {
+            def text = ((ByteBuf) o).toString(StandardCharsets.UTF_8)
+            o.release()
+            return "body: " + text
+        }
+
+        @Consumes(WEIRD_CONTENT_TYPE)
+        @Post("/weird-converted")
+        String converted(@Body MyRecord o) {
+            return "body: " + o
+        }
+    }
+
+    @Singleton
+    @Requires(property = "spec.name", value = "BodyConversionSpec")
+    static class MyConverter implements TypeConverterRegistrar {
+        @Override
+        void register(MutableConversionService conversionService) {
+            conversionService.addConverter(ByteBuf.class, MyRecord.class, buf -> {
+                return new MyRecord(buf.toString(StandardCharsets.UTF_8))
+            })
+        }
+    }
+
+    record MyRecord(String s) {
+    }
+}

http-server-tck/src/main/java/io/micronaut/http/server/tck/tests/BodyTest.java

@@ -184,6 +184,19 @@ void testRedirectFuture() throws IOException {
             .run();
     }
 
+    @Test
+    void testObjectBody() throws IOException {
+        TestScenario.builder()
+            .specName(SPEC_NAME)
+            .request(HttpRequest.POST("/response-body/object", Map.of("fizz", "buzz")))
+            .assertion((server, request) -> AssertionUtils.assertDoesNotThrow(server, request,
+                HttpResponseAssertion.builder()
+                    .status(HttpStatus.CREATED)
+                    .body("obj: {fizz=buzz}")
+                    .build()))
+            .run();
+    }
+
     @Controller("/response-body")
     @Requires(property = "spec.name", value = SPEC_NAME)
     static class BodyController {
@@ -253,6 +266,14 @@ Publisher<HttpResponse<?>> emptySingleResult() {
         CompletableFuture<HttpResponse<?>> redirectFuture() {
             return CompletableFuture.completedFuture(HttpResponse.status(HttpStatus.FOUND).header("Location", "https://example.com"));
         }
+
+        @SuppressWarnings("DefaultAnnotationParam")
+        @Post(uri = "/object", consumes = MediaType.TEXT_PLAIN)
+        @Status(HttpStatus.CREATED)
+        @Consumes(MediaType.APPLICATION_JSON)
+        String objectBody(@Body Object obj) {
+            return "obj: " + obj;
+        }
     }
 
     @Introspected

http/src/main/java/io/micronaut/http/body/TypedMessageBodyReader.java

@@ -38,6 +38,6 @@ public interface TypedMessageBodyReader<T> extends MessageBodyReader<T> {
 
     @Override
     default boolean isReadable(Argument<T> type, MediaType mediaType) {
-        return type.isAssignableFrom(getType());
+        return type.isAssignableFrom(getType()) && !type.getType().equals(Object.class);
     }
 }