Skip to content

fix: enhance token server request handling and add max frame length validation to prevent memory issues #3572

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yungyu16 wants to merge 1 commit into
base: 1.8
Choose a base branch
from
Open

fix: enhance token server request handling and add max frame length validation to prevent memory issues #3572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 30 additions & 28 deletions ...r-default/src/main/java/com/alibaba/csp/sentinel/cluster/server/NettyTransportServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,12 @@
*/
package com.alibaba.csp.sentinel.cluster.server;

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;

import com.alibaba.csp.sentinel.cluster.server.codec.netty.NettyRequestDecoder;
import com.alibaba.csp.sentinel.cluster.server.codec.netty.NettyResponseEncoder;
import com.alibaba.csp.sentinel.cluster.server.connection.Connection;
import com.alibaba.csp.sentinel.cluster.server.connection.ConnectionPool;
import com.alibaba.csp.sentinel.cluster.server.handler.TokenServerHandler;
import com.alibaba.csp.sentinel.log.RecordLog;

import io.netty.bootstrap.ServerBootstrap;
import io.netty.buffer.PooledByteBufAllocator;
import io.netty.channel.ChannelFuture;
Expand All @@ -42,7 +37,14 @@
import io.netty.util.concurrent.GenericFutureListener;
import io.netty.util.internal.SystemPropertyUtil;

import static com.alibaba.csp.sentinel.cluster.server.ServerConstants.*;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;

import static com.alibaba.csp.sentinel.cluster.server.ServerConstants.NETTY_MAX_FRAME_LENGTH;
import static com.alibaba.csp.sentinel.cluster.server.ServerConstants.SERVER_STATUS_OFF;
import static com.alibaba.csp.sentinel.cluster.server.ServerConstants.SERVER_STATUS_STARTED;
import static com.alibaba.csp.sentinel.cluster.server.ServerConstants.SERVER_STATUS_STARTING;

/**
* @author Eric Zhao
Expand All @@ -51,7 +53,7 @@
public class NettyTransportServer implements ClusterTokenServer {

private static final int DEFAULT_EVENT_LOOP_THREADS = Math.max(1,
SystemPropertyUtil.getInt("io.netty.eventLoopThreads", Runtime.getRuntime().availableProcessors() * 2));
SystemPropertyUtil.getInt("io.netty.eventLoopThreads", Runtime.getRuntime().availableProcessors() * 2));
private static final int MAX_RETRY_TIMES = 3;
private static final int RETRY_SLEEP_MS = 2000;

Expand Down Expand Up @@ -79,32 +81,32 @@ public void start() {
this.bossGroup = new NioEventLoopGroup(1);
this.workerGroup = new NioEventLoopGroup(DEFAULT_EVENT_LOOP_THREADS);
b.group(bossGroup, workerGroup)
.channel(NioServerSocketChannel.class)
.option(ChannelOption.SO_BACKLOG, 128)
.handler(new LoggingHandler(LogLevel.INFO))
.childHandler(new ChannelInitializer<SocketChannel>() {
@Override
public void initChannel(SocketChannel ch) throws Exception {
ChannelPipeline p = ch.pipeline();
p.addLast(new LengthFieldBasedFrameDecoder(1024, 0, 2, 0, 2));
p.addLast(new NettyRequestDecoder());
p.addLast(new LengthFieldPrepender(2));
p.addLast(new NettyResponseEncoder());
p.addLast(new TokenServerHandler(connectionPool));
}
})
.childOption(ChannelOption.ALLOCATOR, PooledByteBufAllocator.DEFAULT)
.childOption(ChannelOption.SO_SNDBUF, 32 * 1024)
.childOption(ChannelOption.CONNECT_TIMEOUT_MILLIS, 10000)
.childOption(ChannelOption.SO_TIMEOUT, 10)
.childOption(ChannelOption.TCP_NODELAY, true)
.childOption(ChannelOption.SO_RCVBUF, 32 * 1024);
.channel(NioServerSocketChannel.class)
.option(ChannelOption.SO_BACKLOG, 128)
.handler(new LoggingHandler(LogLevel.INFO))
.childHandler(new ChannelInitializer<SocketChannel>() {
@Override
public void initChannel(SocketChannel ch) throws Exception {
ChannelPipeline p = ch.pipeline();
p.addLast(new LengthFieldBasedFrameDecoder(NETTY_MAX_FRAME_LENGTH, 0, 2, 0, 2));
p.addLast(new NettyRequestDecoder());
p.addLast(new LengthFieldPrepender(2));
p.addLast(new NettyResponseEncoder());
p.addLast(new TokenServerHandler(connectionPool));
}
})
.childOption(ChannelOption.ALLOCATOR, PooledByteBufAllocator.DEFAULT)
.childOption(ChannelOption.SO_SNDBUF, 32 * 1024)
.childOption(ChannelOption.CONNECT_TIMEOUT_MILLIS, 10000)
.childOption(ChannelOption.SO_TIMEOUT, 10)
.childOption(ChannelOption.TCP_NODELAY, true)
.childOption(ChannelOption.SO_RCVBUF, 32 * 1024);
b.bind(port).addListener(new GenericFutureListener<ChannelFuture>() {
@Override
public void operationComplete(ChannelFuture future) {
if (future.cause() != null) {
RecordLog.info("[NettyTransportServer] Token server start failed (port=" + port + "), failedTimes: " + failedTimes.get(),
future.cause());
future.cause());
currentState.compareAndSet(SERVER_STATUS_STARTING, SERVER_STATUS_OFF);
int failCount = failedTimes.incrementAndGet();
if (failCount > MAX_RETRY_TIMES) {
Expand Down
7 changes: 6 additions & 1 deletion ...server-default/src/main/java/com/alibaba/csp/sentinel/cluster/server/ServerConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,10 @@ public final class ServerConstants {

public static final String DEFAULT_NAMESPACE = "default";

private ServerConstants() {}
public static final int NETTY_MAX_FRAME_LENGTH = 1024;
public static final int MAX_PARAM_AMOUNT = 512;
public static final int MAX_PARAM_STRING_LENGTH = 1024;

private ServerConstants() {
}
}
77 changes: 56 additions & 21 deletions .../java/com/alibaba/csp/sentinel/cluster/server/codec/data/ParamFlowRequestDataDecoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@
package com.alibaba.csp.sentinel.cluster.server.codec.data;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import com.alibaba.csp.sentinel.cluster.ClusterConstants;
import com.alibaba.csp.sentinel.cluster.codec.EntityDecoder;
import com.alibaba.csp.sentinel.cluster.request.data.ParamFlowRequestData;
import com.alibaba.csp.sentinel.cluster.server.ServerConstants;

import io.netty.buffer.ByteBuf;
import io.netty.handler.codec.CorruptedFrameException;

/**
* @author jialiang.linjl
Expand All @@ -31,61 +34,93 @@
*/
public class ParamFlowRequestDataDecoder implements EntityDecoder<ByteBuf, ParamFlowRequestData> {

private static final int REQUEST_HEADER_LENGTH = Long.BYTES + Integer.BYTES + Integer.BYTES;
private static final int MIN_PARAM_LENGTH = Byte.BYTES + Byte.BYTES;

@Override
public ParamFlowRequestData decode(ByteBuf source) {
if (source.readableBytes() >= 16) {
ParamFlowRequestData requestData = new ParamFlowRequestData()
ensureReadable(source, REQUEST_HEADER_LENGTH);

ParamFlowRequestData requestData = new ParamFlowRequestData()
.setFlowId(source.readLong())
.setCount(source.readInt());

int amount = source.readInt();
if (amount > 0) {
List<Object> params = new ArrayList<>(amount);
for (int i = 0; i < amount; i++) {
decodeParam(source, params);
}
int amount = source.readInt();
if (amount < 0 || amount > ServerConstants.MAX_PARAM_AMOUNT
|| amount > source.readableBytes() / MIN_PARAM_LENGTH) {
throw new CorruptedFrameException("Invalid parameter amount: " + amount);
}

requestData.setParams(params);
return requestData;
List<Object> params;
if (amount == 0) {
params = Collections.emptyList();
} else {
params = new ArrayList<>(Math.min(amount, 16));
for (int i = 0; i < amount; i++) {
decodeParam(source, params);
}
}
return null;
if (source.isReadable()) {
throw new CorruptedFrameException("Parameter flow payload contains trailing bytes: "
+ source.readableBytes());
}
return requestData.setParams(params);
}

private boolean decodeParam(ByteBuf source, List<Object> params) {
private void decodeParam(ByteBuf source, List<Object> params) {
ensureReadable(source, Byte.BYTES);
byte paramType = source.readByte();

switch (paramType) {
case ClusterConstants.PARAM_TYPE_INTEGER:
ensureReadable(source, Integer.BYTES);
params.add(source.readInt());
return true;
return;
case ClusterConstants.PARAM_TYPE_STRING:
ensureReadable(source, Integer.BYTES);
int length = source.readInt();
if (length < 0 || length > ServerConstants.MAX_PARAM_STRING_LENGTH
|| length > source.readableBytes()) {
throw new CorruptedFrameException("Invalid string parameter length: " + length);
}
byte[] bytes = new byte[length];
source.readBytes(bytes);
// TODO: take care of charset?
params.add(new String(bytes));
return true;
return;
case ClusterConstants.PARAM_TYPE_BOOLEAN:
ensureReadable(source, Byte.BYTES);
params.add(source.readBoolean());
return true;
return;
case ClusterConstants.PARAM_TYPE_DOUBLE:
ensureReadable(source, Double.BYTES);
params.add(source.readDouble());
return true;
return;
case ClusterConstants.PARAM_TYPE_LONG:
ensureReadable(source, Long.BYTES);
params.add(source.readLong());
return true;
return;
case ClusterConstants.PARAM_TYPE_FLOAT:
ensureReadable(source, Float.BYTES);
params.add(source.readFloat());
return true;
return;
case ClusterConstants.PARAM_TYPE_BYTE:
ensureReadable(source, Byte.BYTES);
params.add(source.readByte());
return true;
return;
case ClusterConstants.PARAM_TYPE_SHORT:
ensureReadable(source, Short.BYTES);
params.add(source.readShort());
return true;
return;
default:
return false;
throw new CorruptedFrameException("Unknown parameter type: " + paramType);
}
}

private void ensureReadable(ByteBuf source, int requiredBytes) {
if (source.readableBytes() < requiredBytes) {
throw new CorruptedFrameException("Incomplete parameter flow payload: required=" + requiredBytes
+ ", actual=" + source.readableBytes());
}
}
}
22 changes: 14 additions & 8 deletions .../main/java/com/alibaba/csp/sentinel/cluster/server/codec/data/PingRequestDataDecoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.alibaba.csp.sentinel.cluster.codec.EntityDecoder;

import io.netty.buffer.ByteBuf;
import io.netty.handler.codec.CorruptedFrameException;

/**
* @author Eric Zhao
Expand All @@ -27,14 +28,19 @@ public class PingRequestDataDecoder implements EntityDecoder<ByteBuf, String> {

@Override
public String decode(ByteBuf source) {
if (source.readableBytes() >= 4) {
int length = source.readInt();
if (length > 0 && source.readableBytes() > 0) {
byte[] bytes = new byte[length];
source.readBytes(bytes);
return new String(bytes);
}
if (source.readableBytes() < Integer.BYTES) {
throw new CorruptedFrameException("Incomplete ping payload length");
}
return null;

int packetLen = source.readInt();
int actualLength = source.readableBytes();
if (packetLen < 0 || packetLen != actualLength) {
throw new CorruptedFrameException("Invalid ping payload length: declared=" + packetLen
+ ", actual=" + actualLength);
}

byte[] bytes = new byte[packetLen];
source.readBytes(bytes);
return new String(bytes);
}
}
10 changes: 10 additions & 0 deletions ...ult/src/main/java/com/alibaba/csp/sentinel/cluster/server/handler/TokenServerHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.handler.codec.CorruptedFrameException;
import io.netty.handler.codec.TooLongFrameException;

/**
* Netty server handler for Sentinel token server.
Expand Down Expand Up @@ -81,6 +83,14 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
}
}

@Override
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
if (!(cause instanceof CorruptedFrameException) && !(cause instanceof TooLongFrameException)) {
RecordLog.warn("[TokenServerHandler] Unexpected exception", cause);
}
ctx.close();
}

private void writeBadResponse(ChannelHandlerContext ctx, ClusterRequest request) {
ClusterResponse<?> response = new ClusterResponse<>(request.getId(), request.getType(),
ClusterConstants.RESPONSE_STATUS_BAD, null);
Expand Down
Loading