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.

Sign up for GitHub

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

Avoid large reallocations in webserver's header response #8873

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
+87 −45
Draft

Avoid large reallocations in webserver's header response #8873

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
755021d
avoid large reallocations in webserver header response
d-a-v Feb 28, 2023
16bb9de
make sendHeader generic
d-a-v Feb 28, 2023
5bc850e
automatic factorization of declarations with std::forward
d-a-v Mar 5, 2023
b43a2d5
Merge branch 'master' into webserverwithoutholes
d-a-v Jul 17, 2023
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
107 changes: 67 additions & 40 deletions libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ void ESP8266WebServerTemplate<ServerType>::serveStatic(const char* uri, FS& fs,
if(is_file) {
_addRequestHandler(new StaticFileRequestHandler<ServerType>(fs, path, uri, cache_header));
} else {
_addRequestHandler(new StaticDirectoryRequestHandler<ServerType>(fs, path, uri, cache_header));
_addRequestHandler(new StaticDirectoryRequestHandler<ServerType>(fs, path, uri, cache_header));
}
}

Expand Down Expand Up @@ -420,18 +420,12 @@ void ESP8266WebServerTemplate<ServerType>::stop() {
}

template <typename ServerType>
void ESP8266WebServerTemplate<ServerType>::sendHeader(const String& name, const String& value, bool first) {
String headerLine = name;
headerLine += F(": ");
headerLine += value;
headerLine += "\r\n";

if (first) {
_responseHeaders = headerLine + _responseHeaders;
}
else {
_responseHeaders += headerLine;
}
template <typename S1, typename S2>
void ESP8266WebServerTemplate<ServerType>::sendHeader(S1 name, S2 value, bool first) {
Copy link
Collaborator

@mcspr mcspr Feb 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

minor nit

at a glance, we have a bunch of typed pairs that generate multiple emplace_... for every sendHeader variant
I'd consider using perfect forward and a specific type we already have

using Pair = std::pair<String, String>;

auto foo(Pair pair, bool condition) {
    if (condition) {
        foo(std::move(pair));
    } else {
        bar(std::move(pair));
    }
}

template <typename S1, typename S2>
auto foo(S1&& name, S2&& value, bool condition) {
    foo(std::make_pair(
        String(std::forward<S1>(name)),
        String(std::forward<S2>(value)), condition);
}

if (first)
_userHeaders.emplace_front(std::pair(name, value));
else
_userHeaders.emplace_back(std::pair(name, value));
}

template <typename ServerType>
Expand All @@ -440,44 +434,80 @@ void ESP8266WebServerTemplate<ServerType>::setContentLength(const size_t content
}

template <typename ServerType>
void ESP8266WebServerTemplate<ServerType>::_prepareHeader(String& response, int code, const char* content_type, size_t contentLength) {
response = String(F("HTTP/1.")) + String(_currentVersion) + ' ';
response += String(code);
response += ' ';
response += responseCodeToString(code);
response += "\r\n";
bool ESP8266WebServerTemplate<ServerType>::_streamIt (Stream& s) {
int len = s.streamRemaining();
int sent = s.sendAll(&_currentClient);
if (sent == len)
return true;
DBGWS("HTTPServer: error: sent %zd on %u bytes\n", sent, len);
return false;
}

template <typename ServerType>
template <typename K, typename V>
bool ESP8266WebServerTemplate<ServerType>::_streamHeader (K name, V value)
{
return _streamIt(name) && _streamItC(F(": ")) && _streamIt(value) && _streamItC(F("\r\n"));
}

template <typename ServerType>
bool ESP8266WebServerTemplate<ServerType>::_sendHeader(int code, const char* content_type, size_t contentLength) {
if ( !_streamItC(F("HTTP/1."))
|| !_streamIt(String(_currentVersion))
|| !_streamItC(F(" "))
|| !_streamIt(String(code))
|| !_streamItC(F(" "))
|| !_streamIt(responseCodeToString(code))
|| !_streamItC(F("\r\n")))
{
return false;
}

using namespace mime;
if (!content_type)
content_type = mimeTable[html].mimeType;
if (!_streamHeader(F("Content-Type"), content_type))
return false;

sendHeader(String(F("Content-Type")), String(FPSTR(content_type)), true);
if (_contentLength == CONTENT_LENGTH_NOT_SET) {
sendHeader(String(FPSTR(Content_Length)), String(contentLength));
if (!_streamHeader(Content_Length, String(contentLength)))
return false;
} else if (_contentLength != CONTENT_LENGTH_UNKNOWN) {
sendHeader(String(FPSTR(Content_Length)), String(_contentLength));
} else if(_contentLength == CONTENT_LENGTH_UNKNOWN && _currentVersion){ //HTTP/1.1 or above client
//let's do chunked
_chunked = true;
sendHeader(String(F("Accept-Ranges")),String(F("none")));
sendHeader(String(F("Transfer-Encoding")),String(F("chunked")));
if (!_streamHeader(Content_Length, String(_contentLength)))
return false;
} else if (_contentLength == CONTENT_LENGTH_UNKNOWN && _currentVersion){ //HTTP/1.1 or above client
//let's do chunked
_chunked = true;
if (!_streamHeader(F("Accept-Ranges"), F("none")) || !_streamHeader(F("Transfer-Encoding"), F("chunked")))
return false;
}

if (_corsEnabled) {
sendHeader(String(F("Access-Control-Allow-Origin")), String("*"));
if (!_streamHeader(F("Access-Control-Allow-Origin"), F("*")))
return false;
}

if (_keepAlive && _server.hasClient()) { // Disable keep alive if another client is waiting.
_keepAlive = false;
if (_keepAlive && _server.hasClient()) {
// Disable keep alive if another client is waiting.
_keepAlive = false;
}
if (!_streamHeader(F("Connection"), String(_keepAlive ? F("keep-alive") : F("close")))) {
return false;
}
sendHeader(String(F("Connection")), String(_keepAlive ? F("keep-alive") : F("close")));
if (_keepAlive) {
sendHeader(String(F("Keep-Alive")), String(F("timeout=")) + HTTP_MAX_CLOSE_WAIT);
if (!_streamHeader(F("Keep-Alive"), String(F("timeout=")) + HTTP_MAX_CLOSE_WAIT))
return false;
}

for (const auto& kv: _userHeaders)
if (!_streamHeader(kv.first, kv.second))
return false;
_userHeaders.clear();
Copy link
Contributor

Choose a reason for hiding this comment

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

When will these be cleared if streaming failed?

N.B. I got the impression the current implementation of the webserver does hold some data way longer than needed as opening a web page may sometimes free up quite a bit of memory. (maybe POST data of a previous request???)


response += _responseHeaders;
response += "\r\n";
_responseHeaders = "";
if (!_streamItC(F("\r\n")))
return false;

return true;
}

template <typename ServerType>
Expand All @@ -503,13 +533,10 @@ void ESP8266WebServerTemplate<ServerType>::sendContent(const String& content) {

template <typename ServerType>
void ESP8266WebServerTemplate<ServerType>::send(int code, const char* content_type, Stream* stream, size_t content_length /*= 0*/) {
String header;
if (content_length == 0)
content_length = std::max((ssize_t)0, stream->streamRemaining());
_prepareHeader(header, code, content_type, content_length);
size_t sent = StreamConstPtr(header).sendAll(&_currentClient);
if (sent != header.length())
DBGWS("HTTPServer: error: sent %zd on %u bytes\n", sent, header.length());
if (!_sendHeader(code, content_type, content_length))
return;
if (content_length)
return sendContent(stream, content_length);
}
Expand Down
16 changes: 13 additions & 3 deletions libraries/ESP8266WebServer/src/ESP8266WebServer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,11 @@
#include <functional>
#include <memory>
#include <functional>
#include <list>

#include <ESP8266WiFi.h>
#include <FS.h>
#include <StreamDev.h>
#include "detail/mimetable.h"
#include "Uri.h"

Expand Down Expand Up @@ -176,7 +179,8 @@ class ESP8266WebServerTemplate
}

void setContentLength(const size_t contentLength);
void sendHeader(const String& name, const String& value, bool first = false);
template <typename S1, typename S2>
void sendHeader(S1 name, S2 value, bool first = false);
void sendContent(const String& content);
void sendContent(String& content) {
sendContent((const String&)content);
Expand Down Expand Up @@ -291,7 +295,7 @@ class ESP8266WebServerTemplate
bool _parseFormUploadAborted();
void _uploadWriteByte(uint8_t b);
int _uploadReadByte(ClientType& client);
void _prepareHeader(String& response, int code, const char* content_type, size_t contentLength);
bool _sendHeader(int code, const char* content_type, size_t contentLength);
bool _collectHeader(const char* headerName, const char* headerValue);

void _streamFileCore(const size_t fileSize, const String & fileName, const String & contentType);
Expand All @@ -300,6 +304,12 @@ class ESP8266WebServerTemplate
// for extracting Auth parameters
String _extractParam(String& authReq,const String& param,const char delimit = '"') const;

bool _streamIt (const String& s) { StreamConstPtr c(s.c_str(), s.length()); return _streamIt(c); }
bool _streamItC (StreamConstPtr&& s) { return _streamIt(s); }
bool _streamIt (Stream& s);
template <typename K, typename V>
bool _streamHeader (K name, V value);

struct RequestArgument {
String key;
String value;
Expand Down Expand Up @@ -330,7 +340,7 @@ class ESP8266WebServerTemplate
RequestArgument* _currentHeaders = nullptr;

size_t _contentLength = 0;
String _responseHeaders;
std::list<std::pair<String,String>> _userHeaders;

String _hostHeader;
bool _chunked = false;
Expand Down