search

kpavlov / jreactive-8583

Kotlin/Java Client & Server for ISO8583 & Netty
Apache License 2.0
320 stars 144 forks source link

Iso8583Decoder decode method issue #9

Closed AmirHooshangi closed 4 years ago

AmirHooshangi commented 8 years ago

Hi in the decode method you just check: if (!byteBuf.isReadable()) { return; } since TCP packets may come in a chunked base manner, some parts of your IsoMessage bytes may come in the next read. Shouldn't we say something like this : IsomessageLength = first 4 bytes in the packet if (IsomessageLength < byteBuf.readableBytes()) { return; } Have you ever done StressTest this code ? i think it would clear my point.

kpavlov commented 8 years ago

Hi, thanks for the issue. I didn't do a stress test yet. The fix you're suggesting looks reasonable. Please submit pull request or I'll fix it a bit later.

AmirHooshangi commented 8 years ago

Sure, if i wrote the patch I'll do PR.

kpavlov commented 8 years ago

BTW, there is a pipeline handler responsible for aggregated splitted frames:

pipeline.addLast("lengthFieldFameDecoder", new LengthFieldBasedFrameDecoder(
configuration.getMaxFrameLength(), 0, headerLength, 0, headerLength));

It is configured in Iso8583ChannelInitializer

It needs to write a stress test anyway to prove then everything works as expected.

AmirHooshangi commented 8 years ago

I've wrote a simple ByteToMessage decoder which is based on calculating message length from the first 4 bytes and it works fine under 100 request, but starts to misbehave after that, i think it's due to the ByteBuf reader index. I'll check LengthFieldBasedFrameDecoder to see if it helps.

Vostan commented 4 years ago

Hi folks,

This issue is quite old I wander if there is some solution to it out there,

E.g. currently we are having similar issue. We connect to Jpos iso server and get iso messages in this format (first 4 bytes, the length of message)(MTI)(bit map)(message), currently we just cut the first 4byte so message will be parsed, but this is not correct and in even small load it does not work.

kpavlov commented 4 years ago

Is it still valid?

Vostan commented 4 years ago

It is resolved already by another ticket. https://github.com/kpavlov/jreactive-8583/issues/83