Skip to content

Commit

Permalink
Fix AT_CellularSMS::list_messages breaking in text mode when CRLF is …
Browse files Browse the repository at this point in the history
…contained in SMS payload text

When parsing SMS, it can happen that we receive CRLF in the SMS payload (happened to me when receiving provider texts).
As an example, we can receive:

"""
Hello <CR><LF>
World!
"""

With previous implementation, second consume_to_stop_tag was stopping in <CR><LF> and rest of the code was failing for obvious reasons.
With this commit we consume the full payload as bytes.
  • Loading branch information
davidalo committed May 11, 2024
1 parent d676084 commit 9e7e22d
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 4 deletions.
13 changes: 13 additions & 0 deletions connectivity/cellular/include/cellular/framework/API/ATHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,13 @@ class ATHandler {
*/
void skip_param(ssize_t len, uint32_t count);

/** Consumes the given length from the reading buffer even if the stop tag has been found
*
* @param len length to be consumed from reading buffer
* @param count number of parameters to be skipped
*/
void skip_param_bytes(ssize_t len, uint32_t count);

/** Reads given number of bytes from receiving buffer without checking any subparameter delimiters, such as comma.
*
* @param buf output buffer for the read
Expand Down Expand Up @@ -408,6 +415,12 @@ class ATHandler {
*/
bool consume_to_stop_tag();

/** Consumes the received content until current stop tag is found even if stop tag has been found previously
*
* @return true if stop tag is found, false otherwise
*/
bool consume_to_stop_tag_even_found();

/** Return the last 3GPP error code.
* @return last 3GPP error code
*/
Expand Down
15 changes: 13 additions & 2 deletions connectivity/cellular/source/framework/AT/AT_CellularSMS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1036,6 +1036,7 @@ nsapi_error_t AT_CellularSMS::list_messages()
int index = 0;
int length = 0;
char *pdu = NULL;
char buffer[32]; // 32 > SMS_STATUS_SIZE, SMS_MAX_PHONE_NUMBER_SIZE, SMS_MAX_TIME_STAMP_SIZE

_at.resp_start("+CMGL:");
while (_at.info_resp()) {
Expand All @@ -1058,8 +1059,18 @@ nsapi_error_t AT_CellularSMS::list_messages()
// +CMGL: <index>,<stat>,<oa/da>,[<alpha>],[<scts>][,<tooa/toda>,<length>]<CR><LF><data>[<CR><LF>
// +CMGL: <index>,<stat>,<da/oa>,[<alpha>],[<scts>][,<tooa/toda>,<length>]<CR><LF><data>[...]]
index = _at.read_int();
(void)_at.consume_to_stop_tag(); // consume until <CR><LF>
(void)_at.consume_to_stop_tag(); // consume until <CR><LF>
_at.read_string(buffer, SMS_STATUS_SIZE);
_at.read_string(buffer, SMS_MAX_PHONE_NUMBER_SIZE);
_at.skip_param(); // <alpha>
_at.read_string(buffer, SMS_MAX_TIME_STAMP_SIZE);
_at.read_int();
int size = _at.read_int(); // length
_at.consume_to_stop_tag(); // consume until <CR><LF> end of header
if (size > 0) {
// we can not use skip param because we already consumed stop tag
_at.skip_param_bytes(size, 1);
}
_at.consume_to_stop_tag_even_found(); // consume until <CR><LF> -> data
}

if (index >= 0) {
Expand Down
40 changes: 40 additions & 0 deletions connectivity/cellular/source/framework/device/ATHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,26 @@ void ATHandler::skip_param(ssize_t len, uint32_t count)
return;
}

void ATHandler::skip_param_bytes(ssize_t len, uint32_t count)
{
if (!ok_to_proceed()) {
return;
}

for (uint32_t i = 0; i < count; i++) {
ssize_t read_len = 0;
while (read_len < len) {
int c = get_char();
if (c == -1) {
set_error(NSAPI_ERROR_DEVICE_ERROR);
return;
}
read_len++;
}
}
return;
}

ssize_t ATHandler::read_bytes(uint8_t *buf, size_t len)
{
if (!ok_to_proceed()) {
Expand Down Expand Up @@ -1093,6 +1113,26 @@ bool ATHandler::consume_to_stop_tag()
return false;
}


bool ATHandler::consume_to_stop_tag_even_found()
{
if (_error_found) {
return true;
}

if (!_is_fh_usable) {
_last_err = NSAPI_ERROR_BUSY;
return true;
}

if (consume_to_tag((const char *)_stop_tag->tag, true)) {
return true;
}

clear_error();
return false;
}

// consume by size needed?

void ATHandler::resp_stop()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ void ATHandler::skip_param(ssize_t len, uint32_t count)
{
}

void ATHandler::skip_param_bytes(ssize_t len, uint32_t count)
{
}

ssize_t ATHandler::read_bytes(uint8_t *buf, size_t len)
{
return ATHandler_stub::ssize_value;
Expand Down Expand Up @@ -301,6 +305,11 @@ bool ATHandler::consume_to_stop_tag()
return ATHandler_stub::bool_value;
}

bool ATHandler::consume_to_stop_tag_even_found()
{
return ATHandler_stub::bool_value;
}

void ATHandler::resp_stop()
{
if (ATHandler_stub::resp_stop_success_count > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,16 @@ TEST_F(TestAT_CellularSMS, test_AT_CellularSMS_get_sms)
ATHandler_stub::resp_info_false_counter = 1;
ATHandler_stub::resp_info_true_counter2 = 2;
ATHandler_stub::int_value = 11;
ATHandler_stub::read_string_index = 4;
ATHandler_stub::read_string_table[4] = "";
ATHandler_stub::read_string_index = (3 * 2) + (2 * 2); // 3 read_string in list_messages + 2 read_string in read_sms_from_index
ATHandler_stub::read_string_table[10] = "";
// list_messages
ATHandler_stub::read_string_table[9] = "1"; // status
ATHandler_stub::read_string_table[8] = "+00611223344"; // phone
ATHandler_stub::read_string_table[7] = "24/12/12,11:15:00+04"; // timestamp
ATHandler_stub::read_string_table[6] = "1"; // status
ATHandler_stub::read_string_table[5] = "+00611223344"; // phone
ATHandler_stub::read_string_table[4] = "24/12/12,11:15:00+04"; // timestamp
// read_sms_from_index
ATHandler_stub::read_string_table[3] = "REC READ";
ATHandler_stub::read_string_table[2] = "09/01/12,11:15:00+04";
ATHandler_stub::read_string_table[1] = "REC READ";
Expand Down

0 comments on commit 9e7e22d

Please sign in to comment.