Skip to content

fix: COBS code-byte count: ceil(u/254) vs floor(u/254) + 1 #222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rayozzie wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: COBS code-byte count: ceil(u/254) vs floor(u/254) + 1 #222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 45 additions & 35 deletions n_cobs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,20 +244,26 @@ uint32_t _cobsEncodedLength(const uint8_t *ptr, uint32_t length)

@param length Length of the data to encode

@return The max length required to encode the data
@return The max length required to encode the data (including EOP byte)

@note Worst case is input with no zero bytes (no EOP markers to replace).
The COBS encoder always emits 1 initial code byte, then 1 additional
code byte for every 254 data bytes processed:

codeBytes = floor(length / 254) + 1

IMPORTANT: This is NOT ceil(length / 254). They differ at exact
multiples of 254. Example: encoding 254 non-zero bytes produces
code byte 0xFF, 254 data bytes, then a final code byte 0x01.
That's 2 code bytes, but ceil(254/254) = 1 (wrong).

@note Since the contents of the buffer are unknown, then we must assume
that the entire buffer has no end-of-packet markers. This would
require the injection of overhead bytes (as opposed to the
replacement of end-of-packet markers with overhead bytes) at
intervals of 255, thus producing the worst case scenario.
@note An additional byte is added for the EOP (end-of-packet) marker.
*/
/**************************************************************************/
uint32_t _cobsEncodedMaxLength(uint32_t length)
{
const uint32_t overheadBytes = (length == 0) + ((length != 0) * ((length / COBS_MAX_PACKET_SIZE) + ((length % COBS_MAX_PACKET_SIZE) > 0)));
return (length + overheadBytes + COBS_EOP_OVERHEAD);
const uint32_t codeBytes = (length / COBS_MAX_PACKET_SIZE) + 1;
return (length + codeBytes + COBS_EOP_OVERHEAD);
}

//**************************************************************************/
Expand All @@ -267,36 +273,40 @@ uint32_t _cobsEncodedMaxLength(uint32_t length)

@param bufLen Length of the buffer in bytes

@return the length of unencoded data
@return the length of unencoded data that is guaranteed to fit when
COBS-encoded into bufLen bytes (including EOP)

@note The COBS encoder always emits 1 initial code byte, then 1 additional
code byte for every 254 data bytes. Therefore:

codeBytes(u) = floor(u / 254) + 1

IMPORTANT: This is NOT ceil(u / 254). They differ at exact multiples
of 254. Example: encoding 254 non-zero bytes produces code 0xFF,
254 data bytes, then final code 0x01 — that's 2 code bytes, but
ceil(254/254) = 1 (wrong).

@note An additional byte for the EOP (end-of-packet) marker is assumed.
Buffer requirement: bufLen >= u + floor(u/254) + 1 + 1(EOP)
= u + floor(u/254) + 2

Inversion: find max u where u + floor(u/254) <= t, with t = bufLen-2.
Substituting u = 254q + r (0 <= r <= 253): 255q + r <= t.

Closed form: u = t - floor((t + 1) / 255)

Proof: Let t+1 = 255k + j (0 <= j <= 254), so u = 254k + j - 1.
j >= 1: floor(u/254)=k, u+floor(u/254) = 255k+j-1 = t. Exact fit.
j = 0: u=254(k-1)+253, floor(u/254)=k-1, sum = t-1. One byte slack.
In both cases u+1 would exceed t, confirming u is the maximum.

@see _cobsEncodedMaxLength()
*/
/**************************************************************************/
uint32_t _cobsGuaranteedFit(uint32_t bufLen)
{
// encodedLen = unencodedLen + codeBytesLen
// e = u + c (e = encoded (sorry Euler), and c = code bytes (sorry Einstein))
// u = e - c
// c = ⌈u / 254⌉ (the ceiling of u divided by 254)
//
// Rearranging the ceiling equation:
// (c - 1) < u / 254 <= c
// 254(c - 1) < u <= 254c
//
// Substitute u from first equation:
// 254(c - 1) < e - c <= 254c
// 254c - 254 < e - c <= 254c
// 255c < e + 254 AND e <= 255c
//
// Thus:
// e <= 255c < e + 254
// e / 255 <= c < (e + 254) / 255
//
// Knowing that c is an integer, we can express c as:
// c = ⌊(e + 254) / 255⌋ (the floor of (e + 254) divided by 255)
//
// Substitute c back into the original equation for u:
// u = e - ⌊(e + 254) / 255⌋
const uint32_t encodedLen = (bufLen == 0) + ((bufLen != 0) * (bufLen - COBS_EOP_OVERHEAD));
return (encodedLen - ((encodedLen + COBS_MAX_PACKET_SIZE) / 255));
if (bufLen <= 2) {
return 0;
}
const uint32_t t = bufLen - 2;
return (t - ((t + 1) / 255));
}
77 changes: 77 additions & 0 deletions test/src/_cobsEncodedMaxLength_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,83 @@ SCENARIO("_cobsEncodedMaxLength")
}
}
}

// The following tests verify the fix for the ceil(u/254) vs floor(u/254)+1
// bug. At exact multiples of 254, the encoder emits an extra code byte
// that ceil() misses. Example: 254 non-zero bytes encode as [0xFF, data
// x254, 0x01] — that's 2 code bytes, not ceil(254/254)=1.

GIVEN("A buffer length of exactly 253 bytes (just under one full COBS block)") {
const uint32_t bufLen = 253;

WHEN("The max encoded length is calculated") {
const uint32_t encodedLen = _cobsEncodedMaxLength(bufLen);

THEN("The encoded length is 253 + 1 code byte + 1 EOP = 255") {
CHECK(encodedLen == 255);
}
}
}

GIVEN("A buffer length of exactly 254 bytes (one full COBS block)") {
// BUG FIX: Old code used ceil(254/254)=1 code byte, but the encoder
// actually emits 2: one 0xFF at the start and one 0x01 at the end.
const uint32_t bufLen = 254;

WHEN("The max encoded length is calculated") {
const uint32_t encodedLen = _cobsEncodedMaxLength(bufLen);

THEN("The encoded length is 254 + 2 code bytes + 1 EOP = 257") {
CHECK(encodedLen == 257);
}
}
}

GIVEN("A buffer length of exactly 508 bytes (two full COBS blocks)") {
// BUG FIX: Old code used ceil(508/254)=2 code bytes, but the encoder
// emits 3: one per 254-byte block plus one final.
const uint32_t bufLen = 508;

WHEN("The max encoded length is calculated") {
const uint32_t encodedLen = _cobsEncodedMaxLength(bufLen);

THEN("The encoded length is 508 + 3 code bytes + 1 EOP = 512") {
CHECK(encodedLen == 512);
}
}
}

GIVEN("A buffer length of 255 bytes (one byte past a full COBS block)") {
const uint32_t bufLen = 255;

WHEN("The max encoded length is calculated") {
const uint32_t encodedLen = _cobsEncodedMaxLength(bufLen);

THEN("The encoded length is 255 + 2 code bytes + 1 EOP = 258") {
CHECK(encodedLen == 258);
}
}
}
}

SCENARIO("_cobsEncodedMaxLength round-trip with _cobsGuaranteedFit")
{
GIVEN("Various buffer sizes") {
// For any buffer size B, encoding cobsGuaranteedFit(B) bytes must
// produce at most B bytes (including the EOP byte).
uint32_t testSizes[] = {3, 8, 255, 256, 257, 510, 511, 512, 65536, 65796, 262144};

WHEN("_cobsGuaranteedFit is computed and then _cobsEncodedMaxLength is applied") {
THEN("The encoded max length never exceeds the original buffer") {
for (size_t i = 0; i < sizeof(testSizes)/sizeof(testSizes[0]); i++) {
uint32_t bufLen = testSizes[i];
uint32_t maxUnencoded = _cobsGuaranteedFit(bufLen);
uint32_t encodedLen = _cobsEncodedMaxLength(maxUnencoded);
CHECK(encodedLen <= bufLen);
}
}
}
}
}
}

Expand Down
100 changes: 100 additions & 0 deletions test/src/_cobsGuaranteedFit_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,106 @@ SCENARIO("_cobsGuaranteedFit")
}
}
}

// The following tests verify the fix for the ceil(u/254) vs floor(u/254)+1
// bug. At bufLen = 255k+1, the old formula overestimated by 1, promising a
// fit that would actually overflow the buffer after COBS encoding.

GIVEN("A buffer length of 256 bytes (255*1 + 1, boundary case)") {
// BUG FIX: Old code returned 254. But encoding 254 non-zero bytes
// produces 254 + 2 code bytes = 256, which with +1 EOP = 257 > 256.
// Correct answer: 253 (encodes to 253 + 1 code + 1 EOP = 255 ≤ 256).
const uint32_t bufLen = 256;

WHEN("The max unencoded length is calculated") {
const uint32_t unencodedLen = _cobsGuaranteedFit(bufLen);

THEN("The unencoded length is 253, not 254") {
CHECK(unencodedLen == 253);
}
}
}

GIVEN("A buffer length of 511 bytes (255*2 + 1, boundary case)") {
// BUG FIX: Old code returned 508. But encoding 508 non-zero bytes
// produces 508 + 3 code bytes = 511, plus 1 EOP = 512 > 511.
// Correct answer: 507.
const uint32_t bufLen = 511;

WHEN("The max unencoded length is calculated") {
const uint32_t unencodedLen = _cobsGuaranteedFit(bufLen);

THEN("The unencoded length is 507, not 508") {
CHECK(unencodedLen == 507);
}
}
}

GIVEN("A buffer length of 766 bytes (255*3 + 1, boundary case)") {
const uint32_t bufLen = 766;

WHEN("The max unencoded length is calculated") {
const uint32_t unencodedLen = _cobsGuaranteedFit(bufLen);

THEN("The unencoded length is 761") {
CHECK(unencodedLen == 761);
}
}
}

GIVEN("A buffer length of 257 bytes (non-boundary, one past 256)") {
const uint32_t bufLen = 257;

WHEN("The max unencoded length is calculated") {
const uint32_t unencodedLen = _cobsGuaranteedFit(bufLen);

THEN("The unencoded length is 254") {
CHECK(unencodedLen == 254);
}
}
}

GIVEN("A buffer length of 262144 bytes (customer-reported capacity)") {
const uint32_t bufLen = 262144;

WHEN("The max unencoded length is calculated") {
const uint32_t unencodedLen = _cobsGuaranteedFit(bufLen);

THEN("The unencoded length is 261114") {
CHECK(unencodedLen == 261114);
}
}
}
}

SCENARIO("_cobsGuaranteedFit round-trip with _cobsEncodedMaxLength")
{
GIVEN("Buffer sizes at every 255k+1 boundary from 1 to 2551") {
WHEN("_cobsGuaranteedFit is computed and then _cobsEncodedMaxLength is applied") {
THEN("The encoded max length never exceeds the original buffer") {
for (uint32_t k = 0; k <= 10; k++) {
uint32_t bufLen = 255 * k + 1;
if (bufLen <= 2) continue;
uint32_t maxUnencoded = _cobsGuaranteedFit(bufLen);
uint32_t encodedLen = _cobsEncodedMaxLength(maxUnencoded);
CHECK(encodedLen <= bufLen);
}
}
}
}

GIVEN("Buffer sizes at every 255k boundary from 255 to 2550") {
WHEN("The result is checked against _cobsEncodedMaxLength") {
THEN("The encoded max length never exceeds the original buffer") {
for (uint32_t k = 1; k <= 10; k++) {
uint32_t bufLen = 255 * k;
uint32_t maxUnencoded = _cobsGuaranteedFit(bufLen);
uint32_t encodedLen = _cobsEncodedMaxLength(maxUnencoded);
CHECK(encodedLen <= bufLen);
}
}
}
}
}
}

Expand Down
Loading