Skip to content

Commit

Permalink
Packet payload fixes (#206)
Browse files Browse the repository at this point in the history 
* Relax overly strict condition on buffer encode
* Allow empty publish payload in decode step
  • Loading branch information
@bretambrose
bretambrose authored Nov 10, 2021
1 parent 2d4fe1c commit 60f9a17
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 2 deletions.
4 changes: 2 additions & 2 deletions source/packets.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ static size_t s_sizeof_encoded_buffer(struct aws_byte_cursor *buf) {
static int s_encode_buffer(struct aws_byte_buf *buf, const struct aws_byte_cursor cur) {

AWS_PRECONDITION(buf);
AWS_PRECONDITION(cur.ptr && cur.len);
AWS_PRECONDITION(aws_byte_cursor_is_valid(&cur));

/* Make sure the buffer isn't too big */
if (cur.len > UINT16_MAX) {
Expand Down Expand Up @@ -584,7 +584,7 @@ int aws_mqtt_packet_publish_decode(struct aws_byte_cursor *cur, struct aws_mqtt_
/*************************************************************************/
/* Payload */
packet->payload = aws_byte_cursor_advance(cur, payload_size);
if (packet->payload.len == 0) {
if (packet->payload.len != payload_size) {
return aws_raise_error(AWS_ERROR_SHORT_BUFFER);
}

Expand Down
2 changes: 2 additions & 0 deletions tests/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ add_test_case(mqtt_packet_suback)
add_test_case(mqtt_packet_unsuback)
add_test_case(mqtt_packet_connect)
add_test_case(mqtt_packet_connect_will)
add_test_case(mqtt_packet_connect_empty_payload_will)
add_test_case(mqtt_packet_connect_password)
add_test_case(mqtt_packet_connack)
add_test_case(mqtt_packet_publish_qos0_dup)
add_test_case(mqtt_packet_publish_qos2_retain)
add_test_case(mqtt_packet_publish_empty_payload)
add_test_case(mqtt_packet_subscribe)
add_test_case(mqtt_packet_unsubscribe)
add_test_case(mqtt_packet_pingreq)
Expand Down
87 changes: 87 additions & 0 deletions tests/packet_encoding_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,57 @@ static int s_test_connect_will_init(struct packet_test_fixture *fixture) {
}
PACKET_TEST_NAME(CONNECT, connect_will, connect, &s_test_connect_will_init, NULL, &s_test_connect_eq)

static uint8_t s_empty_payload[] = "";
enum { EMPTY_PAYLOAD_LEN = 0 };

static int s_test_connect_empty_payload_will_init(struct packet_test_fixture *fixture) {
/* Init packet */
ASSERT_SUCCESS(aws_mqtt_packet_connect_init(
fixture->in_packet, aws_byte_cursor_from_array(s_client_id, CLIENT_ID_LEN), false, 0));
ASSERT_SUCCESS(aws_mqtt_packet_connect_add_will(
fixture->in_packet,
aws_byte_cursor_from_array(s_topic_name, TOPIC_NAME_LEN),
AWS_MQTT_QOS_EXACTLY_ONCE,
true /*retain*/,
aws_byte_cursor_from_array(s_empty_payload, EMPTY_PAYLOAD_LEN)));

/* Init buffer */
/* clang-format off */
uint8_t header[] = {
AWS_MQTT_PACKET_CONNECT << 4, /* Packet type */
10 + (2 + CLIENT_ID_LEN) + (2 + TOPIC_NAME_LEN) + (2 + EMPTY_PAYLOAD_LEN), /* Remaining length */
0, 4, 'M', 'Q', 'T', 'T', /* Protocol name */
4, /* Protocol level */
/* Connect Flags: */
(1 << 2) /* Will flag, bit 2 */
| (AWS_MQTT_QOS_EXACTLY_ONCE << 3)/* Will QoS, bits 4-3 */
| (1 << 5), /* Will Retain, bit 5 */

0, 0, /* Keep alive */
};
/* clang-format on */

aws_byte_buf_write(&fixture->buffer, header, sizeof(header));
/* client identifier */
aws_byte_buf_write_be16(&fixture->buffer, CLIENT_ID_LEN);
aws_byte_buf_write(&fixture->buffer, s_client_id, CLIENT_ID_LEN);
/* will topic */
aws_byte_buf_write_be16(&fixture->buffer, TOPIC_NAME_LEN);
aws_byte_buf_write(&fixture->buffer, s_topic_name, TOPIC_NAME_LEN);
/* will payload */
aws_byte_buf_write_be16(&fixture->buffer, EMPTY_PAYLOAD_LEN);
aws_byte_buf_write(&fixture->buffer, s_empty_payload, EMPTY_PAYLOAD_LEN);

return AWS_OP_SUCCESS;
}
PACKET_TEST_NAME(
CONNECT,
connect_empty_payload_will,
connect,
&s_test_connect_empty_payload_will_init,
NULL,
&s_test_connect_eq)

static int s_test_connect_password_init(struct packet_test_fixture *fixture) {
/* Init packet */
ASSERT_SUCCESS(aws_mqtt_packet_connect_init(
Expand Down Expand Up @@ -440,6 +491,41 @@ static int s_test_publish_qos2_retain_init(struct packet_test_fixture *fixture)
return AWS_OP_SUCCESS;
}

static int s_test_publish_empty_payload_init(struct packet_test_fixture *fixture) {

/* Init packet */
ASSERT_SUCCESS(aws_mqtt_packet_publish_init(
fixture->in_packet,
false /* retain */,
AWS_MQTT_QOS_AT_MOST_ONCE,
true /* dup */,
aws_byte_cursor_from_array(s_topic_name, TOPIC_NAME_LEN),
0,
aws_byte_cursor_from_array(s_empty_payload, EMPTY_PAYLOAD_LEN)));

/* Init buffer */
/* clang-format off */
aws_byte_buf_write_u8(
&fixture->buffer,
(AWS_MQTT_PACKET_PUBLISH << 4) /* Packet type bits 7-4 */
| (1 << 3) /* DUP bit 3 */
| (AWS_MQTT_QOS_AT_MOST_ONCE << 1) /* QoS bits 2-1 */
| 0 /* RETAIN bit 0 */);
aws_byte_buf_write_u8(
&fixture->buffer, 2 + TOPIC_NAME_LEN + EMPTY_PAYLOAD_LEN); /* Remaining length */
aws_byte_buf_write_u8(
&fixture->buffer, 0); /* Topic name len byte 1 */
aws_byte_buf_write_u8(
&fixture->buffer, TOPIC_NAME_LEN); /* Topic name len byte 2 */
aws_byte_buf_write(
&fixture->buffer, s_topic_name, TOPIC_NAME_LEN); /* Topic name */
aws_byte_buf_write(
&fixture->buffer, s_empty_payload, EMPTY_PAYLOAD_LEN); /* payload */
/* clang-format on */

return AWS_OP_SUCCESS;
}

static bool s_test_publish_eq(void *a, void *b, size_t size) {

(void)size;
Expand All @@ -452,6 +538,7 @@ static bool s_test_publish_eq(void *a, void *b, size_t size) {
}
PACKET_TEST_NAME(PUBLISH, publish_qos0_dup, publish, &s_test_publish_qos0_dup_init, NULL, &s_test_publish_eq)
PACKET_TEST_NAME(PUBLISH, publish_qos2_retain, publish, &s_test_publish_qos2_retain_init, NULL, &s_test_publish_eq)
PACKET_TEST_NAME(PUBLISH, publish_empty_payload, publish, &s_test_publish_empty_payload_init, NULL, &s_test_publish_eq)

/*****************************************************************************/
/* Subscribe */
Expand Down

0 comments on commit 60f9a17

Please sign in to comment.