Repository navigation
File uploads: check the file against the CRC16 given in ufStart - #29
Merged
Merged
Conversation
ufStart has an optional CRC16, which was parsed and passed along with every
block but never checked, so clients checked uploads with a separate filehash
request.
FileStreamSession now keeps a CRC16-CCITT (as filehash reports it) of the data
as it is written, in order - on the file system worker for background writes -
and after the final block compares it with the CRC16 given. A mismatch fails
the upload with the new RAFT_CRC_MISMATCH: the file is deleted and the client
gets a ufCancel with reason failCRC instead of the final ack (or a ufStatus
CRCMismatch reply when blocks are written as they arrive). When the CRC was
given and matched, the ufEnd reply includes it ("crc16":"xxxx"), so a client
knows the device checked the file and can skip filehash; older firmware
doesn't, so clients that see no crc16 in the reply still check.
Firmware updates aren't checked here (ESP-IDF validates the image).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…its CRC
The upload session ends once the final block is written, so ufEnd usually
finds no session (and got a plain "ok" from ProtocolExchange). The protocol's
own ufEnd reply also couldn't know the CRC had been compared.
The session now reports the CRC only once the whole file has been written and
matched (getCheckedCRC()). ProtocolExchange keeps it, by channel and file name,
when the session ends, and gives it in the ufEnd reply ("crc16":"xxxx") whether
the session is still there or not. A mismatch gives a ufCancel (failCRC), no
crc16 in the ufEnd reply, and the file deleted - tested through
ProtocolExchange. The write job no longer also logs a CRC mismatch as a failed
write.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ufStarthas an optionalCRC16. It was parsed and passed along with every block, but never checked, so clients verified uploads with a separatefilehashrequest.FileStreamSessionkeeps a CRC16-CCITT of the data as it writes it, in order, the same CRCfilehashreports. For background writes this runs on the file system worker. After the final block it compares the result with the givenCRC16.RAFT_CRC_MISMATCH):ufCancelwith reasonfailCRCinstead of the final ack;ufStatusreply with reasonCRCMismatch.ufEndreply says so with"crc16":"xxxx", so a client can skipfilehash. The session ends once the final block is written, soufEndusually finds no session. ProtocolExchange therefore keeps the checked CRC, by channel and file name, after the session ends. A CRC is reported only once the whole file has been written and has matched.crc16, so a client that doesn't see one keeps checking withfilehash. Firmware updates aren't checked here, since ESP-IDF validates the image.RaftCLI sends the CRC and uses the reply: robdobsn/RaftCLI#4.
Testing
test_file_upload_backgroundcovers matching, wrong and absent CRCs: no cancel and a checked CRC;failCRCand the file deleted; no check.test_protocol_exchangecovers the real sequence: upload, the session ends, thenufEnd, with the CRC in the reply only when it matched.make checkpasses.ufEndreplies{"crc16":"19ff","rslt":"ok"};failCRCin place of the final ack, nocrc16in theufEndreply, and the file gone.🤖 Generated with Claude Code