Skip to content

Add header access using same method as arguments #886

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

Closed
wants to merge 7 commits into from
Closed

Add header access using same method as arguments #886

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ec89479
Create RequestArgument array of header pairs
bren3582 Oct 13, 2015
05921b7
Add arg-like access functions and variables
bren3582 Oct 13, 2015
9b3b922
Define header access functions
bren3582 Oct 13, 2015
d9e3a35
Added collectHeaders method and supporting vars
bren3582 Oct 14, 2015
c117d78
Defined method and freed used memory in destructor
bren3582 Oct 15, 2015
d44fc5b
Defined _collectHeader and updated parser
bren3582 Oct 15, 2015
fab08c5
Safer destructor with _headerKeysCount = 0
bren3582 Oct 15, 2015
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
44 changes: 44 additions & 0 deletions hardware/esp8266com/esp8266/libraries/ESP8266WebServer/src/ESP8266WebServer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ ESP8266WebServer::ESP8266WebServer(int port)
}

ESP8266WebServer::~ESP8266WebServer() {
if (_headerKeys) {
free(_headerKeys);
_headerKeys = 0;
Copy link

Choose a reason for hiding this comment

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

Do we also maybe want to do _headerKeysCount = 0 just to be safe?

Copy link
Author

Choose a reason for hiding this comment

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

I doubt it because the object is being destroyed anyway but why not, can't be too safe. Does the rest of the PR work for you?

}
if (!_firstHandler)
return;
RequestHandler* handler = _firstHandler;
Expand Down Expand Up @@ -283,6 +287,46 @@ bool ESP8266WebServer::hasArg(const char* name) {
return false;
}

String ESP8266WebServer::header(const char* name) {
for (int i = 0; i < _currentHeaderCount; ++i) {
if (_currentHeaders[i].key == name)
return _currentHeaders[i].value;
}
return String();
}

void ESP8266WebServer::collectHeaders(const char* headerKeys[], const size_t headerKeysCount) {
_headerKeysCount = headerKeysCount;
_headerKeys = (const char**)malloc(_headerKeysCount*sizeof(char*));
for (int i = 0; i < _headerKeysCount; i++){
_headerKeys[i] = headerKeys[i];
}
}

String ESP8266WebServer::header(int i) {
if (i < _currentHeaderCount)
return _currentHeaders[i].value;
return String();
}

String ESP8266WebServer::headerName(int i) {
if (i < _currentHeaderCount)
return _currentHeaders[i].key;
return String();
}

int ESP8266WebServer::headers() {
return _currentHeaderCount;
}

bool ESP8266WebServer::hasHeader(const char* name) {
for (int i = 0; i < _currentHeaderCount; ++i) {
if (_currentHeaders[i].key == name)
return true;
}
return false;
}

String ESP8266WebServer::hostHeader() {
return _hostHeader;
}
Expand Down
13 changes: 13 additions & 0 deletions hardware/esp8266com/esp8266/libraries/ESP8266WebServer/src/ESP8266WebServer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ class ESP8266WebServer
String argName(int i); // get request argument name by number
int args(); // get arguments count
bool hasArg(const char* name); // check if argument exists

void collectHeaders(const char* headerKeys[], const size_t headerKeysCount); // set the request headers to collect
String header(const char* name); // get request header value by name
Copy link

Choose a reason for hiding this comment

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

Will this end up copying the returned String (or will the compiler optimize)? Is there a "best practice" (examples in other well-used libraries) on how to return a String efficiently?

(Am I focusing too much on optimizations 😄 😞 ?)

Copy link

Choose a reason for hiding this comment

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

For example, would const String& be a better return type? (Will be tricky to handle the "not found" case, but we could make a single empty string to use for all "not found" return values.)

Copy link
Author

Choose a reason for hiding this comment

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

If you look just above (ESP8266WebServer.h Line 81), you'll see that the existing argument access method is the same. There may be a better way but I decided to follow the conventions of the existing code.

Copy link

Choose a reason for hiding this comment

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

OK, cool, good enough for me.

String header(int i); // get request header value by number
String headerName(int i); // get request header name by number
int headers(); // get header count
bool hasHeader(const char* name); // check if header exists

String hostHeader(); // get request host header if available or empty String if not

Expand Down Expand Up @@ -124,6 +131,7 @@ template<typename T> size_t streamFile(T &file, const String& contentType){
void _uploadWriteByte(uint8_t b);
uint8_t _uploadReadByte(WiFiClient& client);
void _prepareHeader(String& response, int code, const char* content_type, size_t contentLength);
bool _collectHeader(const char* headerName);

struct RequestArgument {
String key;
Expand All @@ -139,6 +147,11 @@ template<typename T> size_t streamFile(T &file, const String& contentType){
size_t _currentArgCount;
RequestArgument* _currentArgs;
HTTPUpload _currentUpload;

size_t _currentHeaderCount;
RequestArgument* _currentHeaders;
size_t _headerKeysCount;
const char** _headerKeys;

size_t _contentLength;
String _responseHeaders;
Expand Down
27 changes: 26 additions & 1 deletion hardware/esp8266com/esp8266/libraries/ESP8266WebServer/src/Parsing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,12 @@ bool ESP8266WebServer::_parseRequest(WiFiClient& client) {
// Read the first line of HTTP request
String req = client.readStringUntil('\r');
client.readStringUntil('\n');


if (_currentHeaders)
delete[] _currentHeaders;
_currentHeaders = new RequestArgument[_headerKeysCount];
_currentHeaderCount = 0;

// First line of HTTP request looks like "GET /path HTTP/1.1"
// Retrieve the "/path" part by finding the spaces
int addr_start = req.indexOf(' ');
Expand Down Expand Up @@ -96,6 +101,13 @@ bool ESP8266WebServer::_parseRequest(WiFiClient& client) {
}
headerName = req.substring(0, headerDiv);
headerValue = req.substring(headerDiv + 2);

if (_collectHeader(headerName.c_str())) {
RequestArgument& arg = _currentHeaders[_currentHeaderCount];
arg.key = headerName;
arg.value = headerValue;
_currentHeaderCount++;
}

#ifdef DEBUG
DEBUG_OUTPUT.print("headerName: ");
Expand Down Expand Up @@ -161,6 +173,13 @@ bool ESP8266WebServer::_parseRequest(WiFiClient& client) {
}
headerName = req.substring(0, headerDiv);
headerValue = req.substring(headerDiv + 2);

if (_collectHeader(headerName.c_str())) {
RequestArgument& arg = _currentHeaders[_currentHeaderCount];
arg.key = headerName;
arg.value = headerValue;
_currentHeaderCount++;
}

#ifdef DEBUG
DEBUG_OUTPUT.print("headerName: ");
Expand All @@ -187,6 +206,12 @@ bool ESP8266WebServer::_parseRequest(WiFiClient& client) {
return true;
}

bool ESP8266WebServer::_collectHeader(const char* headerName) {
for (size_t i = 0; i < _headerKeysCount; i++) {
if (strcmp(headerName, _headerKeys[i]) == 0) return true;
}
return false;
}

void ESP8266WebServer::_parseArguments(String data) {
#ifdef DEBUG
Expand Down