Skip to content

bug: NettyApiServer reads request body before validating route exists #291

@sfloess

Description

@sfloess

Bug Description

The HttpRequestHandler reads and decodes the entire request body using request.content().toString() before checking if a route handler exists. This wastes CPU and memory processing request bodies for non-existent routes.

Location

jplatform-rest-api-netty/src/main/java/org/flossware/jplatform/rest/netty/NettyApiServer.java:206-213

Problematic Code

@Override
protected void channelRead0(ChannelHandlerContext ctx, FullHttpRequest request) {
    String uri = request.uri();
    String content = request.content().toString(CharsetUtil.UTF_8);  // Decoded before route check!

    Function<String, String> handler = routes.get(uri);

    FullHttpResponse response;
    if (handler != null) {
        // Use content
    } else {
        // 404 - content was decoded but never used!
        response = new DefaultFullHttpResponse(
            HttpVersion.HTTP_1_1,
            HttpResponseStatus.NOT_FOUND,
            Unpooled.copiedBuffer("{\"error\":\"Route not found\"}", CharsetUtil.UTF_8)
        );
    }
}

Impact

  • Wasted CPU: Decoding UTF-8 for requests that will be rejected
  • DoS amplification: Attacker sends large bodies to non-existent routes
  • Memory allocation: String created and immediately discarded
  • Performance degradation: Unnecessary work on every 404

Example

NettyApiServer server = new NettyApiServer(config);
server.addRoute("/api/valid", content -> "response");
server.start();

// Attacker probes for routes with large body
POST /api/invalid HTTP/1.1
Content-Length: 10485760
Content-Type: application/json

{large 10MB JSON payload}

// HttpRequestHandler:
String uri = "/api/invalid";
String content = <decode 10MB to UTF-8 string>;  // Wasted work!
Function handler = routes.get("/api/invalid");  // null
// Return 404
// 10MB decoded for nothing

// Attacker repeats with different invalid routes:
POST /api/test1 (10MB body) -> 404, 10MB decoded
POST /api/test2 (10MB body) -> 404, 10MB decoded
POST /api/test3 (10MB body) -> 404, 10MB decoded
// CPU exhausted decoding bodies for non-existent routes

Proposed Fix

Check route existence before reading body:

@Override
protected void channelRead0(ChannelHandlerContext ctx, FullHttpRequest request) {
    String uri = request.uri();

    Function<String, String> handler = routes.get(uri);

    FullHttpResponse response;
    if (handler != null) {
        // Route exists, now read and decode content
        String content = request.content().toString(CharsetUtil.UTF_8);
        
        try {
            String responseBody = handler.apply(content);
            response = new DefaultFullHttpResponse(
                HttpVersion.HTTP_1_1,
                HttpResponseStatus.OK,
                Unpooled.copiedBuffer(responseBody, CharsetUtil.UTF_8)
            );
            response.headers().set(HttpHeaderNames.CONTENT_TYPE, "application/json");
        } catch (Exception e) {
            logger.error("Error handling request to {}", uri, e);
            response = new DefaultFullHttpResponse(
                HttpVersion.HTTP_1_1,
                HttpResponseStatus.INTERNAL_SERVER_ERROR,
                Unpooled.copiedBuffer("{\"error\":\"Internal server error\"}", CharsetUtil.UTF_8)
            );
            response.headers().set(HttpHeaderNames.CONTENT_TYPE, "application/json");
        }
    } else {
        // Route not found - don't decode content
        response = new DefaultFullHttpResponse(
            HttpVersion.HTTP_1_1,
            HttpResponseStatus.NOT_FOUND,
            Unpooled.copiedBuffer("{\"error\":\"Route not found\"}", CharsetUtil.UTF_8)
        );
        response.headers().set(HttpHeaderNames.CONTENT_TYPE, "application/json");
    }

    response.headers().set(HttpHeaderNames.CONTENT_LENGTH, response.content().readableBytes());

    if (HttpUtil.isKeepAlive(request)) {
        response.headers().set(HttpHeaderNames.CONNECTION, HttpHeaderValues.KEEP_ALIVE);
    }

    ctx.writeAndFlush(response);

    if (!HttpUtil.isKeepAlive(request)) {
        ctx.close();
    }
}

Additional optimization - lazy content reading:

@Override
protected void channelRead0(ChannelHandlerContext ctx, FullHttpRequest request) {
    String uri = request.uri();
    Function<String, String> handler = routes.get(uri);

    FullHttpResponse response;
    if (handler != null) {
        try {
            // Lazy content reading - only if handler exists
            String content;
            ByteBuf contentBuf = request.content();
            
            if (contentBuf.isReadable()) {
                content = contentBuf.toString(CharsetUtil.UTF_8);
            } else {
                content = "";  // No content
            }
            
            String responseBody = handler.apply(content);
            response = new DefaultFullHttpResponse(
                HttpVersion.HTTP_1_1,
                HttpResponseStatus.OK,
                Unpooled.copiedBuffer(responseBody, CharsetUtil.UTF_8)
            );
            response.headers().set(HttpHeaderNames.CONTENT_TYPE, "application/json");
        } catch (Exception e) {
            logger.error("Error handling request", e);
            response = new DefaultFullHttpResponse(
                HttpVersion.HTTP_1_1,
                HttpResponseStatus.INTERNAL_SERVER_ERROR,
                Unpooled.copiedBuffer("{\"error\":\"Internal server error\"}", CharsetUtil.UTF_8)
            );
            response.headers().set(HttpHeaderNames.CONTENT_TYPE, "application/json");
        }
    } else {
        response = new DefaultFullHttpResponse(
            HttpVersion.HTTP_1_1,
            HttpResponseStatus.NOT_FOUND,
            Unpooled.copiedBuffer("{\"error\":\"Route not found\"}", CharsetUtil.UTF_8)
        );
        response.headers().set(HttpHeaderNames.CONTENT_TYPE, "application/json");
    }

    response.headers().set(HttpHeaderNames.CONTENT_LENGTH, response.content().readableBytes());
    
    if (HttpUtil.isKeepAlive(request)) {
        response.headers().set(HttpHeaderNames.CONNECTION, HttpHeaderValues.KEEP_ALIVE);
    }

    ctx.writeAndFlush(response);

    if (!HttpUtil.isKeepAlive(request)) {
        ctx.close();
    }
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions