Skip to content
New issue

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

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

Already on GitHub? # to your account

WIP Serde codec #32

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

WIP Serde codec #32

wants to merge 4 commits into from

Conversation

povilasb
Copy link
Contributor

Implements codec that encodes serde::Serialize types.
Uses bincode to do the actual serialization.

@Kerollmops
Copy link
Contributor

Would be great to make those dependencies optional and under a feature flag, can be enable by default but can also be disabled (i.e. default-features = false).

@povilasb
Copy link
Contributor Author

Sure, makes sense. Will do within a few days ;)

This test panics at decode()

    if src.len() - U64_LENGTH

because of underflow.
The problem was that 'src' buffer was advanced too soon.
This caused a couple of bugs at least in 2 scenarios:

1. when encoded frame was smaller than 8 bytes.
2. when 'src' did not have enough bytes do decode a complete frame.

 ## Problem 1

Say we encode a frame [1, 2, 3].
The resulting buffer is [0, 0, 0, 0, 0, 0, 0, 3, 1, 2, 3] - with prepended
length header.
Then decode() takes this buffer, correctly parses len as 3, and then
does this comparison:

    if src.len() - U64_LENGTH

Given that we already advanced src buffer, src.len() now is 3 (11 - 8).
This results in uderflow: 3 - 8, which panics.

 ## Problem 2

Say we have framed a TCP stream where data is coming in chunks but not
necessarily in the size of our frames.
Say we call decode() for the first time with src = [0, 0, 0, 0, 0, 0, 0, 64, 1, 2, 3].
Obviously this is not a full frame: it's size is 64 but only first 3 bytes have arrived.
In this case decode() used to advance the src buffer, but still return None.
So when a next TCP packet arrives, decode() was called for the second time.
decode() then used to attempt to parse len header again. And this time it
would parse the bytes [1, 2, 3, ...] as a len header which is actually
our data.
@povilasb povilasb changed the title Serde codec WIP Serde codec Dec 21, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants