From 42555946516ffeb7ce5d2e0d536b77b08850ea4a Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Fri, 26 Jul 2024 22:07:04 +0300 Subject: [PATCH] ESP8266WebServer - StreamString parsing experiment impl based on #9005, but for existing Arduino methods --- cores/esp8266/Stream.cpp | 54 ++++---- libraries/ESP8266WebServer/src/Parsing-impl.h | 26 ++-- tests/host/Makefile | 26 ++++ tests/host/common/mock.h | 15 ++- tests/host/common/noniso.c | 22 +--- tests/host/common/strl.cpp | 116 +++++++++--------- tests/host/core/test_string.cpp | 25 ++++ 7 files changed, 161 insertions(+), 123 deletions(-) diff --git a/cores/esp8266/Stream.cpp b/cores/esp8266/Stream.cpp index b9b5b95f65..6f34680043 100644 --- a/cores/esp8266/Stream.cpp +++ b/cores/esp8266/Stream.cpp @@ -20,8 +20,9 @@ parsing functions based on TextFinder library by Michael Margolis */ -#include -#include +#include "Arduino.h" +#include "Stream.h" +#include "StreamString.h" #define PARSE_TIMEOUT 1000 // default number of milli-seconds to wait #define NO_SKIP_CHAR 1 // a magic char not found in a valid ASCII numeric field @@ -254,34 +255,45 @@ String Stream::readString() { String Stream::readStringUntil(char terminator) { String ret; - int c = timedRead(); - while(c >= 0 && c != terminator) { - ret += (char) c; - c = timedRead(); - } + + S2Stream s2s(ret); + sendUntil(s2s, terminator, _timeout); + return ret; } String Stream::readStringUntil(const char* terminator, uint32_t untilTotalNumberOfOccurrences) { String ret; - int c; + if (!untilTotalNumberOfOccurrences) { + return ret; + } + + const size_t termLen = strlen_P(terminator); + if (!termLen) { + return ret; + } + + S2Stream s2s(ret); uint32_t occurrences = 0; - size_t termLen = strlen(terminator); - size_t termIndex = 0; - size_t index = 0; + const size_t tailLen = termLen - 1; - while ((c = timedRead()) > 0) { - ret += (char) c; - index++; + for (;;) { + sendUntil(s2s, terminator[tailLen], _timeout); + if (s2s.getLastSendReport() != Stream::Report::Success) { + break; + } + + if ((ret.length() >= tailLen) + && ((0 == tailLen) || (0 == memcmp_P(terminator, ret.end() - tailLen, tailLen)))) + { + ++occurrences; + } - if (terminator[termIndex] == c) { - if (++termIndex == termLen && ++occurrences == untilTotalNumberOfOccurrences) { - // don't include terminator in returned string - ret.remove(index - termIndex, termLen); - break; + if (untilTotalNumberOfOccurrences == occurrences) { + if (tailLen) { + ret.remove(ret.length() - tailLen); } - } else { - termIndex = 0; + break; } } diff --git a/libraries/ESP8266WebServer/src/Parsing-impl.h b/libraries/ESP8266WebServer/src/Parsing-impl.h index 672682706e..1d62d7d5e2 100644 --- a/libraries/ESP8266WebServer/src/Parsing-impl.h +++ b/libraries/ESP8266WebServer/src/Parsing-impl.h @@ -44,9 +44,8 @@ static bool readBytesWithTimeout(typename ServerType::ClientType& client, size_t template typename ESP8266WebServerTemplate::ClientFuture ESP8266WebServerTemplate::_parseRequest(ClientType& client) { // Read the first line of HTTP request - String req = client.readStringUntil('\r'); + String req = client.readStringUntil("\r\n"); DBGWS("request: %s\n", req.c_str()); - client.readStringUntil('\n'); //reset header value for (int i = 0; i < _headerKeysCount; ++i) { _currentHeaders[i].value.clear(); @@ -122,8 +121,7 @@ typename ESP8266WebServerTemplate::ClientFuture ESP8266WebServerTemp uint32_t contentLength = 0; //parse headers while(1){ - req = client.readStringUntil('\r'); - client.readStringUntil('\n'); + req = client.readStringUntil("\r\n"); if (req.isEmpty()) break; //no more headers int headerDiv = req.indexOf(':'); if (headerDiv == -1){ @@ -198,8 +196,7 @@ typename ESP8266WebServerTemplate::ClientFuture ESP8266WebServerTemp String headerValue; //parse headers while(1){ - req = client.readStringUntil('\r'); - client.readStringUntil('\n'); + req = client.readStringUntil("\r\n"); if (req.isEmpty()) break;//no moar headers int headerDiv = req.indexOf(':'); if (headerDiv == -1){ @@ -351,7 +348,7 @@ bool ESP8266WebServerTemplate::_parseForm(ClientType& client, const String line; int retry = 0; do { - line = client.readStringUntil('\r'); + line = client.readStringUntil("\r\n"); ++retry; } while (line.length() == 0 && retry < 3); @@ -367,8 +364,7 @@ bool ESP8266WebServerTemplate::_parseForm(ClientType& client, const String argFilename; bool argIsFile = false; - line = client.readStringUntil('\r'); - client.readStringUntil('\n'); + line = client.readStringUntil("\r\n"); if (line.length() > 19 && line.substring(0, 19).equalsIgnoreCase(F("Content-Disposition"))){ int nameStart = line.indexOf('='); if (nameStart != -1){ @@ -388,19 +384,16 @@ bool ESP8266WebServerTemplate::_parseForm(ClientType& client, const DBGWS("PostArg Name: %s\n", argName.c_str()); using namespace mime; argType = FPSTR(mimeTable[txt].mimeType); - line = client.readStringUntil('\r'); - client.readStringUntil('\n'); + line = client.readStringUntil("\r\n"); if (line.length() > 12 && line.substring(0, 12).equalsIgnoreCase(FPSTR(Content_Type))){ argType = line.substring(line.indexOf(':')+2); //skip next line - client.readStringUntil('\r'); - client.readStringUntil('\n'); + client.readStringUntil("\r\n"); } DBGWS("PostArg Type: %s\n", argType.c_str()); if (!argIsFile){ while(1){ - line = client.readStringUntil('\r'); - client.readStringUntil('\n'); + line = client.readStringUntil("\r\n"); if (line.startsWith("--"+boundary)) break; if (argValue.length() > 0) argValue += '\n'; argValue += line; @@ -474,8 +467,7 @@ bool ESP8266WebServerTemplate::_parseForm(ClientType& client, const _currentUpload->type.c_str(), (int)_currentUpload->totalSize); if (!client.connected()) return _parseFormUploadAborted(); - line = client.readStringUntil('\r'); - client.readStringUntil('\n'); + line = client.readStringUntil("\r\n"); if (line == "--") { // extra two dashes mean we reached the end of all form fields DBGWS("Done Parsing POST\n"); break; diff --git a/tests/host/Makefile b/tests/host/Makefile index f819047891..aac9f14bce 100644 --- a/tests/host/Makefile +++ b/tests/host/Makefile @@ -30,6 +30,7 @@ GENHTML ?= genhtml CXXFLAGS += -std=gnu++17 CFLAGS += -std=gnu17 +# 32-bit mode is prefered, but not required ifeq ($(FORCE32),1) SIZEOFLONG = $(shell echo 'int main(){return sizeof(long);}'|$(CXX) -m32 -x c++ - -o sizeoflong 2>/dev/null && ./sizeoflong; echo $$?; rm -f sizeoflong;) ifneq ($(SIZEOFLONG),4) @@ -50,6 +51,7 @@ endif OUTPUT_BINARY := $(BINDIR)/host_tests LCOV_DIRECTORY := $(BINDIR)/../lcov +# Hide full build commands by default ifeq ($(V), 0) VERBC = @echo "C $@"; VERBCXX = @echo "C++ $@"; @@ -66,6 +68,30 @@ endif $(shell mkdir -p $(BINDIR)) +# Core files sometimes override libc functions, check when necessary to hide them +# TODO proper configure script / other build system? +ifeq (,$(wildcard $(BINDIR)/.have_strlcpy)) +$(shell echo -e '#include \nint main(){char a[4]{}; char b[4]{}; strlcpy(&a[0], &b[0], sizeof(a)); return 0;}' | \ + $(CXX) -x c++ - -o $(BINDIR)/.have_strlcpy 2>/dev/null || ( echo -e '#!/bin/sh\nexit 1' > $(BINDIR)/.have_strlcpy ; chmod +x $(BINDIR)/.have_strlcpy; )) +endif + +$(shell $(BINDIR)/.have_strlcpy) +ifeq ($(.SHELLSTATUS), 0) +FLAGS += -DHAVE_STRLCPY +endif + +ifeq (,$(wildcard $(BINDIR)/.have_strlcat)) +$(shell echo -e '#include \nint main(){char a[4]{}; strlcat(&a[0], "test", sizeof(a)); return 0;}' | \ + $(CXX) -x c++ - -o $(BINDIR)/.have_strlcat 2>/dev/null || ( echo -e '#!/bin/sh\nexit 1' > $(BINDIR)/.have_strlcat ; chmod +x $(BINDIR)/.have_strlcat; )) +endif + +$(shell $(BINDIR)/.have_strlcat) +ifeq ($(.SHELLSTATUS), 0) +FLAGS += -DHAVE_STRLCAT +endif + +# Actual build recipes + CORE_CPP_FILES := \ $(addprefix $(abspath $(CORE_PATH))/,\ debug.cpp \ diff --git a/tests/host/common/mock.h b/tests/host/common/mock.h index b3308282f0..ece973edfc 100644 --- a/tests/host/common/mock.h +++ b/tests/host/common/mock.h @@ -56,18 +56,23 @@ #define D8 8 #include +#include +#include + +#include #ifdef __cplusplus extern "C" { #endif - // TODO: #include ? - char* itoa(int val, char* s, int radix); - char* ltoa(long val, char* s, int radix); - + char* utoa(unsigned value, char* result, int base); + char* itoa(int value, char* result, int base); +#ifndef HAVE_STRLCAT size_t strlcat(char* dst, const char* src, size_t size); +#endif +#ifndef HAVE_STRLCPY size_t strlcpy(char* dst, const char* src, size_t size); - +#endif #ifdef __cplusplus } #endif diff --git a/tests/host/common/noniso.c b/tests/host/common/noniso.c index 5c4e14b306..20fd3d1d5a 100644 --- a/tests/host/common/noniso.c +++ b/tests/host/common/noniso.c @@ -18,9 +18,10 @@ #include #include #include -#include "stdlib_noniso.h" -void reverse(char* begin, char* end) +#include + +static void reverse(char* begin, char* end) { char* is = begin; char* ie = end - 1; @@ -84,20 +85,3 @@ char* itoa(int value, char* result, int base) utoa(uvalue, result, base); return out; } - -int atoi(const char* s) -{ - return (int)atol(s); -} - -long atol(const char* s) -{ - char* tmp; - return strtol(s, &tmp, 10); -} - -double atof(const char* s) -{ - char* tmp; - return strtod(s, &tmp); -} diff --git a/tests/host/common/strl.cpp b/tests/host/common/strl.cpp index 0b0725b046..78a6e0efa6 100644 --- a/tests/host/common/strl.cpp +++ b/tests/host/common/strl.cpp @@ -1,84 +1,78 @@ // https://gist.github.com/Fonger/98cc95ac39fbe1a7e4d9 -#ifndef HAVE_STRLCAT -/* - '_cups_strlcat()' - Safely concatenate two strings. -*/ - -size_t /* O - Length of string */ -strlcat(char* dst, /* O - Destination string */ - const char* src, /* I - Source string */ - size_t size) /* I - Size of destination string buffer */ +#include +#include +#include + +extern "C" { - size_t srclen; /* Length of source string */ - size_t dstlen; /* Length of destination string */ +#ifndef HAVE_STRLCAT + // '_cups_strlcat()' - Safely concatenate two strings. - /* - Figure out how much room is left... - */ + size_t /* O - Length of string */ + strlcat(char* dst, /* O - Destination string */ + const char* src, /* I - Source string */ + size_t size) /* I - Size of destination string buffer */ + { + size_t srclen; /* Length of source string */ + size_t dstlen; /* Length of destination string */ - dstlen = strlen(dst); - size -= dstlen + 1; + // Figure out how much room is left... - if (!size) - { - return (dstlen); /* No room, return immediately... */ - } + dstlen = strlen(dst); + size -= dstlen + 1; - /* - Figure out how much room is needed... - */ + if (!size) + { + return (dstlen); /* No room, return immediately... */ + } - srclen = strlen(src); + // Figure out how much room is needed... - /* - Copy the appropriate amount... - */ + srclen = strlen(src); - if (srclen > size) - { - srclen = size; - } + // Copy the appropriate amount... + + if (srclen > size) + { + srclen = size; + } - memcpy(dst + dstlen, src, srclen); - dst[dstlen + srclen] = '\0'; + memcpy(dst + dstlen, src, srclen); + dst[dstlen + srclen] = '\0'; - return (dstlen + srclen); -} + return (dstlen + srclen); + } #endif /* !HAVE_STRLCAT */ #ifndef HAVE_STRLCPY -/* - '_cups_strlcpy()' - Safely copy two strings. -*/ - -size_t /* O - Length of string */ -strlcpy(char* dst, /* O - Destination string */ - const char* src, /* I - Source string */ - size_t size) /* I - Size of destination string buffer */ -{ - size_t srclen; /* Length of source string */ + // '_cups_strlcpy()' - Safely copy two strings. - /* - Figure out how much room is needed... - */ + size_t /* O - Length of string */ + strlcpy(char* dst, /* O - Destination string */ + const char* src, /* I - Source string */ + size_t size) /* I - Size of destination string buffer */ + { + size_t srclen; /* Length of source string */ - size--; + // Figure out how much room is needed... - srclen = strlen(src); + size--; - /* - Copy the appropriate amount... - */ + srclen = strlen(src); - if (srclen > size) - { - srclen = size; - } + // Copy the appropriate amount... + + if (srclen > size) + { + srclen = size; + } - memcpy(dst, src, srclen); - dst[srclen] = '\0'; + memcpy(dst, src, srclen); + dst[srclen] = '\0'; - return (srclen); -} + return (srclen); + } #endif /* !HAVE_STRLCPY */ + +} // extern "C" diff --git a/tests/host/core/test_string.cpp b/tests/host/core/test_string.cpp index 92b37e4279..dd20374b16 100644 --- a/tests/host/core/test_string.cpp +++ b/tests/host/core/test_string.cpp @@ -512,6 +512,31 @@ TEST_CASE("Issue #2736 - StreamString SSO fix", "[core][StreamString]") REQUIRE(s == "{\"message\""); } +TEST_CASE("Issue #9005 - StreamString for Stream->String conversion", "[core][StreamString]") +{ + const char buffer[] = + "this is a test string" + "\r\n" + "delimited as if it was a http request" + "\r\n" + "\r\n"; + + StreamString input; + input.print(buffer); + REQUIRE(input == buffer); + + String out = input.readStringUntil("\r\n"); + REQUIRE(21 == out.length()); + REQUIRE(out == "this is a test string"); + + out = input.readStringUntil("\r\n"); + REQUIRE(37 == out.length()); + REQUIRE(out == "delimited as if it was a http request"); + + out = input.readStringUntil("\r\n"); + REQUIRE(0 == out.length()); +} + TEST_CASE("Strings with NULs", "[core][String]") { // The following should never be done in a real app! This is only to inject 0s in the middle of