Skip to content
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.

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

Fix: eof error handling when decoding in CborCodec #7

Closed
wants to merge 8 commits into from
Closed

Fix: eof error handling when decoding in CborCodec #7

wants to merge 8 commits into from

Conversation

sandreae
Copy link
Contributor

@sandreae sandreae commented Jul 14, 2023

Closes: #6

  • return when decoding is eof and don't advance the buffer
  • tests for this and other error cases
  • update CHANGLOG

@sandreae sandreae changed the title Fix for eof error handling when decoding in CborCodec Fix: eof error handling when decoding in CborCodec Jul 14, 2023
Copy link
Owner

@mxinden mxinden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, thank you for the work here. I have one more suggestion. Otherwise this is good to go.

Related, the other Codec implementations in this crate face the same issue. In case you are interested, I would very much appreciate fixes for those as well. No worries if not.

// Attempt to fetch an item and generate response
let res = match iter.next() {
Some(Ok(v)) => Ok(Some(v)),
Some(Err(ref e)) if e.is_eof() => Ok(None),
Some(Err(e)) => Err(e.into()),
None => Ok(None),
};
// Update offset from iterator
let offset = iter.byte_offset();
// Advance buffer
buf.advance(offset);
res

src/codec/cbor.rs Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
src/codec/cbor.rs Outdated Show resolved Hide resolved
sandreae and others added 2 commits July 18, 2023 09:59
Co-authored-by: Max Inden <mail@max-inden.de>
@sandreae
Copy link
Contributor Author

Related, the other Codec implementations in this crate face the same issue. In case you are interested, I would very much appreciate fixes for those as well. No worries if not.

Happy to add this too 👍

@sandreae sandreae requested a review from mxinden July 18, 2023 09:14
@sandreae
Copy link
Contributor Author

@mxinden thanks for your feedback, think this is ready to go now.

Copy link
Owner

@mxinden mxinden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, well done. Thank you.

@@ -6,6 +6,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic
Versioning](https://semver.org/spec/v2.0.0.html).

## [0.6.2] - unreleased
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## [0.6.2] - unreleased
## [0.6.2]


### Fixed

- Error handling in `CborCodec` and `JsonCodec` `decode` [#7](https://github.com/mxinden/asynchronous-codec/pull/7)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Error handling in `CborCodec` and `JsonCodec` `decode` [#7](https://github.com/mxinden/asynchronous-codec/pull/7)
- Error handling in `CborCodec` and `JsonCodec` `decode`, more specifically not advancing the data buffer on partial decoding. See [#7](https://github.com/mxinden/asynchronous-codec/pull/7) for details.

@mxinden
Copy link
Owner

mxinden commented Jul 20, 2023

I was not able to accept the above suggestions, thus merged through #8. Attribution should still be tracked for your work.

@mxinden
Copy link
Owner

mxinden commented Jul 20, 2023

Tagged and published.

@mxinden mxinden closed this Jul 20, 2023
@sandreae
Copy link
Contributor Author

Tagged and published.

Great, thanks!

@adzialocha adzialocha deleted the fix-decode-eof-handling branch July 24, 2023 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CborCodec: bytes missing when decoding received messages
2 participants