OTA: verify data stream and properly handle errors (#2067)

* webui: improve updater error message

* ota: properly check updater state and finalize on errors

* finish up ota handling refactor

* fail web upgrade when cannot find magic byte

* send updater status with no payload arg and always send payload with

* mention magic byte and flash mode in webui

* gzip magic

* make async ota at least usable. timeouts, show errors, try harder to find data after headers

* simplify setup and destruction

* cleanup class instantiations for ota and url

* handle default case of magicflashchipsize

* shorter

* blobs

code/espurna/libs/URL.h

@@ -8,28 +8,27 @@
 
 #pragma once
 
-struct URL {
-    String value;
-    String protocol;
-    String host;
-    String path;
-    uint16_t port;
-    
-    URL(const char* url) { init(url); }
-    URL(const String& url) { init(url); }
+class URL {
+    public:
+        URL(const String&);
 
-    void init(String url);
+        String protocol;
+        String host;
+        String path;
+        uint16_t port;
+
+    private:
+        String buffer;
 };
 
-void URL::init(String url) {
 
-    this->value = url;
+URL::URL(const String& url) : buffer(url) {
 
     // cut the protocol part
-    int index = url.indexOf("://");
+    int index = buffer.indexOf("://");
     if (index > 0) {
-        this->protocol = url.substring(0, index);
-        url.remove(0, (index + 3));
+        this->protocol = buffer.substring(0, index);
+        buffer.remove(0, (index + 3));
     }
 
     if (this->protocol == "http") {
@@ -41,17 +40,17 @@ void URL::init(String url) {
     // cut the host part
     String _host;
 
-    index = url.indexOf('/');
+    index = buffer.indexOf('/');
     if (index >= 0) {
-        _host = url.substring(0, index);
+        _host = buffer.substring(0, index);
     } else {
-        _host = url;
+        _host = buffer;
     }
 
     // store the remaining part as path
     if (index >= 0) {
-        url.remove(0, index);
-        this->path = url;
+        buffer.remove(0, index);
+        this->path = buffer;
     } else {
         this->path = "/";
     }

code/espurna/ota.h

@@ -0,0 +1,76 @@
+/*
+
+OTA COMMON FUNCTIONS
+
+*/
+
+#pragma once
+
+#include <Updater.h>
+
+void otaPrintError() {
+    if (Update.hasError()) {
+        #if TERMINAL_SUPPORT
+            Update.printError(terminalSerial());
+        #elif DEBUG_SERIAL_SUPPORT && defined(DEBUG_PORT)
+            Update.printError(DEBUG_PORT);
+        #endif
+    }
+}
+
+bool otaFinalize(size_t size, int reason, bool evenIfRemaining = false) {
+    if (Update.isRunning() && Update.end(evenIfRemaining)) {
+        DEBUG_MSG_P(PSTR("[OTA] Success: %7u bytes\n"), size);
+        deferredReset(500, reason);
+        return true;
+    }
+
+    otaPrintError();
+    eepromRotate(true);
+
+    return false;
+}
+
+// Helper methods from UpdaterClass that need to be called manually for async mode,
+// because we are not using Stream interface to feed it data.
+bool otaVerifyHeader(uint8_t* data, size_t len) {
+    if (len < 4) {
+        return false;
+    }
+
+    // ref: https://github.com/esp8266/Arduino/pull/6820
+    // accept gzip, let unpacker figure things out later
+    if (data[0] == 0x1F && data[1] == 0x8B) {
+        return true;
+    }
+
+    // Check for magic byte with a normal .bin
+    if (data[0] != 0xE9) {
+        return false;
+    }
+
+    // Make sure that flash config can be recognized and fit the flash
+    const auto flash_config = ESP.magicFlashChipSize((data[3] & 0xf0) >> 4);
+    if (flash_config && (flash_config > ESP.getFlashChipRealSize())) {
+        return false;
+    }
+
+    return true;
+}
+
+void otaProgress(size_t bytes, size_t each = 8192u) {
+    // Removed to avoid websocket ping back during upgrade (see #1574)
+    // TODO: implement as separate from debugging message
+    if (wsConnected()) return;
+
+    // Telnet and serial will still output things, but slightly throttled
+    static size_t last = 0;
+    if (bytes < last) {
+        last = 0;
+    }
+
+    if ((bytes > each) && (bytes - each > last)) {
+        DEBUG_MSG_P(PSTR("[OTA] Progress: %7u bytes\r"), bytes);
+        last = bytes;
+    }
+}

code/espurna/ota_asynctcp.ino

@@ -9,102 +9,145 @@ Copyright (C) 2016-2019 by Xose Pérez <xose dot perez at gmail dot com>
 #if OTA_CLIENT == OTA_CLIENT_ASYNCTCP
 
 // -----------------------------------------------------------------------------
-// Terminal OTA command
+// Terminal and MQTT OTA command handlers
 // -----------------------------------------------------------------------------
 
 #if TERMINAL_SUPPORT || OTA_MQTT_SUPPORT
 
+#include <Schedule.h>
 #include <ESPAsyncTCP.h>
 
 #include "system.h"
-#include "libs/URL.h"
+#include "ota.h"
 
-std::unique_ptr<AsyncClient> _ota_client = nullptr;
-unsigned long _ota_size = 0;
-bool _ota_connected = false;
-std::unique_ptr<URL> _ota_url = nullptr;
+#include "libs/URL.h"
 
 const char OTA_REQUEST_TEMPLATE[] PROGMEM =
     "GET %s HTTP/1.1\r\n"
     "Host: %s\r\n"
     "User-Agent: ESPurna\r\n"
-    "Connection: close\r\n"
-    "Content-Type: application/x-www-form-urlencoded\r\n"
-    "Content-Length: 0\r\n\r\n\r\n";
+    "Connection: close\r\n\r\n";
 
-void _otaClientOnDisconnect(void *s, AsyncClient *c) {
+struct ota_client_t {
+    enum state_t {
+        HEADERS,
+        DATA,
+        END
+    };
 
-    DEBUG_MSG_P(PSTR("\n"));
+    ota_client_t() = delete;
+    ota_client_t(const ota_client_t&) = delete;
 
-    if (Update.end(true)){
-        DEBUG_MSG_P(PSTR("[OTA] Success: %u bytes\n"), _ota_size);
-        deferredReset(100, CUSTOM_RESET_OTA);
-    } else {
-        #ifdef DEBUG_PORT
-            Update.printError(DEBUG_PORT);
-        #endif
-        eepromRotate(true);
-    }
+    ota_client_t(URL&& url);
 
-    DEBUG_MSG_P(PSTR("[OTA] Disconnected\n"));
+    bool connect();
+
+    state_t state = HEADERS;
+    size_t size = 0;
+
+    const URL url;
+    AsyncClient client;
+};
 
-    _ota_connected = false;
-    _ota_url = nullptr;
+std::unique_ptr<ota_client_t> _ota_client = nullptr;
+
+// -----------------------------------------------------------------------------
+
+void _otaClientDisconnect() {
+    DEBUG_MSG_P(PSTR("[OTA] Disconnected\n"));
     _ota_client = nullptr;
+}
 
+void _otaClientOnDisconnect(void* arg, AsyncClient* client) {
+    DEBUG_MSG_P(PSTR("\n"));
+    otaFinalize(reinterpret_cast<ota_client_t*>(arg)->size, CUSTOM_RESET_OTA, true);
+    schedule_function(_otaClientDisconnect);
 }
 
-void _otaClientOnTimeout(void *s, AsyncClient *c, uint32_t time) {
-    _ota_connected = false;
-    _ota_url = nullptr;
-    _ota_client->close(true);
+void _otaClientOnTimeout(void*, AsyncClient * client, uint32_t) {
+    client->close(true);
 }
 
-void _otaClientOnData(void * arg, AsyncClient * c, void * data, size_t len) {
+void _otaClientOnError(void*, AsyncClient* client, err_t error) {
+    DEBUG_MSG_P(PSTR("[OTA] ERROR: %s\n"), client->errorToString(error));
+}
 
-    char * p = (char *) data;
+void _otaClientOnData(void* arg, AsyncClient* client, void* data, size_t len) {
 
-    if (_ota_size == 0) {
+    ota_client_t* ota_client = reinterpret_cast<ota_client_t*>(arg);
+    auto* ptr = (char *) data;
 
-        Update.runAsync(true);
-        if (!Update.begin((ESP.getFreeSketchSpace() - 0x1000) & 0xFFFFF000)) {
-            #ifdef DEBUG_PORT
-                Update.printError(DEBUG_PORT);
-            #endif
-            c->close(true);
+    // TODO: check status
+    // TODO: check for 3xx, discover `Location:` header and schedule
+    //       another _otaClientFrom(location_header_url)
+    if (_ota_client->state == ota_client_t::HEADERS) {
+        ptr = (char *) strnstr((char *) data, "\r\n\r\n", len);
+        if (!ptr) {
             return;
         }
+        auto diff = ptr - ((char *) data);
 
-        p = strstr((char *)data, "\r\n\r\n") + 4;
-        len = len - (p - (char *) data);
-
+        _ota_client->state = ota_client_t::DATA;
+        len -= diff + 4;
+        if (!len) {
+            return;
+        }
+        ptr += 4;
     }
 
-    if (!Update.hasError()) {
-        if (Update.write((uint8_t *) p, len) != len) {
-            #ifdef DEBUG_PORT
-                Update.printError(DEBUG_PORT);
-            #endif
-            c->close(true);
+    if (ota_client->state == ota_client_t::DATA) {
+
+        if (!ota_client->size) {
+
+            // Check header before anything is written to the flash
+            if (!otaVerifyHeader((uint8_t *) ptr, len)) {
+                DEBUG_MSG_P(PSTR("[OTA] ERROR: No magic byte / invalid flash config"));
+                client->close(true);
+                ota_client->state = ota_client_t::END;
+                return;
+            }
+
+            // XXX: In case of non-chunked response, really parse headers and specify size via content-length value
+            Update.runAsync(true);
+            if (!Update.begin((ESP.getFreeSketchSpace() - 0x1000) & 0xFFFFF000)) {
+                otaPrintError();
+                client->close(true);
+                return;
+            }
+
+        }
+
+        // We can enter this callback even after client->close()
+        if (!Update.isRunning()) {
             return;
         }
-    }
 
-    _ota_size += len;
-    DEBUG_MSG_P(PSTR("[OTA] Progress: %u bytes\r"), _ota_size);
+        if (Update.write((uint8_t *) ptr, len) != len) {
+            otaPrintError();
+            client->close(true);
+            ota_client->state = ota_client_t::END;
+            return;
+        }
+
+        ota_client->size += len;
+        otaProgress(ota_client->size);
 
-    delay(0);
+        delay(0);
+
+    }
 
 }
 
-void _otaClientOnConnect(void *arg, AsyncClient *client) {
+void _otaClientOnConnect(void* arg, AsyncClient* client) {
+
+    ota_client_t* ota_client = reinterpret_cast<ota_client_t*>(arg);
 
     #if ASYNC_TCP_SSL_ENABLED
         int check = getSetting("otaScCheck", OTA_SECURE_CLIENT_CHECK).toInt();
         if ((check == SECURE_CLIENT_CHECK_FINGERPRINT) && (443 == _ota_url->port)) {
             uint8_t fp[20] = {0};
             sslFingerPrintArray(getSetting("otaFP", OTA_FINGERPRINT).c_str(), fp);
-            SSL * ssl = _ota_client->getSSL();
+            SSL * ssl = client->getSSL();
             if (ssl_match_fingerprint(ssl, fp) != SSL_OK) {
                 DEBUG_MSG_P(PSTR("[OTA] Warning: certificate fingerpint doesn't match\n"));
                 client->close(true);
@@ -116,65 +159,55 @@ void _otaClientOnConnect(void *arg, AsyncClient *client) {
     // Disabling EEPROM rotation to prevent writing to EEPROM after the upgrade
     eepromRotate(false);
 
-    DEBUG_MSG_P(PSTR("[OTA] Downloading %s\n"), _ota_url->path.c_str());
-    char buffer[strlen_P(OTA_REQUEST_TEMPLATE) + _ota_url->path.length() + _ota_url->host.length()];
-    snprintf_P(buffer, sizeof(buffer), OTA_REQUEST_TEMPLATE, _ota_url->path.c_str(), _ota_url->host.c_str());
+    DEBUG_MSG_P(PSTR("[OTA] Downloading %s\n"), ota_client->url.path.c_str());
+    char buffer[strlen_P(OTA_REQUEST_TEMPLATE) + ota_client->url.path.length() + ota_client->url.host.length()];
+    snprintf_P(buffer, sizeof(buffer), OTA_REQUEST_TEMPLATE, ota_client->url.path.c_str(), ota_client->url.host.c_str());
     client->write(buffer);
 }
 
+ota_client_t::ota_client_t(URL&& url) :
+    url(std::move(url))
+{
+    client.setRxTimeout(5);
+    client.onError(_otaClientOnError, nullptr);
+    client.onTimeout(_otaClientOnTimeout, nullptr);
+    client.onDisconnect(_otaClientOnDisconnect, this);
+    client.onData(_otaClientOnData, this);
+    client.onConnect(_otaClientOnConnect, this);
+}
+
+bool ota_client_t::connect() {
+    #if ASYNC_TCP_SSL_ENABLED
+        return client.connect(url.host.c_str(), url.port, 443 == url.port);
+    #else
+        return client.connect(url.host.c_str(), url.port);
+    #endif
+}
+
+// -----------------------------------------------------------------------------
+
 void _otaClientFrom(const String& url) {
 
-    if (_ota_connected) {
+    if (_ota_client) {
         DEBUG_MSG_P(PSTR("[OTA] Already connected\n"));
         return;
     }
 
-    _ota_size = 0;
-
-    if (_ota_url) _ota_url = nullptr;
-    _ota_url = std::make_unique<URL>(url);
-    /*
-    DEBUG_MSG_P(PSTR("[OTA] proto:%s host:%s port:%u path:%s\n"),
-        _ota_url->protocol.c_str(),
-        _ota_url->host.c_str(),
-        _ota_url->port,
-        _ota_url->path.c_str()
-    );
-    */
-
-    // we only support HTTP
-    if ((!_ota_url->protocol.equals("http")) && (!_ota_url->protocol.equals("https"))) {
+    URL _url(url);
+    if (!_url.protocol.equals("http") && !_url.protocol.equals("https")) {
         DEBUG_MSG_P(PSTR("[OTA] Incorrect URL specified\n"));
-        _ota_url = nullptr;
         return;
     }
 
-    if (!_ota_client) {
-        _ota_client = std::make_unique<AsyncClient>();
-    }
-
-    _ota_client->onDisconnect(_otaClientOnDisconnect, nullptr);
-    _ota_client->onTimeout(_otaClientOnTimeout, nullptr);
-    _ota_client->onData(_otaClientOnData, nullptr);
-    _ota_client->onConnect(_otaClientOnConnect, nullptr);
-
-    #if ASYNC_TCP_SSL_ENABLED
-        _ota_connected = _ota_client->connect(_ota_url->host.c_str(), _ota_url->port, 443 == _ota_url->port);
-    #else
-        _ota_connected = _ota_client->connect(_ota_url->host.c_str(), _ota_url->port);
-    #endif
-
-    if (!_ota_connected) {
+    _ota_client = std::make_unique<ota_client_t>(std::move(_url));
+    if (!_ota_client->connect()) {
         DEBUG_MSG_P(PSTR("[OTA] Connection failed\n"));
-        _ota_url = nullptr;
-        _ota_client->close(true);
     }
 
 }
 
 #endif // TERMINAL_SUPPORT || OTA_MQTT_SUPPORT
 
-
 #if TERMINAL_SUPPORT
 
 void _otaClientInitCommands() {

code/espurna/web.ino

@@ -10,6 +10,7 @@ Copyright (C) 2016-2019 by Xose Pérez <xose dot perez at gmail dot com>
 
 #include "system.h"
 #include "utils.h"
+#include "ota.h"
 
 #include <ESPAsyncTCP.h>
 #include <ESPAsyncWebServer.h>
@@ -76,8 +77,6 @@ void _onDiscover(AsyncWebServerRequest *request) {
 
     webLog(request);
 
-    AsyncResponseStream *response = request->beginResponseStream("application/json");
-
     const String device = getBoardName();
     const String hostname = getSetting("hostname");
 
@@ -87,6 +86,8 @@ void _onDiscover(AsyncWebServerRequest *request) {
     root["version"] = APP_VERSION;
     root["device"] = device.c_str();
     root["hostname"] = hostname.c_str();
+
+    AsyncResponseStream *response = request->beginResponseStream("application/json", root.measureLength() + 1);
     root.printTo(*response);
 
     request->send(response);
@@ -100,7 +101,7 @@ void _onGetConfig(AsyncWebServerRequest *request) {
         return request->requestAuthentication(getSetting("hostname").c_str());
     }
 
-    AsyncResponseStream *response = request->beginResponseStream("text/json");
+    AsyncResponseStream *response = request->beginResponseStream("application/json");
 
     char buffer[100];
     snprintf_P(buffer, sizeof(buffer), PSTR("attachment; filename=\"%s-backup.json\""), (char *) getSetting("hostname").c_str());
@@ -137,7 +138,7 @@ void _onPostConfig(AsyncWebServerRequest *request) {
     request->send(_webConfigSuccess ? 200 : 400);
 }
 
-void _onPostConfigData(AsyncWebServerRequest *request, String filename, size_t index, uint8_t *data, size_t len, bool final) {
+void _onPostConfigFile(AsyncWebServerRequest *request, String filename, size_t index, uint8_t *data, size_t len, bool final) {
 
     if (!webAuthenticate(request)) {
         return request->requestAuthentication(getSetting("hostname").c_str());
@@ -287,6 +288,39 @@ int _onCertificate(void * arg, const char *filename, uint8_t **buf) {
 
 #endif
 
+void _onUpgradeResponse(AsyncWebServerRequest *request, int code, const String& payload = "") {
+
+    auto *response = request->beginResponseStream("text/plain", 256);
+    response->addHeader("Connection", "close");
+    response->addHeader("X-XSS-Protection", "1; mode=block");
+    response->addHeader("X-Content-Type-Options", "nosniff");
+    response->addHeader("X-Frame-Options", "deny");
+
+    response->setCode(code);
+
+    if (payload.length()) {
+        response->printf("%s", payload.c_str());
+    } else {
+        if (!Update.hasError()) {
+            response->print("OK");
+        } else {
+            #if defined(ARDUINO_ESP8266_RELEASE_2_3_0)
+                Update.printError(reinterpret_cast<Stream&>(response));
+            #else
+                Update.printError(*response);
+            #endif
+        }
+    }
+
+    request->send(response);
+
+}
+
+void _onUpgradeStatusSet(AsyncWebServerRequest *request, int code, const String& payload = "") {
+    _onUpgradeResponse(request, code, payload);
+    request->_tempObject = malloc(sizeof(bool));
+}
+
 void _onUpgrade(AsyncWebServerRequest *request) {
 
     webLog(request);
@@ -294,24 +328,11 @@ void _onUpgrade(AsyncWebServerRequest *request) {
         return request->requestAuthentication(getSetting("hostname").c_str());
     }
 
-    char buffer[10];
-    if (!Update.hasError()) {
-        sprintf_P(buffer, PSTR("OK"));
-    } else {
-        sprintf_P(buffer, PSTR("ERROR %d"), Update.getError());
+    if (request->_tempObject) {
+        return;
     }
 
-    AsyncWebServerResponse *response = request->beginResponse(200, "text/plain", buffer);
-    response->addHeader("Connection", "close");
-    response->addHeader("X-XSS-Protection", "1; mode=block");
-    response->addHeader("X-Content-Type-Options", "nosniff");
-    response->addHeader("X-Frame-Options", "deny");
-    if (Update.hasError()) {
-        eepromRotate(true);
-    } else {
-        deferredReset(100, CUSTOM_RESET_UPGRADE);
-    }
-    request->send(response);
+    _onUpgradeResponse(request, 200);
 
 }
 
@@ -321,43 +342,71 @@ void _onUpgradeFile(AsyncWebServerRequest *request, String filename, size_t inde
         return request->requestAuthentication(getSetting("hostname").c_str());
     }
 
+    // We set this after we are done with the request
+    // It is still possible to re-enter this callback even after connection is already closed
+    // 1.14.2: TODO: see https://github.com/me-no-dev/ESPAsyncWebServer/pull/660
+    // remote close or request sending some data before finishing parsing of the body will leak 1460 bytes
+    // waiting a bit for upstream. fork and point to the fixed version if not resolved before 1.14.2
+    if (request->_tempObject) {
+        return;
+    }
+
     if (!index) {
 
+        // TODO: stop network activity completely when handling Update through ArduinoOTA or `ota` command?
+        if (Update.isRunning()) {
+            _onUpgradeStatusSet(request, 400, F("ERROR: Upgrade in progress"));
+            return;
+        }
+
+        // Check that header is correct and there is more data before anything is written to the flash
+        if (final || !len) {
+            _onUpgradeStatusSet(request, 400, F("ERROR: Invalid request"));
+            return;
+        }
+
+        if (!otaVerifyHeader(data, len)) {
+            _onUpgradeStatusSet(request, 400, F("ERROR: No magic byte / invalid flash config"));
+            return;
+        }
+
         // Disabling EEPROM rotation to prevent writing to EEPROM after the upgrade
         eepromRotate(false);
 
         DEBUG_MSG_P(PSTR("[UPGRADE] Start: %s\n"), filename.c_str());
         Update.runAsync(true);
+
+        // Note: cannot use request->contentLength() for multipart/form-data
         if (!Update.begin((ESP.getFreeSketchSpace() - 0x1000) & 0xFFFFF000)) {
-            #ifdef DEBUG_PORT
-                Update.printError(DEBUG_PORT);
-            #endif
+            _onUpgradeStatusSet(request, 500);
+            eepromRotate(true);
+            return;
         }
 
     }
 
-    if (!Update.hasError()) {
-        if (Update.write(data, len) != len) {
-            #ifdef DEBUG_PORT
-                Update.printError(DEBUG_PORT);
-            #endif
-        }
+    if (request->_tempObject) {
+        return;
+    }
+
+    // Any error will cancel the update, but request may still be alive
+    if (!Update.isRunning()) {
+        return;
+    }
+
+    if (Update.write(data, len) != len) {
+        _onUpgradeStatusSet(request, 500);
+        Update.end();
+        eepromRotate(true);
+        return;
     }
 
     if (final) {
-        if (Update.end(true)){
-            DEBUG_MSG_P(PSTR("[UPGRADE] Success:  %u bytes\n"), index + len);
-        } else {
-            #ifdef DEBUG_PORT
-                Update.printError(DEBUG_PORT);
-            #endif
-        }
+        otaFinalize(index + len, CUSTOM_RESET_UPGRADE, true);
     } else {
-        // Removed to avoid websocket ping back during upgrade (see #1574)
-        // TODO: implement as separate from debugging message
-        if (wsConnected()) return;
-        DEBUG_MSG_P(PSTR("[UPGRADE] Progress: %u bytes\r"), index + len);
+        otaProgress(index + len);
     }
+
 }
 
 bool _onAPModeRequest(AsyncWebServerRequest *request) {
@@ -423,10 +472,7 @@ void _onBody(AsyncWebServerRequest *request, uint8_t *data, size_t len, size_t i
 
 bool webAuthenticate(AsyncWebServerRequest *request) {
     #if USE_PASSWORD
-        String password = getAdminPass();
-        char httpPassword[password.length() + 1];
-        password.toCharArray(httpPassword, password.length() + 1);
-        return request->authenticate(WEB_USERNAME, httpPassword);
+        return request->authenticate(WEB_USERNAME, getAdminPass().c_str());
     #else
         return true;
     #endif
@@ -478,7 +524,7 @@ void webSetup() {
     // Other entry points
     _server->on("/reset", HTTP_GET, _onReset);
     _server->on("/config", HTTP_GET, _onGetConfig);
-    _server->on("/config", HTTP_POST | HTTP_PUT, _onPostConfig, _onPostConfigData);
+    _server->on("/config", HTTP_POST | HTTP_PUT, _onPostConfig, _onPostConfigFile);
     _server->on("/upgrade", HTTP_POST, _onUpgrade, _onUpgradeFile);
     _server->on("/discover", HTTP_GET, _onDiscover);
 

code/html/custom.js

@@ -561,9 +561,22 @@ function checkFirmware(file, callback) {
 
     reader.onloadend = function(evt) {
         if (FileReader.DONE === evt.target.readyState) {
-            if (0xE9 !== evt.target.result.charCodeAt(0)) callback(false);
-            if (0x03 !== evt.target.result.charCodeAt(2)) {
-                var response = window.confirm("Binary image is not using DOUT flash mode. This might cause resets in some devices. Press OK to continue.");
+            var magic = evt.target.result.charCodeAt(0);
+            if ((0x1F === magic) && (0x8B === evt.target.result.charCodeAt(1))) {
+                callback(true);
+                return;
+            }
+
+            if (0xE9 !== magic) {
+                alert("Binary image does not start with a magic byte");
+                callback(false);
+                return;
+            }
+
+            var modes = ['QIO', 'QOUT', 'DIO', 'DOUT'];
+            var flash_mode = evt.target.result.charCodeAt(2);
+            if (0x03 !== flash_mode) {
+                var response = window.confirm("Binary image is using " + modes[flash_mode] + " flash mode! Make sure that the device supports it before proceeding.");
                 callback(response);
             } else {
                 callback(true);
@@ -593,7 +606,6 @@ function doUpgrade() {
     checkFirmware(file, function(ok) {
 
         if (!ok) {
-            alert("The file does not seem to be a valid firmware image.");
             return;
         }
 
@@ -602,8 +614,11 @@ function doUpgrade() {
 
         var xhr = new XMLHttpRequest();
 
-        var network_error = function() {
-            alert("There was a network error trying to upload the new image, please try again.");
+        var msg_ok = "Firmware image uploaded, board rebooting. This page will be refreshed in 5 seconds.";
+        var msg_err = "There was an error trying to upload the new image, please try again: ";
+
+        var network_error = function(e) {
+            alert(msg_err + " xhr request " + e.type);
         };
         xhr.addEventListener("error", network_error, false);
         xhr.addEventListener("abort", network_error, false);
@@ -611,12 +626,10 @@ function doUpgrade() {
         xhr.addEventListener("load", function(e) {
             $("#upgrade-progress").hide();
             if ("OK" === xhr.responseText) {
-                alert("Firmware image uploaded, board rebooting. This page will be refreshed in 5 seconds.");
+                alert(msg_ok);
                 doReload(5000);
             } else {
-                alert("There was an error trying to upload the new image, please try again ("
-                    + "response: " + xhr.responseText + ", "
-                    + "status: " + xhr.statusText + ")");
+                alert(msg_err + xhr.status.toString() + " " + xhr.statusText + ", " + xhr.responseText);
             }
         }, false);