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(crowtty): don't panic on 1-byte SerMux frames #117

Merged
merged 1 commit into from
Jul 1, 2023

Conversation

hawkw
Copy link
Contributor

@hawkw hawkw commented Jul 1, 2023

Currently, crowtty calls carry[..used].split_at(2) every time it decodes a COBS-encoded frame SerMux frame, to extract the port component from the frame. However, if we send a zero byte while not currently in a frame, the COBS decoder will interpret it as a frame with a single zero in it. When we call split_at, crowtty will panic, because the slice only has one byte in it.

Commit 5231001 changed the panic handler to always send a zero before writing out the panic message, in order to terminate the current SerMux frame if one is in the process of being sent. However, that zero makes crowtty crash if it is not decoding a partial frame. This means that, rather than nicely printing the panic message, crowtty just panics immediately when the board panics. That kinda sucks.

This branch fixes this, by changing crowtty to check whether the length of the decoded COBS frame is less than the minimum frame length (the 2-byte port number). Now, if we decode a single-byte zero frame, we'll just ignore it and keep going.

Currently, `crowtty` calls `carry[..used].split_at(2)` every time it
decodes a COBS-encoded frame SerMux frame, to extract the port component
from the frame. However, if we send a zero byte while not currently in a
frame, the COBS decoder will interpret it as a frame with a single zero
in it. When we call `split_at`, crowtty will panic, because the slice
only has one byte in it.

Commit 5231001 changed the panic
handler to always send a zero *before* writing out the panic message, in
order to terminate the current SerMux frame if one is in the process of
being sent. However, that zero makes crowtty crash if it is *not*
decoding a partial frame. This means that, rather than nicely printing
the panic message, crowtty just panics immediately when the board
panics. That kinda sucks.

This branch fixes this, by changing crowtty to check whether the length
of the decoded COBS frame is less than the minimum frame length (the
2-byte port number). Now, if we decode a single-byte zero frame, we'll
just ignore it and keep going.
@hawkw hawkw requested a review from jamesmunns July 1, 2023 17:56
@netlify
Copy link

netlify bot commented Jul 1, 2023

Deploy Preview for merry-scone-cc7a60 ready!

Name Link
🔨 Latest commit 6b99fd1
🔍 Latest deploy log https://app.netlify.com/sites/merry-scone-cc7a60/deploys/64a068c60e4ef50008ffd8b4
😎 Deploy Preview https://deploy-preview-117--merry-scone-cc7a60.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@hawkw hawkw marked this pull request as ready for review July 1, 2023 17:56
@hawkw hawkw merged commit e452653 into main Jul 1, 2023
10 checks passed
@hawkw hawkw deleted the eliza/fix-1-byte-reads branch July 1, 2023 18:10
@hawkw hawkw added this to the beepberry computer v0.1 milestone Jul 10, 2023
@jamesmunns jamesmunns added area: tools & build Related to host developer tools, including tracing, Crowtty and build processes area: protocols Related to communication protocols, including SerMux and Traceproto. labels Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: protocols Related to communication protocols, including SerMux and Traceproto. area: tools & build Related to host developer tools, including tracing, Crowtty and build processes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants