[SPIFFS] optimizations file access & 0-terminate strings in settings

Some settings have char arrays. Make sure these are 0-terminated when stored or loaded.
For example the ExtraTaskSettings were used without these checks
This commit is contained in:
TD-er
2019-04-04 00:33:20 +02:00
parent 118e09be11
commit 4aaa3773fc
9 changed files with 143 additions and 76 deletions
+84 -38
View File
@@ -7,6 +7,7 @@
#endif
#include <cstddef>
#include "FS.h"
// ********************************************************************************
// Check struct sizes at compile time
@@ -20,9 +21,11 @@
// ********************************************************************************
template <typename ToCheck, std::size_t ExpectedSize, std::size_t RealSize = sizeof(ToCheck)>
void check_size() {
static_assert(ExpectedSize == RealSize, "Size is off!");
static_assert(ExpectedSize == RealSize, "");
}
// ********************************************************************************
// User specific configuration
// ********************************************************************************
@@ -449,6 +452,8 @@ void check_size() {
#define CONFIG_FILE_SIZE 131072
#endif
#define ZERO_FILL(S) memset((S), 0, sizeof(S))
#define ZERO_TERMINATE(S) S[sizeof(S) - 1] = 0
// Forward declaration
struct ControllerSettingsStruct;
@@ -470,6 +475,8 @@ void setBitToUL(uint32_t& number, byte bitnr, bool value);
void serialHelper_getGpioNames(struct EventStruct *event, bool rxOptional=false, bool txOptional=false);
fs::File tryOpenFile(const String& fname, const String& mode);
enum SettingsType {
BasicSettings_Type = 0,
TaskSettings_Type,
@@ -689,16 +696,29 @@ bool safe_strncpy(char* dest, const char* source, size_t max_size);
struct SecurityStruct
{
SecurityStruct() {
memset(WifiSSID, 0, sizeof(WifiSSID));
memset(WifiKey, 0, sizeof(WifiKey));
memset(WifiSSID2, 0, sizeof(WifiSSID2));
memset(WifiKey2, 0, sizeof(WifiKey2));
memset(WifiAPKey, 0, sizeof(WifiAPKey));
ZERO_FILL(WifiSSID);
ZERO_FILL(WifiKey);
ZERO_FILL(WifiSSID2);
ZERO_FILL(WifiKey2);
ZERO_FILL(WifiAPKey);
for (byte i = 0; i < CONTROLLER_MAX; ++i) {
memset(ControllerUser[i], 0, sizeof(ControllerUser[i]));
memset(ControllerPassword[i], 0, sizeof(ControllerPassword[i]));
ZERO_FILL(ControllerUser[i]);
ZERO_FILL(ControllerPassword[i]);
}
memset(Password, 0, sizeof(Password));
ZERO_FILL(Password);
}
void validate() {
ZERO_TERMINATE(WifiSSID);
ZERO_TERMINATE(WifiKey);
ZERO_TERMINATE(WifiSSID2);
ZERO_TERMINATE(WifiKey2);
ZERO_TERMINATE(WifiAPKey);
for (byte i = 0; i < CONTROLLER_MAX; ++i) {
ZERO_TERMINATE(ControllerUser[i]);
ZERO_TERMINATE(ControllerPassword[i]);
}
ZERO_TERMINATE(Password);
}
char WifiSSID[32];
@@ -772,6 +792,8 @@ struct SettingsStruct
if (Latitude < -90.0 || Latitude > 90.0) Latitude = 0.0;
if (Longitude < -180.0 || Longitude > 180.0) Longitude = 0.0;
if (VariousBits1 > (1 << 30)) VariousBits1 = 0;
ZERO_TERMINATE(Name);
ZERO_TERMINATE(NTPHost);
}
bool networkSettingsEmpty() {
@@ -789,7 +811,7 @@ struct SettingsStruct
void clearTimeSettings() {
UseNTP = false;
NTPHost[0] = 0;
ZERO_FILL(NTPHost);
TimeZone = 0;
DST = false;
DST_Start = 0;
@@ -829,7 +851,7 @@ struct SettingsStruct
void clearUnitNameSettings() {
Unit = 0;
Name[0] = 0;
ZERO_FILL(Name);
UDPPort = 0;
}
@@ -1023,13 +1045,14 @@ struct ControllerSettingsStruct
for (byte i = 0; i < 4; ++i) {
IP[i] = 0;
}
memset(HostName, 0, sizeof(HostName));
memset(Publish, 0, sizeof(Publish));
memset(Subscribe, 0, sizeof(Subscribe));
memset(MQTTLwtTopic, 0, sizeof(MQTTLwtTopic));
memset(LWTMessageConnect, 0, sizeof(LWTMessageConnect));
memset(LWTMessageDisconnect, 0, sizeof(LWTMessageDisconnect));
ZERO_FILL(HostName);
ZERO_FILL(Publish);
ZERO_FILL(Subscribe);
ZERO_FILL(MQTTLwtTopic);
ZERO_FILL(LWTMessageConnect);
ZERO_FILL(LWTMessageDisconnect);
}
boolean UseDNS;
byte IP[4];
unsigned int Port;
@@ -1057,6 +1080,12 @@ struct ControllerSettingsStruct
if (ClientTimeout < 10 || ClientTimeout > CONTROLLER_CLIENTTIMEOUT_MAX) {
ClientTimeout = CONTROLLER_CLIENTTIMEOUT_DFLT;
}
ZERO_TERMINATE(HostName);
ZERO_TERMINATE(Publish);
ZERO_TERMINATE(Subscribe);
ZERO_TERMINATE(MQTTLwtTopic);
ZERO_TERMINATE(LWTMessageConnect);
ZERO_TERMINATE(LWTMessageDisconnect);
}
IPAddress getIP() const {
@@ -1171,14 +1200,25 @@ typedef std::shared_ptr<ControllerSettingsStruct> ControllerSettingsStruct_ptr_t
struct NotificationSettingsStruct
{
NotificationSettingsStruct() : Port(0), Pin1(0), Pin2(0) {
memset(Server, 0, sizeof(Server));
memset(Domain, 0, sizeof(Domain));
memset(Sender, 0, sizeof(Sender));
memset(Receiver, 0, sizeof(Receiver));
memset(Subject, 0, sizeof(Subject));
memset(Body, 0, sizeof(Body));
memset(User, 0, sizeof(User));
memset(Pass, 0, sizeof(Pass));
ZERO_FILL(Server);
ZERO_FILL(Domain);
ZERO_FILL(Sender);
ZERO_FILL(Receiver);
ZERO_FILL(Subject);
ZERO_FILL(Body);
ZERO_FILL(User);
ZERO_FILL(Pass);
}
void validate() {
ZERO_TERMINATE(Server);
ZERO_TERMINATE(Domain);
ZERO_TERMINATE(Sender);
ZERO_TERMINATE(Receiver);
ZERO_TERMINATE(Subject);
ZERO_TERMINATE(Body);
ZERO_TERMINATE(User);
ZERO_TERMINATE(Pass);
}
char Server[65];
@@ -1216,15 +1256,11 @@ struct ExtraTaskSettingsStruct
void clear() {
TaskIndex = TASKS_MAX;
for (byte j = 0; j < (NAME_FORMULA_LENGTH_MAX + 1); ++j) {
TaskDeviceName[j] = 0;
}
ZERO_FILL(TaskDeviceName);
for (byte i = 0; i < VARS_PER_TASK; ++i) {
for (byte j = 0; j < (NAME_FORMULA_LENGTH_MAX + 1); ++j) {
TaskDeviceFormula[i][j] = 0;
TaskDeviceValueNames[i][j] = 0;
TaskDeviceValueDecimals[i] = 2;
}
TaskDeviceValueDecimals[i] = 2;
ZERO_FILL(TaskDeviceFormula[i]);
ZERO_FILL(TaskDeviceValueNames[i]);
}
for (byte i = 0; i < PLUGIN_EXTRACONFIGVAR_MAX; ++i) {
TaskDevicePluginConfigLong[i] = 0;
@@ -1232,6 +1268,14 @@ struct ExtraTaskSettingsStruct
}
}
void validate() {
ZERO_TERMINATE(TaskDeviceName);
for (byte i = 0; i < VARS_PER_TASK; ++i) {
ZERO_TERMINATE(TaskDeviceFormula[i]);
ZERO_TERMINATE(TaskDeviceValueNames[i]);
}
}
bool checkUniqueValueNames() {
for (int i = 0; i < (VARS_PER_TASK - 1); ++i) {
for (int j = i; j < VARS_PER_TASK; ++j) {
@@ -2008,11 +2052,12 @@ unsigned long timingstats_last_reset = 0;
#define WIFI_ISCONNECTED_STATS 32
#define WIFI_NOTCONNECTED_STATS 33
#define LOAD_TASK_SETTINGS 34
#define RULES_PROCESSING 35
#define GRAT_ARP_STATS 36
#define BACKGROUND_TASKS 37
#define HANDLE_SCHEDULER_IDLE 38
#define HANDLE_SCHEDULER_TASK 39
#define TRY_OPEN_FILE 35
#define RULES_PROCESSING 36
#define GRAT_ARP_STATS 37
#define BACKGROUND_TASKS 38
#define HANDLE_SCHEDULER_IDLE 39
#define HANDLE_SCHEDULER_TASK 40
@@ -2048,6 +2093,7 @@ String getMiscStatsName(int stat) {
case WIFI_ISCONNECTED_STATS: return F("WiFi.isConnected()");
case WIFI_NOTCONNECTED_STATS: return F("WiFi.isConnected() (fail)");
case LOAD_TASK_SETTINGS: return F("LoadTaskSettings()");
case TRY_OPEN_FILE: return F("TryOpenFile()");
case RULES_PROCESSING: return F("rulesProcessing()");
case GRAT_ARP_STATS: return F("sendGratuitousARP()");
case BACKGROUND_TASKS: return F("backgroundtasks()");
+2 -2
View File
@@ -116,7 +116,7 @@ void rulesProcessing(String &event) {
Rules processing
\*********************************************************************************************/
String rulesProcessingFile(const String &fileName, String &event) {
if (!Settings.UseRules)
if (!Settings.UseRules || !fileExists(fileName))
return "";
checkRAM(F("rulesProcessingFile"));
#ifndef BUILD_NO_DEBUG
@@ -138,7 +138,7 @@ String rulesProcessingFile(const String &fileName, String &event) {
return (log);
}
fs::File f = SPIFFS.open(fileName, "r+");
fs::File f = tryOpenFile(fileName, "r+");
SPIFFS_CHECK(f, fileName.c_str());
String line = "";
+30 -13
View File
@@ -42,18 +42,32 @@ String flashGuard()
String appendLineToFile(const String& fname, const String& line) {
fs::File f = SPIFFS.open(fname, "a+");
return appendToFile(fname, reinterpret_cast<const uint8_t* >(line.c_str()), line.length());
}
String appendToFile(const String& fname, const uint8_t* data, unsigned int size) {
fs::File f = tryOpenFile(fname, "a+");
SPIFFS_CHECK(f, fname.c_str());
const size_t lineLength = line.length();
for (size_t i = 0; i < lineLength; ++i) {
// See https://github.com/esp8266/Arduino/commit/b1da9eda467cc935307d553692fdde2e670db258#r32622483
uint8_t value = static_cast<uint8_t>(line[i]);
SPIFFS_CHECK(f.write(&value, 1), fname.c_str());
}
SPIFFS_CHECK(f.write(data, size), fname.c_str());
f.close();
return "";
}
bool fileExists(const String& fname) {
return SPIFFS.exists(fname);
}
fs::File tryOpenFile(const String& fname, const String& mode) {
START_TIMER;
fs::File f;
if (mode == "r" && !fileExists(fname)) {
return f;
}
f = SPIFFS.open(fname, mode.c_str());
STOP_TIMER(TRY_OPEN_FILE);
return f;
}
/********************************************************************************************\
Fix stuff to clear out differences between releases
@@ -66,7 +80,7 @@ String BuildFixes()
if (Settings.Build < 145)
{
String fname=F(FILE_NOTIFICATION);
fs::File f = SPIFFS.open(fname, "w");
fs::File f = tryOpenFile(fname, "w");
SPIFFS_CHECK(f, fname.c_str());
if (f)
@@ -129,7 +143,7 @@ void fileSystemCheck()
}
#endif
fs::File f = SPIFFS.open(FILE_CONFIG, "r");
fs::File f = tryOpenFile(FILE_CONFIG, "r");
if (!f)
{
ResetFactory();
@@ -178,6 +192,7 @@ String SaveSettings(void)
// }
SecuritySettings.validate();
memcpy( SecuritySettings.ProgmemMd5, CRCValues.runTimeMD5, 16);
md5.begin();
md5.add((uint8_t *)&SecuritySettings, sizeof(SecuritySettings)-16);
@@ -257,6 +272,7 @@ String LoadSettings()
}
setUseStaticIP(useStaticIP());
afterloadSettings();
SecuritySettings.validate();
return(err);
}
@@ -441,6 +457,7 @@ String LoadTaskSettings(byte TaskIndex)
//the plugin call should populate ExtraTaskSettings with its default values.
PluginCall(PLUGIN_GET_DEVICEVALUENAMES, &TempEvent, dummyString);
}
ExtraTaskSettings.validate();
STOP_TIMER(LOAD_TASK_SETTINGS);
return result;
@@ -570,7 +587,7 @@ String InitFile(const char* fname, int datasize)
checkRAM(F("InitFile"));
FLASH_GUARD();
fs::File f = SPIFFS.open(fname, "w");
fs::File f = tryOpenFile(fname, "w");
if (f) {
SPIFFS_CHECK(f, fname);
@@ -618,7 +635,7 @@ String SaveToFile(char* fname, int index, byte* memAddress, int datasize)
}
delay(1);
unsigned long timer = millis() + 50;
fs::File f = SPIFFS.open(fname, "r+");
fs::File f = tryOpenFile(fname, "r+");
if (f) {
SPIFFS_CHECK(f, fname);
SPIFFS_CHECK(f.seek(index, fs::SeekSet), fname);
@@ -677,7 +694,7 @@ String ClearInFile(char* fname, int index, int datasize)
checkRAM(F("ClearInFile"));
FLASH_GUARD();
fs::File f = SPIFFS.open(fname, "r+");
fs::File f = tryOpenFile(fname, "r+");
if (f) {
SPIFFS_CHECK(f, fname);
@@ -718,7 +735,7 @@ String LoadFromFile(char* fname, int offset, byte* memAddress, int datasize)
START_TIMER;
checkRAM(F("LoadFromFile"));
fs::File f = SPIFFS.open(fname, "r+");
fs::File f = tryOpenFile(fname, "r");
SPIFFS_CHECK(f, fname);
SPIFFS_CHECK(f.seek(offset, fs::SeekSet), fname);
SPIFFS_CHECK(f.read(memAddress,datasize), fname);
+11 -14
View File
@@ -250,7 +250,7 @@ void sendHeadandTail(const String& tmplName, boolean Tail = false, boolean reboo
String pageTemplate = "";
String fileName = tmplName;
fileName += F(".htm");
fs::File f = SPIFFS.open(fileName, "r+");
fs::File f = tryOpenFile(fileName, "r");
if (f) {
pageTemplate.reserve(f.size());
@@ -822,7 +822,7 @@ void writeDefaultCSS(void)
{
String defaultCSS;
fs::File f = SPIFFS.open(F("esp.css"), "w");
fs::File f = tryOpenFile(F("esp.css"), "w");
if (f)
{
if (loglevelActiveFor(LOG_LEVEL_INFO)) {
@@ -1712,6 +1712,7 @@ void handle_notifications() {
for (byte x = 0; x < NOTIFICATION_MAX; x++)
{
LoadNotificationSettings(x, (byte*)&NotificationSettings, sizeof(NotificationSettingsStruct));
NotificationSettings.validate();
html_TR_TD();
html_add_button_prefix();
TXBuffer += F("notifications?index=");
@@ -1770,6 +1771,7 @@ void handle_notifications() {
{
MakeNotificationSettings(NotificationSettings);
LoadNotificationSettings(notificationindex, (byte*)&NotificationSettings, sizeof(NotificationSettingsStruct));
NotificationSettings.validate();
byte NotificationProtocolIndex = getNotificationProtocolIndex(Settings.Notification[notificationindex]);
if (NotificationProtocolIndex!=NPLUGIN_NOT_FOUND)
@@ -4083,7 +4085,6 @@ void handle_i2cscanner_json() {
TXBuffer.startJsonStream();
TXBuffer += "[{";
char *TempString = (char*)malloc(80);
bool firstentry = true;
byte error, address;
for (address = 1; address <= 127; address++ )
@@ -4101,7 +4102,6 @@ void handle_i2cscanner_json() {
}
TXBuffer += "]";
TXBuffer.endStream();
free(TempString);
}
#endif // WEBSERVER_NEW_UI
@@ -4112,8 +4112,6 @@ void handle_i2cscanner() {
TXBuffer.startStream();
sendHeadandTail_stdtemplate(_HEAD);
char *TempString = (char*)malloc(80);
html_table_class_multirow();
html_table_header(F("I2C Addresses in use"));
html_table_header(F("Supported devices"));
@@ -4241,7 +4239,6 @@ void handle_i2cscanner() {
html_end_table();
sendHeadandTail_stdtemplate(_TAIL);
TXBuffer.endStream();
free(TempString);
}
#ifdef WEBSERVER_NEW_UI
@@ -5171,7 +5168,7 @@ void handle_download()
// sendHeadandTail_stdtemplate();
fs::File dataFile = SPIFFS.open(F(FILE_CONFIG), "r");
fs::File dataFile = tryOpenFile(F(FILE_CONFIG), "r");
if (!dataFile)
return;
@@ -5316,7 +5313,7 @@ void handleFileUpload() {
{
// once we're safe, remove file and create empty one...
SPIFFS.remove((char *)upload.filename.c_str());
uploadFile = SPIFFS.open(upload.filename.c_str(), "w");
uploadFile = tryOpenFile(upload.filename.c_str(), "w");
// dont count manual uploads: flashCount();
}
}
@@ -5380,7 +5377,7 @@ bool loadFromFS(boolean spiffs, String path) {
path = path.substring(1);
if (spiffs)
{
fs::File dataFile = SPIFFS.open(path.c_str(), "r");
fs::File dataFile = tryOpenFile(path.c_str(), "r");
if (!dataFile)
return false;
@@ -5424,7 +5421,7 @@ boolean handle_custom(String path) {
path = path.substring(1);
// create a dynamic custom page, parsing task values into [<taskname>#<taskvalue>] placeholders and parsing %xx% system variables
fs::File dataFile = SPIFFS.open(path.c_str(), "r");
fs::File dataFile = tryOpenFile(path.c_str(), "r");
const bool dashboardPage = path.startsWith(F("dashboard"));
if (!dataFile && !dashboardPage) {
return false; // unknown file that does not exist...
@@ -6389,7 +6386,7 @@ void handle_rules() {
// }
// else
// {
fs::File f = SPIFFS.open(fileName, "w");
fs::File f = tryOpenFile(fileName, "w");
if (f)
{
log += F(" Write to file: ");
@@ -6407,7 +6404,7 @@ void handle_rules() {
{
log += F(" Create new file: ");
log += fileName;
fs::File f = SPIFFS.open(fileName, "w");
fs::File f = tryOpenFile(fileName, "w");
if (f) f.close();
}
}
@@ -6450,7 +6447,7 @@ void handle_rules() {
// load form data from flash
int size = 0;
fs::File f = SPIFFS.open(fileName, "r+");
fs::File f = tryOpenFile(fileName, "r");
if (f)
{
size = f.size();
+6 -5
View File
@@ -314,7 +314,7 @@ bool handle_rules_edit(String originalUri, bool isAddNew) {
// Passed all checks, write file
else
{
fs::File f = SPIFFS.open(fileName, "w");
fs::File f = tryOpenFile(fileName, "w");
if (f)
{
addLog(LOG_LEVEL_INFO, String(F(" Write to file: ")) + fileName);
@@ -371,13 +371,14 @@ bool handle_rules_edit(String originalUri, bool isAddNew) {
int size = 0;
if(!isOverwrite)
{
rules = String(F(""));
fs::File f = SPIFFS.open(fileName, "r+");
rules = "";
fs::File f = tryOpenFile(fileName, "r");
if (f)
{
size = f.size();
if (size < RULES_MAX_SIZE)
{
rules.reserve(size);
while (f.available())
{
rules += (char)f.read();
@@ -424,7 +425,7 @@ bool Rule_Download(const String& path)
Serial.print(F("Rule_Download path: "));
Serial.println(path);
#endif
fs::File dataFile = SPIFFS.open(path, "r");
fs::File dataFile = tryOpenFile(path, "r");
if (!dataFile)
{
addLog(LOG_LEVEL_ERROR, String(F("Invalid path: ")) + path);
@@ -477,7 +478,7 @@ bool EnumerateFileAndDirectory(String& rootPath
hasMore = dir.next();
#endif
#ifdef ESP32
File root = SPIFFS.open(rootPath);
File root = tryOpenFile(rootPath);
if (root)
{
File file = root.openNextFile();
+1
View File
@@ -50,6 +50,7 @@ boolean NPlugin_001(byte function, struct EventStruct *event, String& string)
{
MakeNotificationSettings(NotificationSettings);
LoadNotificationSettings(event->NotificationIndex, (byte*)&NotificationSettings, sizeof(NotificationSettingsStruct));
NotificationSettings.validate();
String subject = NotificationSettings.Subject;
String body = "";
if (event->String1.length() > 0)
+1
View File
@@ -46,6 +46,7 @@ boolean NPlugin_002(byte function, struct EventStruct *event, String& string)
{
MakeNotificationSettings(NotificationSettings);
LoadNotificationSettings(event->NotificationIndex, (byte*)&NotificationSettings, sizeof(NotificationSettingsStruct));
NotificationSettings.validate();
//this reserves IRAM and uninitialized RAM
#ifndef ESP32
// Buzzer not compatible with ESP32 due to lack of tone command.
+5 -2
View File
@@ -73,8 +73,11 @@ void Plugin_037_update_connect_status() {
}
if (!connected) {
// workaround see: https://github.com/esp8266/Arduino/issues/4497#issuecomment-373023864
espclient_037 = WiFiClient();
espclient_037.setTimeout(CONTROLLER_CLIENTTIMEOUT_DFLT);
if (MQTTclient_037 != NULL) {
espclient_037 = WiFiClient();
espclient_037.setTimeout(CONTROLLER_CLIENTTIMEOUT_DFLT);
MQTTclient_037->setClient(espclient_037);
}
++reconnectCount;
addLog(LOG_LEVEL_ERROR, F("IMPT : MQTT 037 Connection lost"));
}
+3 -2
View File
@@ -461,7 +461,7 @@ void Plugin_055_WriteChime(const String& name, const String& tokens)
log += fileName;
log += ' ';
fs::File f = SPIFFS.open(fileName, "w");
fs::File f = tryOpenFile(fileName, "w");
if (f)
{
f.print(tokens);
@@ -484,9 +484,10 @@ byte Plugin_055_ReadChime(const String& name, String& tokens)
log += ' ';
tokens = "";
fs::File f = SPIFFS.open(fileName, "r+");
fs::File f = tryOpenFile(fileName, "r");
if (f)
{
tokens.reserve(f.size());
char c;
while (f.available())
{