Skip to content
New issue

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

By clicking “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? Sign in to your account

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

Open
hash6iron opened this issue Sep 25, 2024 · 7 comments

Comments

@hash6iron
Copy link

hash6iron commented Sep 25, 2024

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"

Copy link

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.

@github-actions github-actions bot added the Stale label Oct 26, 2024
@Tamairon
Copy link

Hello.

Any solution?

@mathieucarbou
Copy link
Contributor

mathieucarbou commented Oct 26, 2024

@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:

  • either when we reached the final byte
  • either when we reach the end of the temporary buffer

So:

  • len is the number of new bytes that can be read in the buffer
  • index is the position where to write in the file
  • When final is true, the file size can be determine with: index+len
  • _itemSize will be the file size at the end (and is put in the request parameters) at the end:

_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);
      }
  • using request->contentLength() is a bug: the content length of a multipart upload is the number of bytes in the body, which is not the number of bytes of the file sent.
  • using upload.totalSize is also wrong: upload.totalSize is the total accumulated bytes, not the final file size

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
Copy link
Contributor

mathieucarbou commented Oct 26, 2024

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
Copy link
Author

@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
Copy link
Contributor

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants