espressif/openthread
1
0
Fork 0
mirror of https://github.com/espressif/openthread.git synced 2026-08-09 20:27:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

[hdlc] report frames smaller than FCS size as error frame (#3395)

Browse Source 
This commit changes the `Hdlc::Decoder` implementation to report
non-zero length received frames shorter than FCS size as an error
`OT_ERROR_PARSE` (instead of ignoring such frames during decoding).
Since the recent changes in `Hdlc::Decoder` buffer model delegates
the management of the frame buffer to the user of `Decoder`,
clearing frame buffer after an error should be be performed by the
`Decoder` user from the `FrameHandler` callback. The change in this
commit addresses an issue where a small (single-byte) frame would
not have been cleared from the decoder buffer and included in the
next frame. This commit also updates the `test_hdlc` unit test to
cover small frame error.
This commit is contained in:
Abtin Keshavarzian
2018-12-24 15:12:16 -08:00
committed by Jonathan Hui
parent 52c06c6858
commit 7dc645a856
2 changed files with 41 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/ncp/hdlc.cpp
+6 -9
View File
@@ -69,6 +69,7 @@ enum
{
kInitFcs = 0xffff, ///< Initial FCS value.
kGoodFcs = 0xf0b8, ///< Good FCS value.
kFcsSize = 2, ///< FCS size (number of bytes).
};
uint16_t UpdateFcs(uint16_t aFcs, uint8_t aByte)
@@ -237,19 +238,15 @@ void Decoder::Decode(const uint8_t *aData, uint16_t aLength)
case kFlagSequence:
// Ignore frames which are smaller than the size of the FCS.
if (mDecodedLength > sizeof(uint16_t))
if (mDecodedLength > 0)
{
otError error = OT_ERROR_NONE;
otError error = OT_ERROR_PARSE;
if (mFcs == kGoodFcs)
if ((mDecodedLength >= kFcsSize) && (mFcs == kGoodFcs))
{
// Remove the FCS from the frame.
mWritePointer.UndoLastWrites(sizeof(uint16_t));
}
else
{
error = OT_ERROR_PARSE;
mWritePointer.UndoLastWrites(kFcsSize);
error = OT_ERROR_NONE;
}
mFrameHandler(mContext, error);
tests/unit/test_hdlc.cpp
+35 -2
View File
@@ -360,6 +360,7 @@ void TestEncoderDecoder(void)
Hdlc::Decoder decoder(decoderBuffer, ProcessDecodedFrame, &decoderContext);
uint8_t * frame;
uint16_t length;
uint8_t badShortFrame[3] = {kFlagSequence, 0xaa, kFlagSequence};
printf("Testing Hdlc::Encoder and Hdlc::Decoder");
@@ -383,13 +384,18 @@ void TestEncoderDecoder(void)
SuccessOrQuit(encoder.EndFrame(), "Encoder::EndFrame() failed");
encoderBuffer.SaveFrame();
SuccessOrQuit(encoder.BeginFrame(), "Encoder::BeginFrame() failed");
// Empty frame
SuccessOrQuit(encoder.EndFrame(), "Encoder::EndFrame() failed");
encoderBuffer.SaveFrame();
byte = kFlagSequence;
SuccessOrQuit(encoder.BeginFrame(), "Encoder::BeginFrame() failed");
SuccessOrQuit(encoder.Encode(&byte, sizeof(uint8_t)), "Encoder::Encode() failed");
SuccessOrQuit(encoder.EndFrame(), "Encoder::EndFrame() failed");
encoderBuffer.SaveFrame();
// Feed the encoded frame to decoder and saved the content
// Feed the encoded frames to decoder and save the content
while (encoderBuffer.ReadSavedFrame(frame, length) == OT_ERROR_NONE)
{
decoderContext.mWasCalled = false;
@@ -402,7 +408,7 @@ void TestEncoderDecoder(void)
decoderBuffer.SaveFrame();
}
// Verify the decoded frame match the original frames
// Verify the decoded frames match the original frames
SuccessOrQuit(decoderBuffer.ReadSavedFrame(frame, length), "Incorrect decoded frame");
VerifyOrQuit(length == sizeof(sOpenThreadText) - 1, "Decoded frame length does not match original frame");
@@ -420,6 +426,9 @@ void TestEncoderDecoder(void)
VerifyOrQuit(length == sizeof(sHelloText) - 1, "Decoded frame length does not match original frame");
VerifyOrQuit(memcmp(frame, sHelloText, length) == 0, "Decoded frame content does not match original frame");
SuccessOrQuit(decoderBuffer.ReadSavedFrame(frame, length), "Incorrect decoded frame");
VerifyOrQuit(length == 0, "Decoded frame length does not match original frame");
SuccessOrQuit(decoderBuffer.ReadSavedFrame(frame, length), "Incorrect decoded frame");
VerifyOrQuit(length == sizeof(uint8_t), "Decoded frame length does not match original frame");
VerifyOrQuit(*frame == kFlagSequence, "Decoded frame content does not match original frame");
@@ -459,6 +468,30 @@ void TestEncoderDecoder(void)
VerifyOrQuit(decoderContext.mWasCalled, "Decoder::Decode() failed");
VerifyOrQuit(decoderContext.mError == OT_ERROR_PARSE, "Decoder::Decode() did not fail with bad FCS");
decoderBuffer.Clear();
// Test `Decoder` behavior with short frame (smaller than FCS)
decoderContext.mWasCalled = false;
decoder.Decode(badShortFrame, sizeof(badShortFrame));
VerifyOrQuit(decoderContext.mWasCalled, "Decoder::Decode() failed");
VerifyOrQuit(decoderContext.mError == OT_ERROR_PARSE, "Decoder::Decode() did not fail for short frame");
decoderBuffer.Clear();
// Test `Decoder` with back to back `kFlagSequence` and ensure callback is not invoked.
byte = kFlagSequence;
decoderContext.mWasCalled = false;
decoder.Decode(&byte, sizeof(uint8_t));
VerifyOrQuit(!decoderContext.mWasCalled, "Decoder::Decode() failed");
decoder.Decode(&byte, sizeof(uint8_t));
VerifyOrQuit(!decoderContext.mWasCalled, "Decoder::Decode() failed");
decoder.Decode(&byte, sizeof(uint8_t));
VerifyOrQuit(!decoderContext.mWasCalled, "Decoder::Decode() failed");
decoder.Decode(&byte, sizeof(uint8_t));
VerifyOrQuit(!decoderContext.mWasCalled, "Decoder::Decode() failed");
printf(" -- PASS\n");
}