Fix ReadPacket and WritePacket payload length is a multiple #209
Conversation
|
PTAL @WangXiangUSTC |
|
@michaelyou could you please tell me how to reappear the bug? provide a test case |
|
I think it's obvious. If the payload len is a multiply of MaxPayloadLen, the origin code will read once more, but there is nothing for reading. you will get an error in line 81. @WangXiangUSTC |
|
This bug may not happen in practical use(the probability is really small that the payload has a length of exactly MaxLengthlen)，but it really exists logically. |
|
I think we should add a test now. Conn needs a net.Conn which we can construct a dummy one in the test with the length of payload equal MaxLength |
|
@michaelyou can you help us add a uint test of this function? otherwise it is difficult to check the correctness |
|
Is this going to be merged? seems like a test like https://github.com/vitessio/vitess/pull/3935/files#diff-6a379d5bc6a249add52fb846b2ee9c3f would be sufficient |
| return errors.Errorf("invalid payload length %d", length) | ||
| } | ||
| // packet length [24 bit] | ||
| length := int(uint32(header[0]) | uint32(header[1])<<8 | uint32(header[2])<<16) |
There was a problem hiding this comment.
Why not checking length with max payload length here?
MaxPayloadLen
#208