micronaut-core icon indicating copy to clipboard operation
micronaut-core copied to clipboard

Improvements to empty body server behavior

Open dstepanov opened this issue 7 months ago • 3 comments

  • Removed special InputStream binder so we can provide an implementation from the message body registry
  • Allow only skip empty body for nullable arguments

I think the null check should be somehow configurable globally or on the @Body annotation.

dstepanov avatar Sep 01 '25 13:09 dstepanov

Can you check the failing tests? We cannot break existing behaviour

graemerocher avatar Sep 01 '25 14:09 graemerocher

Actual diff without indent:

Index: http-server-netty/src/main/java/io/micronaut/http/server/netty/binders/NettyBodyAnnotationBinder.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/http-server-netty/src/main/java/io/micronaut/http/server/netty/binders/NettyBodyAnnotationBinder.java b/http-server-netty/src/main/java/io/micronaut/http/server/netty/binders/NettyBodyAnnotationBinder.java
--- a/http-server-netty/src/main/java/io/micronaut/http/server/netty/binders/NettyBodyAnnotationBinder.java	(revision ef4b1673169c26430b7c81cede455139780c1642)
+++ b/http-server-netty/src/main/java/io/micronaut/http/server/netty/binders/NettyBodyAnnotationBinder.java	(date 1756793254061)
@@ -38,11 +38,13 @@
 import io.micronaut.http.body.MessageBodyReader;
 import io.micronaut.http.codec.CodecException;
 import io.micronaut.http.context.ServerHttpRequestContext;
+import io.micronaut.http.exceptions.ContentLengthExceededException;
 import io.micronaut.http.netty.body.AvailableNettyByteBody;
 import io.micronaut.http.server.netty.FormDataHttpContentProcessor;
 import io.micronaut.http.server.netty.FormRouteCompleter;
 import io.micronaut.http.server.netty.MicronautHttpData;
 import io.micronaut.http.server.netty.NettyHttpRequest;
+import io.micronaut.http.server.netty.NettyHttpServer;
 import io.micronaut.http.server.netty.configuration.NettyHttpServerConfiguration;
 import io.micronaut.http.server.netty.converters.NettyConverters;
 import io.micronaut.web.router.RouteAttributes;
@@ -53,6 +55,8 @@
 import io.netty.buffer.CompositeByteBuf;
 import io.netty.handler.codec.http.DefaultLastHttpContent;
 import io.netty.handler.codec.http.multipart.InterfaceHttpData;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -61,12 +65,15 @@
 
 @Internal
 final class NettyBodyAnnotationBinder<T> extends DefaultBodyAnnotationBinder<T> {
+
+    private static final Logger LOG = LoggerFactory.getLogger(NettyHttpServer.class);
+
     final NettyHttpServerConfiguration httpServerConfiguration;
     final MessageBodyHandlerRegistry bodyHandlerRegistry;
 
     NettyBodyAnnotationBinder(ConversionService conversionService,
                               NettyHttpServerConfiguration httpServerConfiguration,
-                                     MessageBodyHandlerRegistry bodyHandlerRegistry) {
+                              MessageBodyHandlerRegistry bodyHandlerRegistry) {
         super(conversionService);
         this.httpServerConfiguration = httpServerConfiguration;
         this.bodyHandlerRegistry = bodyHandlerRegistry;
@@ -103,7 +110,7 @@
         if (!(source instanceof NettyHttpRequest<?> nhr)) {
             return super.bindFullBody(context, source);
         }
-        if (nhr.byteBody().expectedLength().orElse(-1) == 0) {
+        if (context.getArgument().isNullable() && nhr.byteBody().expectedLength().orElse(-1) == 0) {
             return BindingResult.empty();
         }
 
@@ -148,6 +155,7 @@
     }
 
     Optional<T> transform(NettyHttpRequest<?> nhr, ArgumentConversionContext<T> context, AvailableByteBody imm) throws Throwable {
+        try {
         MessageBodyReader<T> reader = null;
         final RouteInfo<?> routeInfo = RouteAttributes.getRouteInfo(nhr).orElse(null);
         if (routeInfo != null) {
@@ -155,7 +163,7 @@
         }
         MediaType mediaType = nhr.getContentType().orElse(null);
         if (mediaType != null && (reader == null || !reader.isReadable(context.getArgument(), mediaType))) {
-            reader = bodyHandlerRegistry.findReader(context.getArgument(), List.of(mediaType)).orElse(null);
+            reader = bodyHandlerRegistry.findReader(context.getArgument(), mediaType).orElse(null);
         }
         if (reader == null && nhr.isFormOrMultipartData()) {
             FormDataHttpContentProcessor processor = new FormDataHttpContentProcessor(nhr, httpServerConfiguration);
@@ -208,6 +216,12 @@
         NettyConverters.postProcess(byteBuf, converted);
         nhr.setLegacyBody(converted.orElse(null));
         return converted;
+        } catch (ContentLengthExceededException t) {
+            if (LOG.isTraceEnabled()) {
+                LOG.trace("Server received error for argument [{}]: {}", context.getArgument(), t.getMessage(), t);
+            }
+            return Optional.empty();
+        }
     }
 
     private static CompositeByteBuf coerceToComposite(List<?> objects, ByteBufAllocator alloc) {

yawkat avatar Sep 02 '25 06:09 yawkat

Look like just adding nullable doesn't cover cases where HttpRequest is injected

dstepanov avatar Sep 02 '25 10:09 dstepanov