-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Update HTTPClient.cpp to send big payload #7638
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
Conversation
Handle sending big payload data per 1000 bytes
|
// } | ||
|
||
// send the payload per 1000 bytes | ||
log_d("Sending %u byte ...\n", size); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like your PR is introducing a mix of TAB and SPACE characters resulting in a jumbled mess of formatting. Can you please reformat your additional code to match the existing formatting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@atanisoft Fixed. Please kindly check it. Thank you.
if(_client->write(&payload[0], size) != size) { | ||
return returnError(HTTPC_ERROR_SEND_PAYLOAD_FAILED); | ||
|
||
// this will fail for big payload |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of leaving the old code in comments, it would be better to simply drop it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@atanisoft Fixed. Please kindly check it. Thank you.
// send the payload per 1000 bytes | ||
log_d("Sending %u byte ...\n", size); | ||
size_t sent_bytes = 0; | ||
for (size_t id_data = 0; id_data < size; id_data+=1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of a for loop using fixed sizes for chunks, please consider a loop similar to below (note this is not tested):
while(sent_bytes < size) {
size_t sent = _client->write(&payload[sent_bytes], std::min(size - sent_bytes, _chunkSize));
if (sent < 0) {
log_e("Failed to send chunk!");
break;
}
sent_bytes += sent;
}
Note that there is ZERO guarantee that the write call will send the full block size you request it to send thus you must check that it actually sent data and only then move the data starting point. The check for negative sent byte count is to check for sending errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@atanisoft Fixed. Please kindly check it. Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ekapujiw2002 Did you take a look on comments from @atanisoft? If not, please do the changes you were asked for.
@P-R-O-C-H-Y Will do sir ASAP. Thank you. |
Fixed according to espressif#7638
@ekapujiw2002 I invite you to see the changes that I have made on the topic of large uploads here. I have tested it and also added some more safeguards and error propagation. |
closing as already implemented |
Description of Change
This update will handle sending big payload packet correctly by sending it per determined chunk size, for example per 1000 bytes. Also fix content length header on subsequence redirection request.
Tests scenarios
Tested on ESP32-CAM AiThinker using Arduino-esp32 core v3.3.1 for sending photo as base64 payload to GDrive via Google App Script. If payload size is more than 16384 bytes then HTTPClient::sendRequest will fail.
Related links
None