Skip to content

Commit f704fd7

Browse files
committed
Fix crash when http is started then stopped so webserver reference in authentication module is no more valid
Fix AuthenticationService starting logic because started too late in process Transfer http registration/unregistration to AuthenticationService fully to http server itself Add several sanity check to avoid call to not existing webserver during life cycle thanks to @RafOSS-br for pointing this out
1 parent 6d02ec7 commit f704fd7

4 files changed

Lines changed: 30 additions & 9 deletions

File tree

esp3d/src/core/esp3d.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,12 @@ bool Esp3D::begin() {
107107
restart_now();
108108
}
109109

110+
#ifdef AUTHENTICATION_FEATURE
111+
// Passwords must be loaded before any command channel (serial, USB serial,
112+
// network, ...) can start dispatching commands.
113+
AuthenticationService::begin();
114+
#endif // AUTHENTICATION_FEATURE
115+
110116
esp3d_commands.getOutputClient(true);
111117

112118
#if defined(USB_SERIAL_FEATURE)
@@ -166,9 +172,6 @@ bool Esp3D::begin() {
166172
esp3d_gcode_host.processFile(ESP_AUTOSTART_SCRIPT_FILE);
167173
#endif // ESP_AUTOSTART_FEATURE
168174
#endif // GCODE_HOST_FEATURE
169-
#ifdef AUTHENTICATION_FEATURE
170-
AuthenticationService::begin();
171-
#endif // AUTHENTICATION_FEATURE
172175
esp3d_log("Esp3d Started");
173176
_started = true;
174177
return res;

esp3d/src/modules/authentication/authentication_service.cpp

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,19 +122,23 @@ bool AuthenticationService::begin() {
122122
}
123123

124124
#if defined(HTTP_FEATURE)
125-
bool AuthenticationService::begin_session(Authwebserver *webserver) {
125+
bool AuthenticationService::attachWebServer(Authwebserver *webserver) {
126126
_webserver = webserver;
127127
// value is in ms but storage is in min
128128
_sessionTimeout = 1000 * 60 * ESP3DSettings::readByte(ESP_SESSION_TIMEOUT);
129129
return true;
130130
}
131-
#endif // HTTP_FEATURE
132131

133-
void AuthenticationService::end() {
134-
#if defined(HTTP_FEATURE)
132+
void AuthenticationService::detachWebServer() {
135133
_webserver = nullptr;
136134
ClearAllSessions();
135+
}
137136
#endif // HTTP_FEATURE
137+
138+
void AuthenticationService::end() {
139+
// webserver attach/detach and HTTP session lifecycle are now owned by
140+
// HTTP_Server (see attachWebServer/detachWebServer) to avoid this being
141+
// called out of order relative to HTTP_Server::begin()/end().
138142
}
139143

140144
void AuthenticationService::update() {
@@ -187,6 +191,10 @@ char *AuthenticationService::create_session_ID() {
187191
for (int i = 0; i < 17; i++) {
188192
sessionID[i] = '\0';
189193
}
194+
if (!_webserver) {
195+
strcpy(sessionID, "NONE");
196+
return sessionID;
197+
}
190198
// get time
191199
uint32_t now = millis();
192200
// get remote IP
@@ -215,6 +223,9 @@ bool AuthenticationService::ClearAllSessions() {
215223
}
216224

217225
bool AuthenticationService::ClearCurrentHttpSession() {
226+
if (!_webserver) {
227+
return false;
228+
}
218229
String cookie = _webserver->header("Cookie");
219230
int pos = cookie.indexOf("ESPSESSIONID=");
220231
String sessionID;
@@ -228,6 +239,9 @@ bool AuthenticationService::ClearCurrentHttpSession() {
228239
bool AuthenticationService::CreateSession(ESP3DAuthenticationLevel auth_level,
229240
ESP3DClientType client_type,
230241
const char *session_ID) {
242+
if (!_webserver) {
243+
return false;
244+
}
231245
auth_ip *current_auth = (auth_ip *)malloc(sizeof(auth_ip));
232246
if (!current_auth) {
233247
esp3d_log_e("Error allocating memory for session");

esp3d/src/modules/authentication/authentication_service.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ class AuthenticationService {
6363
static void update();
6464
static bool isuser(const char *pwd);
6565
#if defined(HTTP_FEATURE)
66-
static bool begin_session(Authwebserver *webserver);
66+
static bool attachWebServer(Authwebserver *webserver);
67+
static void detachWebServer();
6768
static uint32_t setSessionTimeout(uint32_t timeout);
6869
static uint32_t getSessionTimeout();
6970
static uint32_t getSessionRemaining(const char *sessionID);

esp3d/src/modules/http/http_server.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ bool HTTP_Server::begin() {
244244
_webserver->collectHeaders(headerkeys, headerkeyssize);
245245
_webserver->begin();
246246
#ifdef AUTHENTICATION_FEATURE
247-
AuthenticationService::begin_session(_webserver);
247+
AuthenticationService::attachWebServer(_webserver);
248248
#endif // AUTHENTICATION_FEATURE
249249

250250
_started = no_error;
@@ -334,6 +334,9 @@ Embedded; http://www.esp3d.io) Host: http://192.168.0.1:8181
334334
void HTTP_Server::end() {
335335
_started = false;
336336
_upload_status = UPLOAD_STATUS_NONE;
337+
#ifdef AUTHENTICATION_FEATURE
338+
AuthenticationService::detachWebServer();
339+
#endif // AUTHENTICATION_FEATURE
337340
if (_webserver) {
338341
_webserver->stop();
339342
delete _webserver;

0 commit comments

Comments
 (0)