Wrong cast in StringCodec may lead to IndexOutOfBoundsException

[dmandalidis]

Bug Report

Current Behavior

Consider the following:

int total_1 = (int) (3.0f * 5_592_795); // prints 16778384
int total_2 = (int) 3.0f * 5_592_795; // prints 16778385

In StringCodec#encodeAndAllocateBuffer the byte buffer size is allocated using the method in total_1 above. However, in ByteBufUtil.writeUtf8(target, str) of StringCodec#encode which subsequently calls ByteBufUtil#utf8MaxBytes, the Netty buffer is allocated with something more similar to total_2 above.

In such cases the exception thrown is as follows:

Exception in thread "main" io.lettuce.core.RedisException: io.netty.handler.codec.EncoderException: Cannot encode command. Please close the connection as the connection state may be out of sync.
                at io.lettuce.core.LettuceFutures.awaitOrCancel(LettuceFutures.java:135)
                at io.lettuce.core.FutureSyncInvocationHandler.handleInvocation(FutureSyncInvocationHandler.java:69)
                at io.lettuce.core.internal.AbstractInvocationHandler.invoke(AbstractInvocationHandler.java:80)
                at com.sun.proxy.$Proxy0.set(Unknown Source)
                at eu.europa.echa.ecs.message.rx.redis.storage.RedisStringSerde.serialize(RedisStringSerde.java:26)
                at eu.europa.echa.ecs.message.rx.redis.storage.RedisStorage.put(RedisStorage.java:118)
                at eu.europa.echa.ecs.message.rx.redis.storage.RedisStorage.lambda$put$0(RedisStorage.java:76)
                at eu.europa.echa.ecs.message.rx.redis.RxRedis$NodeCommandSyncExecutor.doExecute(RxRedis.java:159)
                at eu.europa.echa.ecs.message.rx.redis.RxRedis$NodeCommandSyncExecutor.call(RxRedis.java:144)
                at eu.europa.echa.ecs.message.rx.redis.storage.RedisStorage.put(RedisStorage.java:76)
                at eu.europa.echa.scip.checker.AltController.main(AltController.java:70)
Caused by: io.netty.handler.codec.EncoderException: Cannot encode command. Please close the connection as the connection state may be out of sync.
                at io.lettuce.core.protocol.CommandEncoder.encode(CommandEncoder.java:95)
                at io.lettuce.core.protocol.CommandEncoder.encode(CommandEncoder.java:77)
                at io.netty.handler.codec.MessageToByteEncoder.write(MessageToByteEncoder.java:107)
                at io.netty.channel.AbstractChannelHandlerContext.invokeWrite0(AbstractChannelHandlerContext.java:717)
                at io.netty.channel.AbstractChannelHandlerContext.invokeWrite(AbstractChannelHandlerContext.java:709)
                at io.netty.channel.AbstractChannelHandlerContext.write(AbstractChannelHandlerContext.java:792)
                at io.netty.channel.AbstractChannelHandlerContext.write(AbstractChannelHandlerContext.java:702)
                at io.lettuce.core.protocol.CommandHandler.writeSingleCommand(CommandHandler.java:421)
                at io.lettuce.core.protocol.CommandHandler.write(CommandHandler.java:353)
                at io.netty.channel.AbstractChannelHandlerContext.invokeWrite0(AbstractChannelHandlerContext.java:717)
                at io.netty.channel.AbstractChannelHandlerContext.invokeWriteAndFlush(AbstractChannelHandlerContext.java:764)
                at io.netty.channel.AbstractChannelHandlerContext$WriteTask.run(AbstractChannelHandlerContext.java:1071)
                at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:164)
                at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:472)
                at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:500)
                at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989)
                at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
                at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
                at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.IndexOutOfBoundsException: writerIndex(0) + minWritableBytes(16781385) exceeds maxCapacity(16781384): UnpooledHeapByteBuf(ridx: 0, widx: 0, cap: 16781384/16781384)
                at io.netty.buffer.AbstractByteBuf.ensureWritable0(AbstractByteBuf.java:295)
                at io.netty.buffer.ByteBufUtil.reserveAndWriteUtf8Seq(ByteBufUtil.java:553)
                at io.netty.buffer.ByteBufUtil.writeUtf8(ByteBufUtil.java:510)
                at io.lettuce.core.codec.StringCodec.encode(StringCodec.java:93)
                at io.lettuce.core.codec.StringCodec.encodeAndAllocateBuffer(StringCodec.java:173)
                at io.lettuce.core.codec.StringCodec.encodeValue(StringCodec.java:154)
                at io.lettuce.core.codec.StringCodec.encodeValue(StringCodec.java:39)
                at io.lettuce.core.codec.CompressionCodec$CompressingValueCodecWrapper.encodeValue(CompressionCodec.java:89)
                at io.lettuce.core.protocol.CommandArgs$ValueArgument.encode(CommandArgs.java:712)
                at io.lettuce.core.protocol.CommandArgs.encode(CommandArgs.java:347)
                at io.lettuce.core.protocol.Command.encode(Command.java:120)
                at io.lettuce.core.protocol.AsyncCommand.encode(AsyncCommand.java:185)
                at io.lettuce.core.protocol.CommandEncoder.encode(CommandEncoder.java:92)
                ... 18 more

Environment

• Lettuce: 5.3.1.RELEASE

Possible Solution

I believe that the following line in StringCodec#encodeAndAllocateBuffer:

ByteBuffer buffer = ByteBuffer.allocate((int) (encoder.maxBytesPerChar() * key.length()));

should change to

ByteBuffer buffer = ByteBuffer.allocate((int) encoder.maxBytesPerChar() * key.length());

similar handling may apply to StringCodec#encode where the line:

int length = (int) ((double) str.length() * encoder.maxBytesPerChar());

should change to:

int length = str.length() * (int) encoder.maxBytesPerChar();

[dmandalidis]

Moreover, I would really appreciate to have this included in an upcoming 5.3.x release (obviously if you accept it as a bug and agree with the fix). Workaround is to implement our own StringCodec (extending the original one)

[mp911de]

Good catch. In fact, we have multiple length calculations. Do you want to submit a pull request that fixes the issue and pulls calculation into a single place?

[dmandalidis]

@mp911de sure. Hopefully, it would be ready by tomorrow for review

[dmandalidis]

@mp911de done. I didn't follow the same approach for estimateSize (I guess we don't need to be that accurate on that)

[dmandalidis]

Hi @mp911de

It appears that the polishing-named commit (849173f) after my PR got merged, re-introduced the issue we were trying to fix :( Casting is again wrong, please expect a new PR today for this