ayushsharma82 / ElegantOTA

OTA updates made slick and simple for everyone!
https://elegantota.pro
GNU Affero General Public License v3.0
643 stars 119 forks source link

totalSize and currentSize return both the same value, the value of currentSize. #223

Open hash6iron opened 1 month ago

hash6iron commented 1 month ago

Hi,

I try to use "totalSize" to know the size of the file to be uploaded and "currentSize" to know how many bytes were uploaded, but currentSize and totalSize always return the same value, bytes uploaded, then is not possible to know the size of the file previously.

The version that I'm using is, "ElegantOTA @ 3.1.5"

github-actions[bot] commented 3 weeks ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

Tamairon commented 3 weeks ago

Hello.

Any solution?

mathieucarbou commented 3 weeks ago

@ayushsharma82 @hash6iron @Tamairon : there is a bug in ElegantOTA.

Here is how the upload works behind....

The callback signature is:

[&](AsyncWebServerRequest *request, String filename, size_t index, uint8_t *data, size_t len, bool final)

The internal code is:

void AsyncWebServerRequest::_handleUploadByte(uint8_t data, bool last) {
  _itemBuffer[_itemBufferIndex++] = data;

  if (last || _itemBufferIndex == RESPONSE_STREAM_BUFFER_SIZE) {
    // check if authenticated before calling the upload
    if (_handler)
      _handler->handleUpload(this, _itemFilename, _itemSize - _itemBufferIndex, _itemBuffer, _itemBufferIndex, false);
    _itemBufferIndex = 0;
  }
}

RESPONSE_STREAM_BUFFER_SIZE is 1460: this size is set based on the max TCP packet size (CONFIG_TCP_MSS).

The callback is called:

So:

_params.emplace_back(_itemName, _itemFilename, true, true, _itemSize);

With ESpAsyncWS the code is doing:

        if(len){
            if (Update.write(data, len) != len) {
                return request->send(400, "text/plain", "Failed to write chunked data to free space");
            }
            _current_progress_size += len;
            // Progress update callback
            if (progressUpdateCallback != NULL) progressUpdateCallback(_current_progress_size, request->contentLength());
        }

or with WebServer:

      } else if (upload.status == UPLOAD_FILE_WRITE) {
          if (Update.write(upload.buf, upload.currentSize) != upload.currentSize) {
            #if UPDATE_DEBUG == 1
              Update.printError(Serial);
            #endif
          }

          _current_progress_size += upload.currentSize;
          // Progress update callback
          if (progressUpdateCallback != NULL) progressUpdateCallback(_current_progress_size, upload.totalSize);
      }

Generally speaking, whether in AsyncWS or Arduino WebServer, the file upload callback is called during the multipart parsing of the request so the total file size is not yet known.

The callback of ElegantOTA is wrong and should be:

std::function<void(size_t current)

instead of:

std::function<void(size_t current, size_t final)

We only know the current growing size, not the final one. And relying on final is definitely wrong.

mathieucarbou commented 3 weeks ago

Solution

There is a solution though.

The solution is to calculate the file size in javascript and when sending the multipart upload, add a header to the request, for example for 2Mb file:

X-Upload-File-Size: 2097152

Headers are always parsed BEFORE the body. So during the callback, it is possible to do: request.getHeader("X-Upload-File-Size") to get the file size from the header value. This value could even be cached in request->_tempObject to avoid subsequent search in the headers (which is a case insensitive string comparison of all items one by one).

This approach would fix the issue and allow to keep the callback as-is.

hash6iron commented 3 weeks ago

@mathieucarbou Is it possible to implement in the library? or that is a local solution? How can I apply this solution to my code? Thanks!

mathieucarbou commented 3 weeks ago

It requires a UI change so only @ayushsharma82 can fix it for the oss version. For those who have the pro version, then they can fix if they know how, or they can ask @ayushsharma82 to issue a fix.

ayushsharma82 commented 3 weeks ago

Yeah, OSS version is due for an update. I'll fix it with next release, along with #229, but it will take some time.