Skip to content

Commit c275ffa

Browse files
committed
Fix AT_CellularSMS::list_messages breaking in text mode when CRLF is 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.
1 parent d676084 commit c275ffa

File tree

5 files changed

+85
-4
lines changed

5 files changed

+85
-4
lines changed

connectivity/cellular/include/cellular/framework/API/ATHandler.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,13 @@ class ATHandler {
318318
*/
319319
void skip_param(ssize_t len, uint32_t count);
320320

321+
/** Consumes the given length from the reading buffer even if the stop tag has been found
322+
*
323+
* @param len length to be consumed from reading buffer
324+
* @param count number of parameters to be skipped
325+
*/
326+
void skip_param_bytes(ssize_t len, uint32_t count);
327+
321328
/** Reads given number of bytes from receiving buffer without checking any subparameter delimiters, such as comma.
322329
*
323330
* @param buf output buffer for the read
@@ -408,6 +415,12 @@ class ATHandler {
408415
*/
409416
bool consume_to_stop_tag();
410417

418+
/** Consumes the received content until current stop tag is found even if stop tag has been found previously
419+
*
420+
* @return true if stop tag is found, false otherwise
421+
*/
422+
bool consume_to_stop_tag_even_found();
423+
411424
/** Return the last 3GPP error code.
412425
* @return last 3GPP error code
413426
*/

connectivity/cellular/source/framework/AT/AT_CellularSMS.cpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,6 +1036,7 @@ nsapi_error_t AT_CellularSMS::list_messages()
10361036
int index = 0;
10371037
int length = 0;
10381038
char *pdu = NULL;
1039+
char buffer[32]; // 32 > SMS_STATUS_SIZE, SMS_MAX_PHONE_NUMBER_SIZE, SMS_MAX_TIME_STAMP_SIZE
10391040

10401041
_at.resp_start("+CMGL:");
10411042
while (_at.info_resp()) {
@@ -1058,8 +1059,18 @@ nsapi_error_t AT_CellularSMS::list_messages()
10581059
// +CMGL: <index>,<stat>,<oa/da>,[<alpha>],[<scts>][,<tooa/toda>,<length>]<CR><LF><data>[<CR><LF>
10591060
// +CMGL: <index>,<stat>,<da/oa>,[<alpha>],[<scts>][,<tooa/toda>,<length>]<CR><LF><data>[...]]
10601061
index = _at.read_int();
1061-
(void)_at.consume_to_stop_tag(); // consume until <CR><LF>
1062-
(void)_at.consume_to_stop_tag(); // consume until <CR><LF>
1062+
_at.read_string(buffer, SMS_STATUS_SIZE);
1063+
_at.read_string(buffer, SMS_MAX_PHONE_NUMBER_SIZE);
1064+
_at.skip_param(); // <alpha>
1065+
_at.read_string(buffer, SMS_MAX_TIME_STAMP_SIZE);
1066+
_at.read_int();
1067+
int size = _at.read_int(); // length
1068+
_at.consume_to_stop_tag(); // consume until <CR><LF> end of header
1069+
if (size > 0) {
1070+
// we can not use skip param because we already consumed stop tag
1071+
_at.skip_param_bytes(size, 1);
1072+
}
1073+
_at.consume_to_stop_tag_even_found(); // consume until <CR><LF> -> data
10631074
}
10641075

10651076
if (index >= 0) {

connectivity/cellular/source/framework/device/ATHandler.cpp

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,26 @@ void ATHandler::skip_param(ssize_t len, uint32_t count)
484484
return;
485485
}
486486

487+
void ATHandler::skip_param_bytes(ssize_t len, uint32_t count)
488+
{
489+
if (!ok_to_proceed()) {
490+
return;
491+
}
492+
493+
for (uint32_t i = 0; i < count; i++) {
494+
ssize_t read_len = 0;
495+
while (read_len < len) {
496+
int c = get_char();
497+
if (c == -1) {
498+
set_error(NSAPI_ERROR_DEVICE_ERROR);
499+
return;
500+
}
501+
read_len++;
502+
}
503+
}
504+
return;
505+
}
506+
487507
ssize_t ATHandler::read_bytes(uint8_t *buf, size_t len)
488508
{
489509
if (!ok_to_proceed()) {
@@ -1093,6 +1113,26 @@ bool ATHandler::consume_to_stop_tag()
10931113
return false;
10941114
}
10951115

1116+
1117+
bool ATHandler::consume_to_stop_tag_even_found()
1118+
{
1119+
if (_error_found) {
1120+
return true;
1121+
}
1122+
1123+
if (!_is_fh_usable) {
1124+
_last_err = NSAPI_ERROR_BUSY;
1125+
return true;
1126+
}
1127+
1128+
if (consume_to_tag((const char *)_stop_tag->tag, true)) {
1129+
return true;
1130+
}
1131+
1132+
clear_error();
1133+
return false;
1134+
}
1135+
10961136
// consume by size needed?
10971137

10981138
void ATHandler::resp_stop()

connectivity/cellular/tests/UNITTESTS/doubles/ATHandler_stub.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,10 @@ void ATHandler::skip_param(ssize_t len, uint32_t count)
201201
{
202202
}
203203

204+
void ATHandler::skip_param_bytes(ssize_t len, uint32_t count)
205+
{
206+
}
207+
204208
ssize_t ATHandler::read_bytes(uint8_t *buf, size_t len)
205209
{
206210
return ATHandler_stub::ssize_value;
@@ -301,6 +305,11 @@ bool ATHandler::consume_to_stop_tag()
301305
return ATHandler_stub::bool_value;
302306
}
303307

308+
bool ATHandler::consume_to_stop_tag_even_found()
309+
{
310+
return ATHandler_stub::bool_value;
311+
}
312+
304313
void ATHandler::resp_stop()
305314
{
306315
if (ATHandler_stub::resp_stop_success_count > 0) {

connectivity/cellular/tests/UNITTESTS/framework/AT/at_cellularsms/at_cellularsmstest.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,16 @@ TEST_F(TestAT_CellularSMS, test_AT_CellularSMS_get_sms)
155155
ATHandler_stub::resp_info_false_counter = 1;
156156
ATHandler_stub::resp_info_true_counter2 = 2;
157157
ATHandler_stub::int_value = 11;
158-
ATHandler_stub::read_string_index = 4;
159-
ATHandler_stub::read_string_table[4] = "";
158+
ATHandler_stub::read_string_index = (3*2) + (2*2); // 3 read_string in list_messages + 2 read_string in read_sms_from_index
159+
ATHandler_stub::read_string_table[10] = "";
160+
// list_messages
161+
ATHandler_stub::read_string_table[9] = "1"; // status
162+
ATHandler_stub::read_string_table[8] = "+00611223344"; // phone
163+
ATHandler_stub::read_string_table[7] = "24/12/12,11:15:00+04"; // timestamp
164+
ATHandler_stub::read_string_table[6] = "1"; // status
165+
ATHandler_stub::read_string_table[5] = "+00611223344"; // phone
166+
ATHandler_stub::read_string_table[4] = "24/12/12,11:15:00+04"; // timestamp
167+
// read_sms_from_index
160168
ATHandler_stub::read_string_table[3] = "REC READ";
161169
ATHandler_stub::read_string_table[2] = "09/01/12,11:15:00+04";
162170
ATHandler_stub::read_string_table[1] = "REC READ";

0 commit comments

Comments
 (0)