Description
Basic Infos
- This issue complies with the issue POLICY doc.
- I have read the documentation at readthedocs and the issue is not addressed there.
- I have tested that the issue is present in current master branch (aka latest git).
- I have searched the issue tracker for a similar issue.
- If there is a stack dump, I have decoded it.
- I have filled out all fields below.
Platform
- Hardware: All
- Core Version: Latest auto install
- Development Env: Arduino IDE
- Operating System: Ubuntu
Settings in IDE
- Module: Generic ESP8266 Module
- Flash Mode: qio
- Flash Size: 4MB
- lwip Variant: v2 Lower Memory
- Reset Method: ck
- Flash Frequency: 40Mhz
- CPU Frequency: 80Mhz|
- Upload Using: SERIAL
- Upload Speed: 115200
Problem Description
There is an existing bug in the code for the Updater class that is still present in the latest GIT source available online. The error is in the write() method and more particularly the check for running out of space with:
if(len > remaining()){I have been developing my own code to update OTA via a web server ESP8266WebServer that then calls Update.begin(), Update.write(), Update.end() and if I upload MORE bytes than I specify in begin(), write() does NOT trigger an error and continues writing to flash, eventually overwriting the SPIFFS directory.
This may be an issue with the _bufferSize = FLASH_SECTOR_SIZE = 4096 and the buffer sent to write being 2048 which are exact multiples of each other or it may be generically incorrect
The bug can be verified by walking through an example where _size = 5000, and manually considering what happens on each call to write() with a buffer of 2048
Call 1 -> 2048 bytes get stored in buffer, method returns 2048
Call 2 -> 2048 bytes get stored, buffer is full but NOT written yet, method returns 2048
Call 3 -> 2048 is now too many, but the test succeeds. Now the 4096 byte buffer gets flashed, the next 2048 bytes get stored in the buffer, and remaining() will forever more return a negative number (size_t being unsigned this means a huge number) and an unlimited amount of bytes can be written
I believe the test should be changed to check if written (progress() + bytes in buffer + len is greater than size
if (progress() + _bufferLen + len > _size) {Why hasn't this been caught
It appears that most examples just use the:
uint32_t maxSketchSpace = (ESP.getFreeSketchSpace() - 0x1000) & 0xFFFFF000;to pass to begin() which means that the file will typically finish before the problem is seen