Skip to content

ota: fix potential network error by checking return values #4054

New issue

Have a question about this project? No Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

No Sign up for GitHub

By clicking “No Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? No Sign in to your account

Closed
wants to merge 9 commits into from
Closed

ota: fix potential network error by checking return values #4054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e38b292
ota: fix potential network error by checking return values
d-a-v Dec 31, 2017
67b7f6b
Merge branch 'master' into ota
d-a-v Oct 12, 2018
3efd93a
Merge branch 'master' into ota
d-a-v May 12, 2019
1921cc3
Merge branch 'master' into ota
d-a-v Aug 29, 2019
a8fbe1a
Merge branch 'master' into ota
earlephilhower Aug 1, 2020
b5f4714
simplify test
d-a-v Aug 2, 2020
c33566c
sanity check
d-a-v Aug 2, 2020
81ae1cd
Merge branch 'master' into ota
d-a-v Feb 27, 2021
9ccae31
Merge branch 'master' into ota
mcspr Oct 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 14 additions & 28 deletions cores/esp8266/Updater.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,38 +140,24 @@ class UpdaterClass {
*/
template<typename T>
size_t write(T &data){
size_t written = 0;
if (hasError() || !isRunning())
return 0;

size_t available = data.available();
while(available) {
if(_bufferLen + available > remaining()){
available = remaining() - _bufferLen;
}
if(_bufferLen + available > _bufferSize) {
size_t toBuff = _bufferSize - _bufferLen;
data.read(_buffer + _bufferLen, toBuff);
_bufferLen += toBuff;
if(!_writeBuffer())
return written;
written += toBuff;
} else {
data.read(_buffer + _bufferLen, available);
_bufferLen += available;
written += available;
if(_bufferLen == remaining()) {
if(!_writeBuffer()) {
return written;
}
}
}
if(remaining() == 0)
return written;
delay(1);
available = data.available();
// load exactly one _bufSize before flashing it
// (the last one may be smaller)
size_t wantedBufSize = std::min(remaining(), _bufferSize);
size_t readThisTime = 0;

while (data.available() && _bufferLen < wantedBufSize) {
size_t got = data.read(_buffer + _bufferLen, wantedBufSize - _bufferLen);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check for if (!got) return fail; due to something weird on the connection side?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semantic ensures data.read()>0 because data.available() is true, but adding a test just in case.
There is no constraint on number of bytes read, nor duration / timeout.

if (!got)
break;
_bufferLen += got;
readThisTime += got;
}
return written;
if (_bufferLen == wantedBufSize && !_writeBuffer())
return 0;
return readThisTime;
}

private:
Expand Down
3 changes: 2 additions & 1 deletion libraries/ArduinoOTA/ArduinoOTA.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ void ArduinoOTAClass::_runUpdate() {
int waited = 1000;
while (!client.available() && waited--)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a PolledTimeout, now that they exist...

delay(1);
if (!waited){
if (!client.available()){
#ifdef OTA_DEBUG
OTA_DEBUG.printf("Receive Failed\n");
#endif
Expand All @@ -303,6 +303,7 @@ void ArduinoOTAClass::_runUpdate() {
_error_callback(OTA_RECEIVE_ERROR);
}
_state = OTA_IDLE;
break;
}
written = Update.write(client);
if (written > 0) {
Expand Down