Skip to content

Commit

Permalink
fix: {io} logs (#162)
Browse files Browse the repository at this point in the history 
All ERROR messages were being reported as {io} errors
  • Loading branch information
@zfields
zfields authored Jan 23, 2025
1 parent 5b1edf7 commit 23c2338
Show file tree
Hide file tree
Showing 2 changed files with 184 additions and 69 deletions.
32 changes: 22 additions & 10 deletions n_request.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ static uint16_t lastRequestSeqno = 0;
#define CRC_FIELD_LENGTH 22 // ,"crc":"SSSS:CCCCCCCC"
#define CRC_FIELD_NAME_OFFSET 1
#define CRC_FIELD_NAME_TEST "\"crc\":\""
#define ERR_FIELD_NAME_TEST "\"err\":\""
NOTE_C_STATIC int32_t _crc32(const void* data, size_t length);
NOTE_C_STATIC char * _crcAdd(char *json, uint16_t seqno);
NOTE_C_STATIC bool _crcError(char *json, uint16_t shouldBeSeqno);

static bool notecardSupportsCrc = false;
NOTE_C_STATIC bool notecardFirmwareSupportsCrc = false;
#endif

/*!
Expand Down Expand Up @@ -930,27 +931,38 @@ NOTE_C_STATIC char *_crcAdd(char *json, uint16_t seqno)
*/
NOTE_C_STATIC bool _crcError(char *json, uint16_t shouldBeSeqno)
{
// Strip off any crlf for crc calculation
// Trim whitespace (specifically "\r\n") for CRC calculation
size_t jsonLen = strlen(json);
while (jsonLen > 0 && json[jsonLen-1] <= ' ') {
while (jsonLen > 0 && json[jsonLen - 1] <= ' ') {
jsonLen--;
}

// Minimum valid JSON is "{}" (2 bytes) and must end with a closing "}".
if (jsonLen < CRC_FIELD_LENGTH+2 || json[jsonLen-1] != '}') {
// Ignore CRC check if the response contains an error
// A valid JSON string begins with "{". Therefore, the presence of an error
// message in a well-formed JSON string MUST result in a non-zero response.
if (strstr(json, ERR_FIELD_NAME_TEST)) {
return false;
}

// Skip if invalid JSON or is too short to contain a CRC parameter
// Minimum length is "{}" (2 bytes) + CRC_FIELD_LENGTH
// Valid JSON ends with a closing "}"
if ((jsonLen < (CRC_FIELD_LENGTH + 2)) || (json[jsonLen - 1] != '}')) {
return false;
}

// See if it has a compliant CRC field
size_t fieldOffset = ((jsonLen-1) - CRC_FIELD_LENGTH);
if (memcmp(&json[fieldOffset+CRC_FIELD_NAME_OFFSET], CRC_FIELD_NAME_TEST, sizeof(CRC_FIELD_NAME_TEST)-1) != 0) {
size_t fieldOffset = ((jsonLen - 1) - CRC_FIELD_LENGTH);
if (memcmp(&json[fieldOffset + CRC_FIELD_NAME_OFFSET], CRC_FIELD_NAME_TEST, ((sizeof(CRC_FIELD_NAME_TEST) - 1)) != 0)) {
// If we've seen a CRC before, we should see one every time
return notecardSupportsCrc ? true : false;
return notecardFirmwareSupportsCrc;
}

// If we get here, we've seen at least one CRC from the Notecard, so we should expect it.
notecardSupportsCrc = true;
char *p = &json[fieldOffset + CRC_FIELD_NAME_OFFSET + (sizeof(CRC_FIELD_NAME_TEST)-1)];
notecardFirmwareSupportsCrc = true;

// Extract the CRC field and the sequence number
char *p = &json[fieldOffset + CRC_FIELD_NAME_OFFSET + (sizeof(CRC_FIELD_NAME_TEST) - 1)];
uint16_t actualSeqno = (uint16_t) _n_atoh(p, 4);
uint32_t actualCrc32 = (uint32_t) _n_atoh(p+5, 8);
json[fieldOffset++] = '}';
Expand Down
221 changes: 162 additions & 59 deletions test/src/_crcError_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,72 +17,175 @@

#include "test_static.h"

namespace
{
extern bool notecardFirmwareSupportsCrc;

SCENARIO("_crcError")
namespace
{
NoteSetFnDefault(malloc, free, NULL, NULL);

uint16_t seqNo = 1;

SECTION("Empty string") {
char json[] = "";

CHECK(!_crcError(json, seqNo));
}

SECTION("Invalid JSON") {
char json[] = "{\"req\":";

CHECK(!_crcError(json, seqNo));
}

SECTION("No CRC field") {
char json[] = "{\"req\": \"hub.sync\"}";

CHECK(!_crcError(json, seqNo));
}

SECTION("CRC field at unexpected position") {
char json[] = "{\"crc\":\"0009:10BAC79A\",\"req\": \"hub.sync\"}";

CHECK(!_crcError(json, seqNo));
}

SECTION("Valid JSON and CRC field present") {

SECTION("CRC doesn't match") {
char json[] = "{\"req\":\"hub.sync\",\"crc\":\"0001:DEADBEEF\"}";

CHECK(_crcError(json, seqNo));
}

SECTION("Sequence number doesn't match") {
char json[] = "{\"req\":\"hub.sync\",\"crc\":\"0009:10BAC79A\"}";

CHECK(_crcError(json, seqNo));
SCENARIO("_crcError")
{
GIVEN("The Notecard firmware does NOT support CRC") {
notecardFirmwareSupportsCrc = false;

NoteSetFnDefault(malloc, free, NULL, NULL);

uint16_t seqNo = 1;

AND_GIVEN("An empty string")
{
char json[] = "";

THEN("A CRC error SHALL NOT be reported")
{
CHECK(!_crcError(json, seqNo));
}
}

AND_GIVEN("Invalid JSON")
{
char json[] = "{\"req\":";

THEN("A CRC error SHALL NOT be reported")
{
CHECK(!_crcError(json, seqNo));
}
}

AND_GIVEN("The Notecard returns an error message") {
char json[] = "{\"err\":\"cannot interpret JSON: bool being placed into a non-bool field {io}\"}";

THEN("A CRC error SHALL NOT be reported")
{
CHECK(!_crcError(json, seqNo));
}
}

AND_GIVEN("No CRC field")
{
char json[] = "{\"req\": \"hub.sync\"}";

THEN("A CRC error SHALL NOT be reported")
{
CHECK(!_crcError(json, seqNo));
}
}

AND_GIVEN("The CRC field exists, but is not at the tail of the response")
{
char json[] = "{\"crc\":\"0009:10BAC79A\",\"req\": \"hub.sync\"}";

THEN("A CRC error SHALL NOT be reported")
{
CHECK(!_crcError(json, seqNo));
}
}
}

SECTION("Everything matches") {
char json[] = "{\"req\":\"hub.sync\"}";
char *jsonWithCrc = _crcAdd(json, seqNo);
REQUIRE(jsonWithCrc != NULL);

CHECK(!_crcError(jsonWithCrc, seqNo));

NoteFree(jsonWithCrc);
}

SECTION("Trailing CRLF") {
char json[] = "{\"req\":\"hub.sync\",\"crc\":\"0001:10BAC79A\"}\r\n";

// Trailing \r\n should be ignored.
CHECK(!_crcError(json, seqNo));
GIVEN("The Notecard firmware supports CRC") {
notecardFirmwareSupportsCrc = true;

NoteSetFnDefault(malloc, free, NULL, NULL);

uint16_t seqNo = 1;

AND_GIVEN("An empty string")
{
char json[] = "";

THEN("A CRC error SHALL NOT be reported")
{
CHECK(!_crcError(json, seqNo));
}
}

AND_GIVEN("Invalid JSON")
{
char json[] = "{\"req\":";

THEN("A CRC error SHALL NOT be reported")
{
CHECK(!_crcError(json, seqNo));
}
}

AND_GIVEN("The Notecard returns an error message") {
char json[] = "{\"err\":\"cannot interpret JSON: bool being placed into a non-bool field {io}\"}";

THEN("A CRC error SHALL NOT be reported")
{
CHECK(!_crcError(json, seqNo));
}
}

AND_GIVEN("No CRC field")
{
char json[] = "{\"req\": \"hub.sync\"}";

THEN("A CRC error SHALL NOT be reported")
{
CHECK(!_crcError(json, seqNo));
}
}

AND_GIVEN("The CRC field exists, but is not at the tail of the response")
{
char json[] = "{\"crc\":\"0009:10BAC79A\",\"req\": \"hub.sync\"}";

THEN("A CRC error SHALL be reported")
{
CHECK(_crcError(json, seqNo));
}
}

AND_GIVEN("Valid JSON and CRC field present")
{
WHEN("CRC doesn't match")
{
char json[] = "{\"req\":\"hub.sync\",\"crc\":\"0001:DEADBEEF\"}";

THEN("A CRC error SHALL be reported")
{
CHECK(_crcError(json, seqNo));
}
}

WHEN("Sequence number doesn't match")
{
char json[] = "{\"req\":\"hub.sync\",\"crc\":\"0009:10BAC79A\"}";

THEN("A CRC error SHALL be reported")
{
CHECK(_crcError(json, seqNo));
}
}

WHEN("Everything matches")
{
char json[] = "{\"req\":\"hub.sync\"}";
char *jsonWithCrc = _crcAdd(json, seqNo);
REQUIRE(jsonWithCrc != NULL);

THEN("A CRC error SHALL NOT be reported")
{
CHECK(!_crcError(json, seqNo));
}

NoteFree(jsonWithCrc);
}

AND_GIVEN("a trailing CRLF")
{
char json[] = "{\"req\":\"hub.sync\",\"crc\":\"0001:10BAC79A\"}\r\n";

THEN("A CRC error SHALL NOT be reported")
{
// Trailing \r\n should be ignored.
CHECK(!_crcError(json, seqNo));
}
}
}
}
}
}

}

#endif // !NOTE_C_LOW_MEM

0 comments on commit 23c2338

Please sign in to comment.