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

WebServer: rename & expose internal 'contentLength' variable for POST… #7012

Merged
merged 6 commits into from
Aug 11, 2022

Conversation

WebDust21
Copy link
Contributor

@WebDust21 WebDust21 commented Jul 19, 2022

The purpose of this PR is to expose the internal "contentHeader" of the WebServer, so user code can determine the approximate size of an incoming HTTP POST upload (file or otherwise).

Description of Change

Currently, there is no way to determine the size of a file upload to the ESP32 until after all of the data has been read. The WebServer.Upload structure does a good job providing all of the available HTTP information to the user software, but for whatever HTTP reason, the total size is not provided in the HTTP form header (see picture below):
image

It turns out that the initial HTTP POST request header does provide a general idea of how much data is incoming via the "content-length" header:
image
Yes, I am fully aware that this is not the exact size of the uploaded file (as the size of the HTTP headers are included in this count--the file I uploaded here is only 84,103 bytes)...but for larger files, this can be very useful to the user software for determining whether to process or ignore the incoming data. If someone's trying to upload their prized photo that's a 6MB JPEG into an ESP32 with only 498KB of available space, it would make sense to be able to reject the upload on the spot.

The WebServer implementation does not store the HTTP headers for the user software, but it does parse them. It turned out that the "content-length" header was parsed and internally used--but not exposed to the user.

This PR changes the scope of the internal "content-length" variable, and exposes a read-only access point in the WebServer structure.

Tests scenarios

Tested on the latest "master" (2.0.4) with an ESP32-WROOM32 on a custom PCB. Works beautifully ;-)

Example code snippit:

void uploadHandler()
{
  // This is called 
  //    Status = 0 -> when the request starts
  //    Status = 1 -> every time there is a new "chunk" of data
  //    Status = 2 -> when the last chunk of data comes in
  HTTPUpload& upload = server.upload();
  if (upload.status == 0) {
    Serial.print("Packet Length: "); Serial.println(server.clientContentLength());  // new function in this PR
  }
  
  Serial.print("ulStatus: "); Serial.println(upload.status);
  Serial.print("ulFilename: "); Serial.println(upload.filename);
  Serial.print("ulName: "); Serial.println(upload.name);
  Serial.print("ulType: "); Serial.println(upload.type);
  Serial.print("ulLen: "); Serial.println(upload.totalSize);
  Serial.print("ulBufLen: "); Serial.println(upload.currentSize);
  Serial.write(upload.buf, 50);  // just print the beginning of the buffer
  Serial.println();
}

Result when uploading a file to the ESP32:
image

AFAIK the PR should not break anything. Changes are minimal.
I am not sure what the best name for the exposed function is. To the best of my knowledge, it is only usable for incoming HTTP POST requests to the ESP32...and is not to be confused with the existing "setContentLength" function (used for setting the length of an outgoing HTTP request)

(also includes a quick code optimization, consolidating 2 IF statements on the same boolean variable into an IF-ELSE. Also couldn't quite help myself with some spaces vs tabs & lining comments up...)

@CLAassistant
Copy link

CLAassistant commented Jul 19, 2022

CLA assistant check
All committers have signed the CLA.

@vortigont
Copy link
Contributor

Nice feature, the same is already present in AsyncWebServer.
But maybe it should be better accessible as HTTPUpload struct member than HTTPServer, i.e. for GET requests it makes no sense even if reported as zero. Seems more logical to me.

@WebDust21
Copy link
Contributor Author

Nice feature, the same is already present in AsyncWebServer. But maybe it should be better accessible as HTTPUpload struct member than HTTPServer, i.e. for GET requests it makes no sense even if reported as zero. Seems more logical to me.

AsyncWebServer has everything, I just don't want to have to rewrite my entire project to switch over ;-). Plus then I'd have the old WebServer code consuming space in the binary...PLUS the AsyncWebServer.

Whatever way makes the most sense...
...my reservations with putting it in the HTTPUpload struct is that it isn't specifically limited to HTTPUpload alone. And if HTTPUpload can parse multiple file uploads in one POST stream (which I haven't tested), the "content-length" property will include the total of all files, not just the one currently being handled by said struct.

@vortigont
Copy link
Contributor

my reservations with putting it in the HTTPUpload struct is that it isn't specifically limited to HTTPUpload alone

you are right, this makes sense actually. POST could be without file-body also, i.e. large json for REST.
Anyway being able to get body size length is handy. Like it!

@P-R-O-C-H-Y P-R-O-C-H-Y added Type: Feature request Feature request for Arduino ESP32 Area: Libraries Issue is related to Library support. labels Jul 21, 2022
@VojtechBartoska VojtechBartoska added this to the 2.0.5 milestone Jul 27, 2022
@SuGlider SuGlider self-requested a review July 29, 2022 17:02
@SuGlider SuGlider self-assigned this Jul 29, 2022
@SuGlider
Copy link
Collaborator

@WebDust21 - Thanks for the contribution.
I just added a comment in the review. PTAL

@SuGlider SuGlider merged commit 0260cd6 into espressif:master Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Libraries Issue is related to Library support. Type: Feature request Feature request for Arduino ESP32
Projects
Development

Successfully merging this pull request may close these issues.

None yet

7 participants