Public Access
Debug Console put: verify the card's copy, never zero-fill after a failed write
Found by M3's shared-bus test: when the card refused a write, the retry closed the file (losing up to 3 KB of earlier chunks still in the write buffer), then truncate() extended it back with zeros. The checksum only covered the received bytes, so `put` reported success with 3 KB of zeros on the card. Now a retry gives up if the card lost data, and the finished file is read back and must hash the same before it's renamed. The card refuses a write about once in five 1.7 MB uploads, with the radio asleep as often as listening. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EhqxQ49eCju4CzKYNjZzwT
This commit is contained in:
@@ -94,6 +94,11 @@ void FileReceiver::chunkWritten(bool ok, uint32_t nowMs) {
|
||||
state_ = received_ == size_ ? State::Finishing : State::Receiving;
|
||||
}
|
||||
|
||||
void FileReceiver::cardChecked(const uint8_t digest[32]) {
|
||||
if (state_ != State::Finishing) return;
|
||||
if (std::memcmp(digest, expected_, sizeof expected_) != 0) fail("the copy on the card differs");
|
||||
}
|
||||
|
||||
void FileReceiver::finished(bool ok) {
|
||||
if (state_ != State::Finishing) return;
|
||||
if (!ok) return fail("rename failed");
|
||||
|
||||
@@ -34,6 +34,9 @@ class FileReceiver {
|
||||
// Takes bytes for the current chunk; returns how many were used (none while a chunk waits).
|
||||
size_t feed(const uint8_t* data, size_t len, uint32_t nowMs);
|
||||
void chunkWritten(bool ok, uint32_t nowMs);
|
||||
// While Finishing: the SHA-256 of the file as read back from the card. The checksum on the
|
||||
// received bytes doesn't prove the card kept them (a failed write can lose buffered data).
|
||||
void cardChecked(const uint8_t digest[32]);
|
||||
void finished(bool ok);
|
||||
void tick(uint32_t nowMs);
|
||||
void reset() { *this = FileReceiver(); }
|
||||
|
||||
@@ -15,6 +15,7 @@
|
||||
#include <memory>
|
||||
|
||||
#include "file_receiver.h"
|
||||
#include "sha256.h"
|
||||
#include "platform/console.h"
|
||||
#include "version.h"
|
||||
|
||||
@@ -233,11 +234,20 @@ void DebugConsole::put(NetworkClient& client, const std::string& args) {
|
||||
const auto& c = r.chunk();
|
||||
bool ok = f.write(c.data(), c.size()) == c.size();
|
||||
// A card can fail one write and take the next. FATFS keeps a failed file in error,
|
||||
// so: close, cut back to the last good byte, reopen, try again.
|
||||
// so: close, cut back to the last good byte, reopen, try again. Earlier chunks may
|
||||
// have been lost with the write buffer (M3: 3 KB came back as zeros): never extend
|
||||
// the file to cover them; give up instead, as the sender can't resend them.
|
||||
for (int retry = 1; !ok && retry <= 3; retry++) {
|
||||
console.printf("put: write failed at %u, retry %d\n", (unsigned)r.received(), retry);
|
||||
f.close();
|
||||
delay(50 * retry);
|
||||
f = SD.open(part.c_str(), FILE_READ);
|
||||
size_t onCard = f ? f.size() : 0;
|
||||
if (f) f.close();
|
||||
if (onCard < r.received()) {
|
||||
console.printf("put: the card lost %u B written before\n", (unsigned)(r.received() - onCard));
|
||||
break;
|
||||
}
|
||||
truncate(("/sd" + part).c_str(), r.received());
|
||||
f = SD.open(part.c_str(), FILE_APPEND);
|
||||
ok = f && f.size() == r.received() && f.write(c.data(), c.size()) == c.size();
|
||||
@@ -254,6 +264,15 @@ void DebugConsole::put(NetworkClient& client, const std::string& args) {
|
||||
}
|
||||
}
|
||||
f.close();
|
||||
if (r.state() == S::Finishing) { // read it back: the received checksum doesn't cover the card
|
||||
Sha256 sha;
|
||||
f = SD.open(part.c_str(), FILE_READ);
|
||||
for (int n; f && (n = f.read(buf, sizeof buf)) > 0;) sha.update(buf, n);
|
||||
if (f) f.close();
|
||||
uint8_t digest[32];
|
||||
sha.finish(digest);
|
||||
r.cardChecked(digest);
|
||||
}
|
||||
if (r.state() == S::Finishing) {
|
||||
if (SD.exists(r.path().c_str())) SD.remove(r.path().c_str());
|
||||
r.finished(SD.rename(part.c_str(), r.path().c_str()));
|
||||
|
||||
@@ -184,6 +184,34 @@ void test_reset_returns_to_idle() {
|
||||
TEST_ASSERT_FALSE(r.active());
|
||||
}
|
||||
|
||||
// The received bytes can be right and the card's copy wrong: read back, it must hash the same.
|
||||
void test_card_check() {
|
||||
auto data = bytes(10);
|
||||
FileReceiver r;
|
||||
r.begin(args("/a.bin", data), 0);
|
||||
r.feed(data.data(), data.size(), 0);
|
||||
r.chunkWritten(true, 0);
|
||||
TEST_ASSERT_TRUE(r.state() == FileReceiver::State::Finishing);
|
||||
uint8_t good[32];
|
||||
Sha256::hash(data.data(), data.size(), good);
|
||||
r.cardChecked(good);
|
||||
TEST_ASSERT_TRUE(r.state() == FileReceiver::State::Finishing);
|
||||
r.finished(true);
|
||||
TEST_ASSERT_TRUE(r.state() == FileReceiver::State::Done);
|
||||
|
||||
FileReceiver bad;
|
||||
bad.begin(args("/a.bin", data), 0);
|
||||
bad.feed(data.data(), data.size(), 0);
|
||||
bad.chunkWritten(true, 0);
|
||||
auto zeroed = data;
|
||||
zeroed[4] = 0;
|
||||
uint8_t wrong[32];
|
||||
Sha256::hash(zeroed.data(), zeroed.size(), wrong);
|
||||
bad.cardChecked(wrong);
|
||||
TEST_ASSERT_TRUE(bad.state() == FileReceiver::State::Failed);
|
||||
TEST_ASSERT_EQUAL_STRING("the copy on the card differs", bad.error().c_str());
|
||||
}
|
||||
|
||||
int main() {
|
||||
UNITY_BEGIN();
|
||||
RUN_TEST(test_begin_accepts_path_size_and_checksum);
|
||||
@@ -198,5 +226,6 @@ int main() {
|
||||
RUN_TEST(test_wanted_counts_down_within_a_chunk);
|
||||
RUN_TEST(test_a_rename_that_never_completes_times_out);
|
||||
RUN_TEST(test_reset_returns_to_idle);
|
||||
RUN_TEST(test_card_check);
|
||||
return UNITY_END();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user