mirror of
https://github.com/rstrouse/ESPSomfy-RTS.git
synced 2026-10-31 13:52:15 +01:00
fix: never write past a JSON buffer or send a broken socket event
When something didn't fit, the formatters skipped it silently, closing brackets included, and clients got malformed JSON. Two writes could also go past the buffer: - AsyncJsonResp flushed when full and then wrote the value without checking again, so a value bigger than the whole buffer overflowed. It is now streamed straight out instead. - JsonSockEvent::closeEvent added the closing ] when only one byte was left, putting the terminator one byte past the buffer. The formatters now set an overflowed() flag and log an error, and a socket event that overflowed is logged and not sent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
b2294ced34
commit
8881289b1d
4 changed files with 61 additions and 6 deletions
|
|
@ -1,10 +1,14 @@
|
||||||
#include "JsonFormatter.h"
|
#include "JsonFormatter.h"
|
||||||
|
#include "esp_log.h"
|
||||||
|
|
||||||
|
static const char *TAG = "JSON";
|
||||||
|
|
||||||
void BufferedJsonFormatter::setBuffer(char *b, size_t sz) {
|
void BufferedJsonFormatter::setBuffer(char *b, size_t sz) {
|
||||||
this->buff = b;
|
this->buff = b;
|
||||||
this->buffSize = sz;
|
this->buffSize = sz;
|
||||||
this->buff[0] = 0;
|
this->buff[0] = 0;
|
||||||
this->_nocomma = true;
|
this->_nocomma = true;
|
||||||
|
this->_overflow = false;
|
||||||
this->_objects = 0;
|
this->_objects = 0;
|
||||||
this->_arrays = 0;
|
this->_arrays = 0;
|
||||||
}
|
}
|
||||||
|
|
@ -86,6 +90,8 @@ void JsonFormatter::_safecat(const char *val, bool escape) {
|
||||||
size_t len = (escape ? this->calcEscapedLength(val) : strlen(val)) + strlen(this->buff);
|
size_t len = (escape ? this->calcEscapedLength(val) : strlen(val)) + strlen(this->buff);
|
||||||
if(escape) len += 2;
|
if(escape) len += 2;
|
||||||
if(len >= this->buffSize) {
|
if(len >= this->buffSize) {
|
||||||
|
if(!this->_overflow) ESP_LOGE(TAG, "JSON exceeded buffer size %u", (unsigned)this->buffSize);
|
||||||
|
this->_overflow = true;
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if(escape) strcat(this->buff, "\"");
|
if(escape) strcat(this->buff, "\"");
|
||||||
|
|
|
||||||
|
|
@ -12,10 +12,13 @@ class JsonFormatter {
|
||||||
uint8_t _objects = 0;
|
uint8_t _objects = 0;
|
||||||
uint8_t _arrays = 0;
|
uint8_t _arrays = 0;
|
||||||
bool _nocomma = true;
|
bool _nocomma = true;
|
||||||
|
// Something didn't fit and was left out so the JSON is incomplete.
|
||||||
|
bool _overflow = false;
|
||||||
char _numbuff[25] = {0};
|
char _numbuff[25] = {0};
|
||||||
virtual void _safecat(const char *val, bool escape = false);
|
virtual void _safecat(const char *val, bool escape = false);
|
||||||
void _appendNumber(const char *name);
|
void _appendNumber(const char *name);
|
||||||
public:
|
public:
|
||||||
|
bool overflowed() const { return _overflow; }
|
||||||
// Writes c as it must appear inside a JSON string (out needs 7 bytes) and
|
// Writes c as it must appear inside a JSON string (out needs 7 bytes) and
|
||||||
// returns its length.
|
// returns its length.
|
||||||
static size_t escapeChar(char c, char *out);
|
static size_t escapeChar(char c, char *out);
|
||||||
|
|
|
||||||
|
|
@ -8,18 +8,25 @@ void JsonSockEvent::beginEvent(AsyncWebSocket *server, const char *evt, char *bu
|
||||||
this->buffSize = buffSize;
|
this->buffSize = buffSize;
|
||||||
this->_nocomma = true;
|
this->_nocomma = true;
|
||||||
this->_closed = false;
|
this->_closed = false;
|
||||||
|
this->_overflow = false;
|
||||||
snprintf(this->buff, buffSize, "42[%s,", evt);
|
snprintf(this->buff, buffSize, "42[%s,", evt);
|
||||||
}
|
}
|
||||||
void JsonSockEvent::closeEvent() {
|
void JsonSockEvent::closeEvent() {
|
||||||
if(!this->_closed) {
|
if(!this->_closed) {
|
||||||
if(strlen(this->buff) < buffSize) strcat(this->buff, "]");
|
// The ] and the terminator both have to fit.
|
||||||
else this->buff[buffSize - 1] = ']';
|
if(strlen(this->buff) + 1 < buffSize) strcat(this->buff, "]");
|
||||||
|
else this->_overflow = true;
|
||||||
}
|
}
|
||||||
this->_nocomma = true;
|
this->_nocomma = true;
|
||||||
this->_closed = true;
|
this->_closed = true;
|
||||||
}
|
}
|
||||||
void JsonSockEvent::endEvent(uint32_t clientId) {
|
void JsonSockEvent::endEvent(uint32_t clientId) {
|
||||||
this->closeEvent();
|
this->closeEvent();
|
||||||
|
// Part of it is missing so clients would only fail to parse it.
|
||||||
|
if(this->_overflow) {
|
||||||
|
ESP_LOGE(TAG, "Socket event too large for %u bytes, not sent: %.32s", (unsigned)this->buffSize, this->buff);
|
||||||
|
return;
|
||||||
|
}
|
||||||
if(clientId == 0) this->server->textAll(this->buff);
|
if(clientId == 0) this->server->textAll(this->buff);
|
||||||
else this->server->text(clientId, this->buff);
|
else this->server->text(clientId, this->buff);
|
||||||
}
|
}
|
||||||
|
|
@ -27,7 +34,7 @@ void JsonSockEvent::_safecat(const char *val, bool escape) {
|
||||||
size_t len = (escape ? this->calcEscapedLength(val) : strlen(val)) + strlen(this->buff);
|
size_t len = (escape ? this->calcEscapedLength(val) : strlen(val)) + strlen(this->buff);
|
||||||
if(escape) len += 2;
|
if(escape) len += 2;
|
||||||
if(len >= this->buffSize) {
|
if(len >= this->buffSize) {
|
||||||
ESP_LOGE(TAG, "Socket exceeded buffer size %d - %d: %s", this->buffSize, len, this->buff);
|
this->_overflow = true;
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if(escape) strcat(this->buff, "\"");
|
if(escape) strcat(this->buff, "\"");
|
||||||
|
|
@ -42,6 +49,7 @@ void AsyncJsonResp::beginResponse(AsyncWebServerRequest *request, char *buff, si
|
||||||
this->buff[0] = 0x00;
|
this->buff[0] = 0x00;
|
||||||
this->_nocomma = true;
|
this->_nocomma = true;
|
||||||
this->_headersSent = false;
|
this->_headersSent = false;
|
||||||
|
this->_overflow = false;
|
||||||
this->_stream = request->beginResponseStream("application/json");
|
this->_stream = request->beginResponseStream("application/json");
|
||||||
}
|
}
|
||||||
void AsyncJsonResp::endResponse() {
|
void AsyncJsonResp::endResponse() {
|
||||||
|
|
@ -57,10 +65,28 @@ void AsyncJsonResp::flush() {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
void AsyncJsonResp::_safecat(const char *val, bool escape) {
|
void AsyncJsonResp::_safecat(const char *val, bool escape) {
|
||||||
size_t len = (escape ? this->calcEscapedLength(val) : strlen(val)) + strlen(this->buff);
|
const size_t vlen = escape ? this->calcEscapedLength(val) + 2 : strlen(val);
|
||||||
if(escape) len += 2;
|
if(vlen + strlen(this->buff) >= this->buffSize) {
|
||||||
if(len >= this->buffSize) {
|
|
||||||
this->flush();
|
this->flush();
|
||||||
|
if(vlen + strlen(this->buff) >= this->buffSize) {
|
||||||
|
// Bigger than the whole buffer so stream it straight out.
|
||||||
|
if(!this->_stream) {
|
||||||
|
this->_overflow = true;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if(!escape) {
|
||||||
|
this->_stream->print(val);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
char esc[7];
|
||||||
|
this->_stream->print("\"");
|
||||||
|
for(size_t i = 0; val[i]; i++) {
|
||||||
|
escapeChar(val[i], esc);
|
||||||
|
this->_stream->print(esc);
|
||||||
|
}
|
||||||
|
this->_stream->print("\"");
|
||||||
|
return;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if(escape) strcat(this->buff, "\"");
|
if(escape) strcat(this->buff, "\"");
|
||||||
if(escape) this->escapeString(val, &this->buff[strlen(this->buff)]);
|
if(escape) this->escapeString(val, &this->buff[strlen(this->buff)]);
|
||||||
|
|
|
||||||
|
|
@ -140,6 +140,24 @@ static void test_value_needing_one_byte_too_many_is_refused() {
|
||||||
TEST_ASSERT_TRUE(strlen(storage) < 11);
|
TEST_ASSERT_TRUE(strlen(storage) < 11);
|
||||||
assertCanary(11);
|
assertCanary(11);
|
||||||
}
|
}
|
||||||
|
static void test_overflow_is_flagged() {
|
||||||
|
useBuffer(16);
|
||||||
|
fmt.beginObject();
|
||||||
|
fmt.addElem("name", "fits");
|
||||||
|
TEST_ASSERT_FALSE(fmt.overflowed());
|
||||||
|
fmt.addElem("long", "this value does not fit");
|
||||||
|
TEST_ASSERT_TRUE(fmt.overflowed());
|
||||||
|
// What was already written (here up to the comma) stays intact and terminated.
|
||||||
|
TEST_ASSERT_EQUAL_STRING("{\"name\":\"fits\",", storage);
|
||||||
|
assertCanary(16);
|
||||||
|
}
|
||||||
|
static void test_setBuffer_clears_overflow() {
|
||||||
|
useBuffer(4);
|
||||||
|
fmt.addElem("too long");
|
||||||
|
TEST_ASSERT_TRUE(fmt.overflowed());
|
||||||
|
useBuffer(BUFF_SIZE);
|
||||||
|
TEST_ASSERT_FALSE(fmt.overflowed());
|
||||||
|
}
|
||||||
|
|
||||||
int main(int argc, char **argv) {
|
int main(int argc, char **argv) {
|
||||||
(void)argc;
|
(void)argc;
|
||||||
|
|
@ -157,5 +175,7 @@ int main(int argc, char **argv) {
|
||||||
RUN_TEST(test_escaped_length);
|
RUN_TEST(test_escaped_length);
|
||||||
RUN_TEST(test_escaped_length_counts_first_character);
|
RUN_TEST(test_escaped_length_counts_first_character);
|
||||||
RUN_TEST(test_value_needing_one_byte_too_many_is_refused);
|
RUN_TEST(test_value_needing_one_byte_too_many_is_refused);
|
||||||
|
RUN_TEST(test_overflow_is_flagged);
|
||||||
|
RUN_TEST(test_setBuffer_clears_overflow);
|
||||||
return UNITY_END();
|
return UNITY_END();
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue