decode_variable_int func bug

[TakuKitamura]

According to [1.5.5 Variable Byte Integer] of the MQTT5.0 specification, the algorithm to decode a variable byte integer type is as follows

Algorithm to decode a variable byte integer type

multiplier = 1

value = 0

do

   encodedByte = 'next byte from stream'

   value += (encodedByte AND 127) * multiplier

   if (multiplier > 128*128*128)

      throw Error(Malformed Variable Byte Integer)

   multiplier *= 128

while ((encodedByte AND 128) != 0)

Original

However, the implementation of decoding variable-byte integer types is wrong.
Specifically, the implementation of the arithmetic part of "multiplier" is different.

fn decode_variable_int(bytes: &mut Cursor<&mut BytesMut>) -> Result<Option<u32>, DecodeError> {
    let mut multiplier = 1;
    let mut value: u32 = 0;

    loop {
        let encoded_byte = read_u8!(bytes);

        value += ((encoded_byte & 0b0111_1111) as u32) * multiplier;
        multiplier *= 128;

        if multiplier > (128 * 128 * 128) {
            return Err(DecodeError::InvalidRemainingLength);
        }

        if encoded_byte & 0b1000_0000 == 0b0000_0000 {
            break;
        }
    }

    Ok(Some(value))
}

With the current implementation, if the client sends a PUBLISH packet with a variable-byte integer size of 2,097,152 (0x80,0x80,0x80,0x01) to 268,435,455 (0xFF,0xFF,0xFF,0xFF,0x7F), the following error will be output.

Error while reading frame: InvalidRemainingLength

Listening on 0.0.0.0:1883
Listening on 0.0.0.0:8080
Got a new socket from addr: V4(127.0.0.1:52705)
Handling a client
got a packet: Some(Ok(Connect(ConnectPacket { protocol_name: "MQTT", protocol_version: V500, clean_start: true, keep_alive: 60, session_expiry_interval: None, receive_maximum: Some(ReceiveMaximum(20)), maximum_packet_size: None, topic_alias_maximum: None, request_response_information: None, request_problem_information: None, user_properties: [], authentication_method: None, authentication_data: None, client_id: "", will: None, user_name: None, password: None })))
Client ID s5nsSQSgkIsv-BOYwKdUZ connected (Version: V500)
Error while reading frame: InvalidRemainingLength
rx
Client ID s5nsSQSgkIsv-BOYwKdUZ disconnected

Fixed

The revision is like this.

fn decode_variable_int(bytes: &mut Cursor<&mut BytesMut>) -> Result<Option<u32>, DecodeError> {
    let mut multiplier = 1;
    let mut value: u32 = 0;

    loop {
        let encoded_byte = read_u8!(bytes);

        value += ((encoded_byte & 0b0111_1111) as u32) * multiplier;

        if multiplier > (128 * 128 * 128) {
            return Err(DecodeError::InvalidRemainingLength);
        }

        multiplier *= 128; // here

        if encoded_byte & 0b1000_0000 == 0b0000_0000 {
            break;
        }
    }

    Ok(Some(value))
}

Please let me know if I'm saying something wrong.

[bschwind]

Wow yeah, I totally got the implementation wrong there, good catch!

If you'd like, feel free to open a PR with the fix and a test case for this, otherwise I can get to it a bit later today.

Thanks for the bug report!

[TakuKitamura]

All right, I'll open the PR!

[bschwind]

Thanks again for the PR! I merged it and published a new version on crates