diff --git a/AntScope.pro b/AntScope.pro index df459a8..d94f012 100644 --- a/AntScope.pro +++ b/AntScope.pro @@ -267,6 +267,15 @@ unix:!macx { SOURCES += analyzer/usbhid/hidapi/linux/hid.c LIBS += -lusb-1.0 DEFINES += _NO_WINDOWS_ + + # Without this, analyzer/usbhid/hidapi/linux/hid.c's + # libusb_claim_interface() is called without ever detaching the + # kernel's usbhid driver first. usbhid auto-binds normal-mode HID + # interfaces on Linux, and while libusb can still open/claim the + # interface in that state, an interrupt-OUT hid_write() to it can fail + # outright (observed: hid_write() returning -1 for the firmware-update + # RESET report on a real device the kernel had already claimed). + DEFINES += DETACH_KERNEL_DRIVER } macx { diff --git a/README.md b/README.md index 3ee6417..e99ca41 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,16 @@ The AntScope2 software is designed to support various models of RigExpert analyzers under various operating systems. -Windows: QT5, QT Creator 2 and higher +Windows: Qt6, Qt Creator 2 and higher -Linux: to do +Linux: Qt6, qmake. Build with: + +``` +qmake6 CONFIG+=release AntScope.pro +make +``` + +Depends on `qt6-base`, `qt6-serialport`, `qt6-connectivity`, and `libusb` (used via the vendored HIDAPI backend for HID-connected analyzers). Packaged for Arch Linux/AUR as +[`antscope2`](https://aur.archlinux.org/packages/antscope2) (pinned to a known-good commit, patched) +and [`antscope2-git`](https://aur.archlinux.org/packages/antscope2-git) (tracks a patched branch's tip - see either package's `PKGBUILD` for the udev rule, desktop file, and data-path packaging details this repo's own build doesn't set up on its own). Mac OS: to do diff --git a/analyzer/ble_analyzer.cpp b/analyzer/ble_analyzer.cpp index 5c86d5e..273c38c 100644 --- a/analyzer/ble_analyzer.cpp +++ b/analyzer/ble_analyzer.cpp @@ -474,6 +474,13 @@ void BleAnalyzer::dataReceived(const QLowEnergyCharacteristic &c, const QByteArr { if (c.uuid() != QBluetoothUuid(uuidRead)) return; + if (value.size() < BLE_PACKET_SIZE) { + // checkCRC()/returnCRC() index data[BLE_PACKET_SIZE-1] with no + // bounds check of their own; a malformed or malicious peripheral + // sending a short notification would otherwise read out of bounds. + qInfo() << "dataReceived: short packet, ignoring" << value.size(); + return; + } //qInfo() << trace("dataReceived: ", const_cast(value)); m_lastReadTimeMS = QDateTime::currentMSecsSinceEpoch(); if (!checkCRC(value)) { diff --git a/analyzer/hid_analyzer.cpp b/analyzer/hid_analyzer.cpp index a3d9fd1..6adffee 100644 --- a/analyzer/hid_analyzer.cpp +++ b/analyzer/hid_analyzer.cpp @@ -2,6 +2,7 @@ #include "customanalyzer.h" #include #include +#include #include "analyzerpro.h" extern bool g_usbOnly; @@ -453,14 +454,20 @@ void HidAnalyzer::hidRead (void) { return; } - unsigned char readBuff[64]; + unsigned char readBuff[64] = {0}; int read = hid_read(m_hidDevice, readBuff, 64); m_mutexRead.lock(); if(read > 0) { if(readBuff[0] == ANTSCOPE_REPORT) { - for(int i = 0; i < readBuff[1]; i++) + // readBuff[1] is a payload length supplied by the device + // (0-255); clamp it to both the fixed-size buffer and to how + // many bytes hid_read() actually returned, so a device + // reporting more than 62 payload bytes - or a short read - + // cannot walk past the end of readBuff. + int len = qMin(readBuff[1], qMax(0, read - 2)); + for(int i = 0; i < len; i++) { m_incomingBuffer.append(readBuff[i+2]); } @@ -680,9 +687,59 @@ void HidAnalyzer::preUpdate () { hid_close(m_hidDevice); m_hidDevice = nullptr; + // Leaving boot mode (whichever way update() ended, success or + // failure) - without this, a second update() call would skip the + // boot-mode entry block above and immediately try to write firmware + // commands to the now-null m_hidDevice. + m_bootMode = false; searchAnalyzer(true); } +bool HidAnalyzer::waitForBootDevice(qint64 timeoutMs) +{ + QElapsedTimer elapsed; + elapsed.start(); + while (!elapsed.hasExpired(timeoutMs)) + { + struct hid_device_info* devs = hid_enumerate(RE_BOOT_VID, RE_BOOT_PID); + hid_device* newDevice = nullptr; + for (struct hid_device_info* cur = devs; cur != nullptr; cur = cur->next) + { + // The serial number is preserved across the mode switch + // (verified against a real capture). If for some reason it's + // not known (m_serialNumber empty), fall back to matching on + // VID:PID alone rather than refusing to proceed. + QString serial = QString::fromWCharArray(cur->serial_number); + if (!m_serialNumber.isEmpty() && serial != m_serialNumber) + { + continue; + } + newDevice = hid_open(RE_BOOT_VID, RE_BOOT_PID, cur->serial_number); + if (newDevice != nullptr) + { + break; + } + } + hid_free_enumeration(devs); + + if (newDevice != nullptr) + { + if (m_hidDevice != nullptr) + { + hid_close(m_hidDevice); + } + m_hidDevice = newDevice; + hid_set_nonblocking(m_hidDevice, 1); + m_bootMode = true; + return true; + } + + QCoreApplication::processEvents(); + QThread::msleep(100); + } + return false; +} + bool HidAnalyzer::update (QIODevice *fw) { m_hidReadTimer->stop(); @@ -697,16 +754,17 @@ bool HidAnalyzer::update (QIODevice *fw) hid_write(m_hidDevice, buff, sizeof(buff)); qDebug() << "RESET: " << hidError(m_hidDevice); - QTimer::singleShot(5000, this, [this]() { - this->preUpdate(); - }); - while(1)//for(int i = 0; i < 565535; ++i) - { - if(m_bootMode) - break; - QCoreApplication::processEvents(); - } - if(!m_bootMode) + // The device closes its normal-mode connection and re-enumerates + // under RE_BOOT_VID:RE_BOOT_PID as its DFU bootloader, keeping the + // same USB serial number (verified against a real capture of a + // successful update - see docs/firmware-update-protocol-findings.md + // in this tree). The app's normal hot-plug detection + // (searchAnalyzer()/m_devices) is gated behind g_usbOnly and, even + // if it were reachable, only ever looks for the application-mode + // VID:PID - it can never find the device in this state. Poll for it + // directly instead. 15s is a generous bound; the one real session + // this was verified against re-enumerated within about 1.3s. + if (!waitForBootDevice(15000)) { g_showMessageBox(nullptr, QMessageBox::Warning,tr("Warning"),tr("Can't enter to boot mode!")); return false; @@ -752,16 +810,24 @@ bool HidAnalyzer::update (QIODevice *fw) emit updatePercentChanged(i*100/totsize); QCoreApplication::processEvents(); - if (firstWrite) + // Read and check the response to every chunk, not just the first. + // hidapi's background read thread queues unread reports and drops + // the oldest once the queue is full (see + // docs/firmware-update-protocol-findings.md); previously only the + // first chunk's response was ever consumed, so by the time the + // final BL_CMD_CHECK was sent, the queue held nothing but stale + // BL_CMD_OK reports left over from earlier chunks - the "checksum + // verification" was reading one of those, not a real answer to + // CHECK, and so reported success unconditionally. Reading (and + // pacing on) every response fixes that as a side effect, and + // means a real BL_CMD_ERROR from the device is no longer silently + // ignored. + firstWrite = false; + res = waitAnswer(); + if (!res) { - res = waitAnswer(); - firstWrite = false; - if (!res) - { - emit updatePercentChanged(100); - return false; - break; - } + emit updatePercentChanged(100); + return false; } } emit updatePercentChanged(100); diff --git a/analyzer/hid_analyzer.h b/analyzer/hid_analyzer.h index 39448d1..9593847 100644 --- a/analyzer/hid_analyzer.h +++ b/analyzer/hid_analyzer.h @@ -61,6 +61,20 @@ class HidAnalyzer : public BaseAnalyzer bool update(QIODevice *fw); +#ifdef ANTSCOPE2_UNIT_TEST + // Test-only seam: inject an already-open device handle and serial + // number, bypassing the normal UI-dialog-driven connection + // establishment (SelectDeviceDialog / AnalyzerPro::createDevice), + // so update() can be exercised in isolation against a mock hidapi + // backend. Compiled out entirely unless ANTSCOPE2_UNIT_TEST is + // defined; has no effect on and is not reachable from the real app. + void setTestHidDevice(hid_device* dev, const QString& serial) + { + m_hidDevice = dev; + m_serialNumber = serial; + } +#endif + void nonblocking (int nonblock); void preUpdate(); QString hidError(hid_device* _device); @@ -104,6 +118,7 @@ private slots: bool disconnectHid(void); qint32 parse (QByteArray arr); bool waitAnswer(); + bool waitForBootDevice(qint64 timeoutMs); QFuture *m_futureRefresh; QFutureWatcher *m_watcherRefresh; }; diff --git a/analyzer/hidanalyzer.cpp b/analyzer/hidanalyzer.cpp index 78b3754..654df49 100644 --- a/analyzer/hidanalyzer.cpp +++ b/analyzer/hidanalyzer.cpp @@ -2,6 +2,7 @@ #include "customanalyzer.h" #include #include +#include #include "analyzer.h" extern int g_showMessageBox(QWidget* parent, QMessageBox::Icon icon, QString title, QString text, @@ -462,14 +463,20 @@ void hidAnalyzer::hidRead (void) { return; } - unsigned char readBuff[64]; + unsigned char readBuff[64] = {0}; int read = hid_read(m_hidDevice, readBuff, 64); m_mutexRead.lock(); if(read > 0) { if(readBuff[0] == ANTSCOPE_REPORT) { - for(int i = 0; i < readBuff[1]; i++) + // readBuff[1] is a payload length supplied by the device + // (0-255); clamp it to both the fixed-size buffer and to how + // many bytes hid_read() actually returned, so a device + // reporting more than 62 payload bytes - or a short read - + // cannot walk past the end of readBuff. + int len = qMin(readBuff[1], qMax(0, read - 2)); + for(int i = 0; i < len; i++) { m_incomingBuffer.append(readBuff[i+2]); } @@ -755,10 +762,22 @@ bool hidAnalyzer::update (QIODevice *fw) QTimer::singleShot(5000, this, [this]() { this->preUpdate(); }); - while(1)//for(int i = 0; i < 565535; ++i) + // m_bootMode is only ever set by the device re-arrival path, which + // does not currently work (the feature this belongs to is + // incomplete and unreachable from the UI). Previously this was an + // unbounded while(1) with no way out but m_bootMode becoming true, + // so a call that could never detect the device coming back would + // spin forever instead of ever reaching the failure handling right + // below. Bound the wait so that path is actually reachable. + QElapsedTimer bootModeWait; + bootModeWait.start(); + const qint64 bootModeTimeoutMs = 15000; + while(1) { if(m_bootMode) break; + if(bootModeWait.hasExpired(bootModeTimeoutMs)) + break; QCoreApplication::processEvents(); } if(!m_bootMode) diff --git a/analyzer/updater/aa230firmwareupdater.cpp b/analyzer/updater/aa230firmwareupdater.cpp index 8f86a37..2fcdb07 100644 --- a/analyzer/updater/aa230firmwareupdater.cpp +++ b/analyzer/updater/aa230firmwareupdater.cpp @@ -124,7 +124,10 @@ AA230FirmwareUpdater::FirmwareInfo AA230FirmwareUpdater::firmwareInfo(const ReDe arr = m_port.readAll(); } - if (arr.isEmpty()) { + // The probe loop above gives up after 10 timeouts and falls through + // regardless of how many bytes actually arrived, so arr can be + // shorter than FirmwareInfo on a slow/short/malicious response. + if (arr.size() < static_cast(sizeof(FirmwareInfo))) { return info; } diff --git a/analyzer/updater/downloader.cpp b/analyzer/updater/downloader.cpp index 47cb10d..529a8c8 100644 --- a/analyzer/updater/downloader.cpp +++ b/analyzer/updater/downloader.cpp @@ -29,9 +29,6 @@ Downloader::State Downloader::startDownloadInfo(QUrl url) QNetworkRequest request(url); m_mng.clearAccessCache(); - QSslConfiguration conf = request.sslConfiguration(); - conf.setPeerVerifyMode(QSslSocket::VerifyNone); - request.setSslConfiguration(conf); m_mng.get(request); @@ -61,9 +58,6 @@ Downloader::State Downloader::startDownloadFw() QNetworkReply *reply; m_mng.clearAccessCache(); - QSslConfiguration conf = request.sslConfiguration(); - conf.setPeerVerifyMode(QSslSocket::VerifyNone); - request.setSslConfiguration(conf); reply = m_mng.get(request); connect(reply, SIGNAL(downloadProgress(qint64,qint64)), diff --git a/analyzer/updater/hidfirmwareupdater.cpp b/analyzer/updater/hidfirmwareupdater.cpp index 8b039a2..0849462 100644 --- a/analyzer/updater/hidfirmwareupdater.cpp +++ b/analyzer/updater/hidfirmwareupdater.cpp @@ -167,7 +167,15 @@ HidFirmwareUpdater::FirmwareInfo HidFirmwareUpdater::firmwareInfo(const ReDevice ret = hid_read_timeout(m_handleDev, buff, sizeof(buff), 100); if (ret > 0) { - memcpy((char*)&info, &buff[6], 60); + // buff is 65 bytes (valid indices 0-64); copying 60 bytes + // starting at offset 6 would read buff[65], one byte past the + // end. Clamp to both the buffer's real size and to how many + // bytes were actually received. + int avail = qMin(ret, sizeof(buff)); + int len = qMin(60, avail - 6); + if (len > 0) { + memcpy((char*)&info, &buff[6], len); + } } closeDevice(); diff --git a/licenseagent.cpp b/licenseagent.cpp index ecfd99d..5c38436 100644 --- a/licenseagent.cpp +++ b/licenseagent.cpp @@ -62,9 +62,6 @@ void LicenseAgent::requestEmailStatus() request.setTransferTimeout(REPLY_TIMEOUT); m_mng.clearAccessCache(); - QSslConfiguration conf = request.sslConfiguration(); - conf.setPeerVerifyMode(QSslSocket::VerifyNone); - request.setSslConfiguration(conf); setState(WaitEmailStatusWeb); m_mng.get(request); @@ -149,9 +146,6 @@ void LicenseAgent::requestLicense(QString key) request.setTransferTimeout(REPLY_TIMEOUT); m_mng.clearAccessCache(); - QSslConfiguration conf = request.sslConfiguration(); - conf.setPeerVerifyMode(QSslSocket::VerifyNone); - request.setSslConfiguration(conf); setState(WaitLicense); m_canceled = false; @@ -296,9 +290,6 @@ void LicenseAgent::requestInfo() request.setTransferTimeout(REPLY_TIMEOUT); m_mng.clearAccessCache(); - QSslConfiguration conf = request.sslConfiguration(); - conf.setPeerVerifyMode(QSslSocket::VerifyNone); - request.setSslConfiguration(conf); setState(WaitInfoWeb); //showModeless(tr("Register device"),tr("Registration..."), tr("Cancel")); @@ -362,9 +353,6 @@ void LicenseAgent::requestUnit() request.setTransferTimeout(REPLY_TIMEOUT); m_mng.clearAccessCache(); - QSslConfiguration conf = request.sslConfiguration(); - conf.setPeerVerifyMode(QSslSocket::VerifyNone); - request.setSslConfiguration(conf); setState(WaitUnitWeb); m_mng.get(request); @@ -432,9 +420,6 @@ void LicenseAgent::requestStatus_B16(QByteArray data) request.setTransferTimeout(REPLY_TIMEOUT); m_mng.clearAccessCache(); - QSslConfiguration conf = request.sslConfiguration(); - conf.setPeerVerifyMode(QSslSocket::VerifyNone); - request.setSslConfiguration(conf); setState(WaitProfileB16); m_mng.get(request); @@ -453,9 +438,6 @@ void LicenseAgent::requestInfo_B16(QByteArray data) request.setTransferTimeout(REPLY_TIMEOUT); m_mng.clearAccessCache(); - QSslConfiguration conf = request.sslConfiguration(); - conf.setPeerVerifyMode(QSslSocket::VerifyNone); - request.setSslConfiguration(conf); setState(WaitInfoB16); m_mng.get(request); diff --git a/tests/hid_update_mock/.gitignore b/tests/hid_update_mock/.gitignore new file mode 100644 index 0000000..6712f01 --- /dev/null +++ b/tests/hid_update_mock/.gitignore @@ -0,0 +1,6 @@ +Makefile +.qmake.stash +*.o +moc_*.cpp +moc_*.h +hid_update_mock diff --git a/tests/hid_update_mock/analyzerpro.h b/tests/hid_update_mock/analyzerpro.h new file mode 100644 index 0000000..6199680 --- /dev/null +++ b/tests/hid_update_mock/analyzerpro.h @@ -0,0 +1,28 @@ +// Minimal stand-in for the real analyzer/analyzerpro.h, picked up instead of +// it via include-path ordering (this directory is searched before ../../src). +// hid_analyzer.cpp only ever qobject_casts its parent() to AnalyzerPro* and, +// if that succeeds, emits two signals on it - none of that is reachable in +// this test (HidAnalyzer is constructed with no parent, so the cast always +// yields nullptr), so this only needs to be enough for it to compile and +// link, not behave like the real class. Linking the real analyzerpro.h +// pulls in its entire slot surface (~20 methods) for a class this test +// never actually needs. +#ifndef ANALYZERPRO_H +#define ANALYZERPRO_H + +#include +#include +#include "baseanalyzer.h" + +class AnalyzerPro : public QObject +{ + Q_OBJECT +public: + explicit AnalyzerPro(QObject* parent = nullptr) : QObject(parent) {} + +signals: + void updateAutocalibrate5(int, QString); + void stopAutocalibrate5(); +}; + +#endif // ANALYZERPRO_H diff --git a/tests/hid_update_mock/hid_update_mock.pro b/tests/hid_update_mock/hid_update_mock.pro new file mode 100644 index 0000000..2498218 --- /dev/null +++ b/tests/hid_update_mock/hid_update_mock.pro @@ -0,0 +1,28 @@ +QT += core widgets network serialport bluetooth concurrent printsupport xml opengl +CONFIG += console c++17 +CONFIG -= app_bundle +TEMPLATE = app + +SRCDIR = ../.. + +INCLUDEPATH += . $$SRCDIR $$SRCDIR/analyzer $$SRCDIR/analyzer/updater $$SRCDIR/analyzer/usbhid/hidapi + +DEFINES += ANTSCOPE2_UNIT_TEST +DEFINES += ANTSCOPE2VER='\\"2.0.3\\"' +DEFINES += OLD_TDR +DEFINES += NEW_CONNECTION +DEFINES += NEW_ANALYZER + +SOURCES += \ + test_main.cpp \ + mock_hid.cpp \ + test_stubs.cpp \ + $$SRCDIR/analyzer/hid_analyzer.cpp \ + $$SRCDIR/analyzer/baseanalyzer.cpp \ + $$SRCDIR/AA55BTPacket.cpp \ + $$SRCDIR/crc32.cpp + +HEADERS += \ + $$SRCDIR/analyzer/hid_analyzer.h \ + $$SRCDIR/analyzer/baseanalyzer.h \ + analyzerpro.h diff --git a/tests/hid_update_mock/mock_hid.cpp b/tests/hid_update_mock/mock_hid.cpp new file mode 100644 index 0000000..887e63d --- /dev/null +++ b/tests/hid_update_mock/mock_hid.cpp @@ -0,0 +1,211 @@ +// Software-simulated RigExpert Stick 500 DFU bootloader, implementing the +// same function signatures as analyzer/usbhid/hidapi/hidapi.h, so that the +// REAL, unmodified HidAnalyzer::update() and HidAnalyzer::waitForBootDevice() +// (analyzer/hid_analyzer.cpp) can be exercised without any real hardware or +// USB stack involved. +// +// The simulated protocol (command bytes, chunk size, ACK value) matches what +// was independently verified against a real, successful update session +// captured from the genuine vendor Windows client. This is a model of that +// protocol, not the real bootloader: it validates that update()'s +// client-side logic (state machine, error handling, chunking, boot-mode +// re-detection) is internally correct. It cannot and does not validate +// real-world timing/pacing tolerance. + +#include "hidapi.h" +#include +#include +#include + +namespace { + +const unsigned short kBootVid = 0x0483; +const unsigned short kBootPid = 0xA1DA; +const wchar_t* kSerial = L"450001370"; // matches the real capture + +enum { + BL_CMD_GET_ID = 1, BL_CMD_ERASE = 2, BL_CMD_WRITE = 3, BL_CMD_DATA = 4, + BL_CMD_CHECK = 5, BL_CMD_START = 6, BL_CMD_OK = 7, BL_CMD_ERROR = 8 +}; + +bool g_resetSent = false; +int g_enumeratePolls = 0; +const int kPollsBeforeBootAvailable = 3; // simulates re-enumeration delay + +bool g_hasPendingResponse = false; +unsigned char g_pendingResponse[65]; +bool g_unreadResponseOverwritten = false; // client wrote again without reading + +int g_chunkCount = 0; +int g_failAfterChunk = -1; // -1 = never fail a chunk +bool g_checkReceived = false; +bool g_checkShouldFail = false; +bool g_startReceived = false; +int g_lastChunkDeclaredLength = 0; // data[2] of the most recent WRITE/DATA frame + +struct MockDevice { bool isBoot; }; + +} // namespace + +// ---- test-control API (not part of real hidapi; declared for the test driver) ---- +extern "C" { + void mock_hid_reset_state() + { + g_resetSent = false; + g_enumeratePolls = 0; + g_hasPendingResponse = false; + g_unreadResponseOverwritten = false; + g_chunkCount = 0; + g_failAfterChunk = -1; + g_checkReceived = false; + g_checkShouldFail = false; + g_startReceived = false; + g_lastChunkDeclaredLength = 0; + } + void mock_hid_set_fail_after_chunk(int n) { g_failAfterChunk = n; } + void mock_hid_set_check_should_fail(bool fail) { g_checkShouldFail = fail; } + bool mock_hid_reset_was_sent() { return g_resetSent; } + int mock_hid_chunk_count() { return g_chunkCount; } + bool mock_hid_check_received() { return g_checkReceived; } + bool mock_hid_start_received() { return g_startReceived; } + bool mock_hid_unread_response_overwritten() { return g_unreadResponseOverwritten; } + int mock_hid_last_chunk_declared_length() { return g_lastChunkDeclaredLength; } + // A real, safely hid_close()-able handle for the test driver to inject + // as the "already connected in normal mode" starting state, instead of + // a raw non-heap pointer that would crash hid_close()'s delete later. + hid_device* mock_hid_make_normal_mode_device() + { + MockDevice* dev = new MockDevice(); + dev->isBoot = false; + return reinterpret_cast(dev); + } +} + +// ---- hidapi surface ---- + +struct hid_device_info* HID_API_EXPORT HID_API_CALL hid_enumerate(unsigned short vid, unsigned short pid) +{ + if (vid != kBootVid || pid != kBootPid) + return nullptr; + if (!g_resetSent) + return nullptr; + + g_enumeratePolls++; + if (g_enumeratePolls < kPollsBeforeBootAvailable) + return nullptr; + + struct hid_device_info* info = new struct hid_device_info(); + memset(info, 0, sizeof(*info)); + info->vendor_id = kBootVid; + info->product_id = kBootPid; + info->serial_number = wcsdup(kSerial); + info->next = nullptr; + return info; +} + +void HID_API_EXPORT HID_API_CALL hid_free_enumeration(struct hid_device_info* devs) +{ + while (devs != nullptr) { + struct hid_device_info* next = devs->next; + free(devs->serial_number); + delete devs; + devs = next; + } +} + +HID_API_EXPORT hid_device* HID_API_CALL hid_open(unsigned short vid, unsigned short pid, const wchar_t* serial) +{ + if (vid != kBootVid || pid != kBootPid) + return nullptr; + if (!g_resetSent || g_enumeratePolls < kPollsBeforeBootAvailable) + return nullptr; + if (serial != nullptr && wcscmp(serial, kSerial) != 0) + return nullptr; + + MockDevice* dev = new MockDevice(); + dev->isBoot = true; + return reinterpret_cast(dev); +} + +void HID_API_EXPORT HID_API_CALL hid_close(hid_device* device) +{ + delete reinterpret_cast(device); +} + +int HID_API_EXPORT HID_API_CALL hid_set_nonblocking(hid_device*, int) +{ + return 0; +} + +int HID_API_EXPORT HID_API_CALL hid_write(hid_device* device, const unsigned char* data, size_t length) +{ + if (device == nullptr || data == nullptr) + return -1; + + // RESET: buff[0]=1, "RESET" ascii at buff[1..5] (see + // HidAnalyzer::update()). This is what triggers the simulated + // device to start "re-enumerating" as the boot-mode device. + if (length >= 6 && data[0] == 1 && memcmp(&data[1], "RESET", 5) == 0) { + g_resetSent = true; + return static_cast(length); + } + + if (length < 3) + return -1; + + unsigned char cmd = data[1]; + + if (cmd == BL_CMD_WRITE || cmd == BL_CMD_DATA) { + if (g_hasPendingResponse) + g_unreadResponseOverwritten = true; // caller didn't read the last one + g_chunkCount++; + g_lastChunkDeclaredLength = data[2]; + memset(g_pendingResponse, 0, sizeof(g_pendingResponse)); + g_pendingResponse[0] = (g_failAfterChunk >= 0 && g_chunkCount == g_failAfterChunk) + ? BL_CMD_ERROR : BL_CMD_OK; + g_hasPendingResponse = true; + return static_cast(length); + } + if (cmd == BL_CMD_CHECK) { + if (g_hasPendingResponse) + g_unreadResponseOverwritten = true; + g_checkReceived = true; + memset(g_pendingResponse, 0, sizeof(g_pendingResponse)); + g_pendingResponse[0] = g_checkShouldFail ? BL_CMD_ERROR : BL_CMD_OK; + g_hasPendingResponse = true; + return static_cast(length); + } + if (cmd == BL_CMD_START) { + g_startReceived = true; + return static_cast(length); + } + + return static_cast(length); +} + +int HID_API_EXPORT HID_API_CALL hid_read_timeout(hid_device* device, unsigned char* data, size_t length, int) +{ + if (device == nullptr) + return -1; + if (!g_hasPendingResponse) + return 0; // matches real hidapi: 0 = no data ready yet, keep polling + + memset(data, 0, length); + size_t n = length < sizeof(g_pendingResponse) ? length : sizeof(g_pendingResponse); + memcpy(data, g_pendingResponse, n); + g_hasPendingResponse = false; + return static_cast(n); +} + +int HID_API_EXPORT HID_API_CALL hid_read(hid_device* device, unsigned char* data, size_t length) +{ + // Non-blocking variant: same behavior as hid_read_timeout() here since + // this mock always has the response ready synchronously (produced + // inside hid_write() above) or not at all. + return hid_read_timeout(device, data, length, 0); +} + +HID_API_EXPORT const wchar_t* HID_API_CALL hid_error(hid_device*) +{ + return L"(mock hidapi backend - no error string)"; +} diff --git a/tests/hid_update_mock/test_main.cpp b/tests/hid_update_mock/test_main.cpp new file mode 100644 index 0000000..86e235f --- /dev/null +++ b/tests/hid_update_mock/test_main.cpp @@ -0,0 +1,177 @@ +// Drives the REAL, unmodified HidAnalyzer::update() (analyzer/hid_analyzer.cpp) +// against the mock hidapi backend in mock_hid.cpp, to validate the client-side +// logic of the fix without any real hardware: previously, only the first +// firmware-write chunk's response was ever read, so the read queue filled +// with stale OK responses and the final integrity check (BL_CMD_CHECK) +// silently read one of those instead of the device's real answer - meaning +// firmware verification always appeared to pass. This validates the state +// machine (per-chunk error handling, short final chunk, boot-mode +// re-detection after RESET), not real-world device/timing behavior - that +// was separately validated on real hardware (see the PR description). + +#include +#include +#include +#include +#include + +#include "analyzer/hid_analyzer.h" + +// hid_analyzer.cpp references these as extern; normally defined in main.cpp, +// which this test does not link against. +bool g_usbOnly = false; + +int g_showMessageBox(QWidget*, QMessageBox::Icon, QString title, QString text, + QMessageBox::StandardButtons, QMessageBox::StandardButton) +{ + printf(" [message box suppressed] %s: %s\n", qPrintable(title), qPrintable(text)); + return 0; +} + +extern "C" { + void mock_hid_reset_state(); + void mock_hid_set_fail_after_chunk(int n); + void mock_hid_set_check_should_fail(bool fail); + bool mock_hid_reset_was_sent(); + int mock_hid_chunk_count(); + bool mock_hid_check_received(); + bool mock_hid_start_received(); + bool mock_hid_unread_response_overwritten(); + int mock_hid_last_chunk_declared_length(); + hid_device* mock_hid_make_normal_mode_device(); +} + +namespace { + +int g_failures = 0; + +void expect(bool cond, const char* what) +{ + if (cond) { + printf(" PASS: %s\n", what); + } else { + printf(" FAIL: %s\n", what); + g_failures++; + } +} + +QByteArray makeFakeFirmware(int sizeBytes) +{ + QByteArray data; + data.resize(sizeBytes); + for (int i = 0; i < sizeBytes; i++) { + data[i] = static_cast(i & 0xFF); + } + return data; +} + +} // namespace + +int main(int argc, char* argv[]) +{ + QCoreApplication app(argc, argv); + + // --- Scenario 1: clean successful update --- + printf("Scenario 1: successful update\n"); + { + mock_hid_reset_state(); + HidAnalyzer analyzer; + analyzer.setTestHidDevice(mock_hid_make_normal_mode_device(), "450001370"); + + QByteArray fw = makeFakeFirmware(48 * 10); // 10 clean chunks + QBuffer buf(&fw); + buf.open(QIODevice::ReadOnly); + + bool ok = analyzer.update(&buf); + + expect(ok, "update() returns true"); + expect(mock_hid_reset_was_sent(), "RESET was sent"); + expect(mock_hid_chunk_count() == 10, "all 10 chunks were written"); + expect(mock_hid_check_received(), "BL_CMD_CHECK was sent"); + expect(mock_hid_start_received(), "BL_CMD_START was sent after a passing CHECK"); + expect(!mock_hid_unread_response_overwritten(), + "no response was ever left unread before the next write (the bug this fixes)"); + } + + printf("\n"); + + // --- Scenario 2: a chunk fails partway through (simulated BL_CMD_ERROR) --- + printf("Scenario 2: BL_CMD_ERROR on chunk 5 of 10\n"); + { + mock_hid_reset_state(); + mock_hid_set_fail_after_chunk(5); + HidAnalyzer analyzer; + analyzer.setTestHidDevice(mock_hid_make_normal_mode_device(), "450001370"); + + QByteArray fw = makeFakeFirmware(48 * 10); + QBuffer buf(&fw); + buf.open(QIODevice::ReadOnly); + + bool ok = analyzer.update(&buf); + + expect(!ok, "update() returns false"); + expect(mock_hid_chunk_count() == 5, "stopped at chunk 5, did not send the remaining 5"); + expect(!mock_hid_check_received(), "BL_CMD_CHECK was never sent after a chunk error"); + expect(!mock_hid_start_received(), "BL_CMD_START was never sent after a chunk error"); + } + + printf("\n"); + + // --- Scenario 3: CHECK itself fails (device reports bad checksum) --- + printf("Scenario 3: BL_CMD_CHECK reports failure\n"); + { + mock_hid_reset_state(); + mock_hid_set_check_should_fail(true); + HidAnalyzer analyzer; + analyzer.setTestHidDevice(mock_hid_make_normal_mode_device(), "450001370"); + + QByteArray fw = makeFakeFirmware(48 * 10); + QBuffer buf(&fw); + buf.open(QIODevice::ReadOnly); + + bool ok = analyzer.update(&buf); + + expect(!ok, "update() returns false"); + expect(mock_hid_chunk_count() == 10, "all chunks were still written"); + expect(mock_hid_check_received(), "BL_CMD_CHECK was sent"); + expect(!mock_hid_start_received(), + "BL_CMD_START was NOT sent after a failing CHECK " + "(this is the exact defect the fix closes: a stale queued OK " + "previously made this check always pass)"); + } + + printf("\n"); + + // --- Scenario 4: firmware size is not a multiple of 48 (the real + // Stick230 recovery image: 227216-byte payload, 227216 % 48 == 32) --- + // Uses a scaled-down size with the same remainder shape (3 full chunks + // + one 32-byte tail) so the test stays fast; the chunking arithmetic + // in update() doesn't care about absolute size. + printf("Scenario 4: firmware size not a multiple of 48 (short final chunk)\n"); + { + mock_hid_reset_state(); + HidAnalyzer analyzer; + analyzer.setTestHidDevice(mock_hid_make_normal_mode_device(), "450001370"); + + const int kFullChunks = 3; + const int kTailBytes = 32; + QByteArray fw = makeFakeFirmware(48 * kFullChunks + kTailBytes); + QBuffer buf(&fw); + buf.open(QIODevice::ReadOnly); + + bool ok = analyzer.update(&buf); + + expect(ok, "update() returns true"); + expect(mock_hid_chunk_count() == kFullChunks + 1, + "sent 3 full 48-byte chunks plus one short tail chunk"); + expect(mock_hid_last_chunk_declared_length() == kTailBytes, + "the final chunk declared its true length (32), not 48 " + "(this is the exact path the Stick230 recovery image exercises " + "that the Stick500 image, an exact multiple of 48, never did)"); + expect(mock_hid_check_received() && mock_hid_start_received(), + "CHECK and START both sent after a short final chunk"); + } + + printf("\n%s\n", g_failures == 0 ? "ALL SCENARIOS PASSED" : "SOME SCENARIOS FAILED"); + return g_failures == 0 ? 0 : 1; +} diff --git a/tests/hid_update_mock/test_stubs.cpp b/tests/hid_update_mock/test_stubs.cpp new file mode 100644 index 0000000..22fd200 --- /dev/null +++ b/tests/hid_update_mock/test_stubs.cpp @@ -0,0 +1,12 @@ +// Minimal definitions for a few static members that HidAnalyzer's +// translation unit references transitively (through analyzerpro.h / +// analyzerparameters.h) but this test never actually exercises. Providing +// these directly avoids pulling in their real owners (selectdevicedialog.cpp, +// analyzerpro.cpp), which carry a large, unrelated UI/application dependency +// graph that has nothing to do with what's under test here. + +#include "analyzer/analyzerparameters.h" + +SelectionParameters SelectionParameters::selected; +QList AnalyzerParameters::m_analyzers; +AnalyzerParameters* AnalyzerParameters::m_current = nullptr;