Skip to content

Commit fc67fa4

Browse files
mairasclaude
andcommitted
fix(signalk): connect WebSocket directly, detect token rejection on the upgrade
SKWSClient validated the stored auth token with a separate HTTPS GET to /signalk/v1/stream (test_token) before opening the WebSocket. On memory-constrained targets (e.g. ESP32-C3) the token-probe TLS handshake and the WebSocket TLS handshake run back-to-back; mbedTLS's two 16 KB record buffers fragment the heap, and the second handshake fails to allocate (MBEDTLS_ERR_SSL_ALLOC_FAILED) -- so the device can never connect to a TLS Signal K server. Connect the WebSocket directly with the stored token and move auth-failure recovery onto the upgrade itself: on a 401 handshake status, on_error() clears the token and the next reconnect re-requests access -- the same recovery test_token did, now driven by the real upgrade instead of a separate probe. This removes one TLS handshake per connect attempt. A non-401 transport error keeps the token and simply retries. server_detected_/token_test_success_ are set at connect time so the on_disconnected() bad-token heuristic stays inert; bad-token detection is now precise via the 401 status. Removes the now-unused test_token(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e4deda6 commit fc67fa4

2 files changed

Lines changed: 38 additions & 99 deletions

File tree

src/sensesp/signalk/signalk_ws_client.cpp

Lines changed: 35 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,17 @@ static void websocket_event_handler(void* handler_args,
135135
}
136136
break;
137137
case WEBSOCKET_EVENT_ERROR:
138-
ws_client->on_error();
138+
// The HTTP status of the failed upgrade (e.g. 401 for a rejected token)
139+
// lets us distinguish a bad token from a transient transport error. The
140+
// handshake status field only exists in the newer esp_websocket_client
141+
// component pulled for SSL builds; the version bundled with the EOL
142+
// espressif32 Arduino platform lacks it, so fall back to 0 (not
143+
// applicable), matching how on_error() treats a transient failure.
144+
#ifdef SENSESP_SSL_SUPPORT
145+
ws_client->on_error(data->error_handle.esp_ws_handshake_status_code);
146+
#else
147+
ws_client->on_error(0);
148+
#endif
139149
break;
140150
}
141151
}
@@ -219,9 +229,19 @@ void SKWSClient::on_disconnected() {
219229
* Called in the websocket task context.
220230
*
221231
*/
222-
void SKWSClient::on_error() {
232+
void SKWSClient::on_error(int handshake_status) {
223233
this->set_connection_state(SKWSConnectionState::kSKWSDisconnected);
224-
ESP_LOGW(__FILENAME__, "Websocket client error.");
234+
if (handshake_status == 401) {
235+
// The server rejected the token on the WebSocket upgrade (e.g. the server
236+
// was reinstalled, or the device was moved to a different server). Clear it
237+
// so the next reconnect requests fresh access. A non-401 error (transport,
238+
// TLS, network) leaves the token intact and simply retries.
239+
ESP_LOGW(__FILENAME__, "Token rejected (401), requesting new access");
240+
auth_token_ = NULL_AUTH_TOKEN;
241+
save();
242+
} else {
243+
ESP_LOGW(__FILENAME__, "Websocket client error.");
244+
}
225245
}
226246

227247
/**
@@ -731,106 +751,24 @@ void SKWSClient::connect() {
731751
return;
732752
}
733753

734-
// Test the validity of the authorization token
735-
this->test_token(this->server_address_, this->server_port_);
754+
// A token is already present. Connect the WebSocket directly rather than
755+
// first probing the token over a separate HTTPS request: on memory-constrained
756+
// targets (e.g. ESP32-C3) the back-to-back token-probe TLS handshake and the
757+
// WebSocket TLS handshake fragment the heap, and the second fails to allocate
758+
// (MBEDTLS_ERR_SSL_ALLOC_FAILED). The server validates the token on the
759+
// upgrade itself; a 401 there is handled in on_error() (clears the token and
760+
// re-requests access on the next reconnect). server_detected_/
761+
// token_test_success_ are set so the on_disconnected() bad-token heuristic
762+
// stays inert -- bad-token detection is now precise via the 401 status.
763+
server_detected_ = true;
764+
token_test_success_ = true;
765+
this->connect_ws(this->server_address_, this->server_port_);
736766
}
737767

738768
void SKWSClient::loop() {
739769
// No-op: esp_websocket_client handles data via event callbacks
740770
}
741771

742-
void SKWSClient::test_token(const String server_address,
743-
const uint16_t server_port) {
744-
String protocol = ssl_enabled_ ? "https://" : "http://";
745-
String url = protocol + server_address + ":" + server_port +
746-
"/signalk/v1/stream";
747-
ESP_LOGD(__FILENAME__, "Testing token with url %s", url.c_str());
748-
749-
const String full_token = String("Bearer ") + auth_token_;
750-
ESP_LOGD(__FILENAME__, "Authorization: %.8s...[redacted]", full_token.c_str());
751-
752-
esp_http_client_config_t config = {};
753-
config.url = url.c_str();
754-
config.timeout_ms = 10000;
755-
#ifdef SENSESP_SSL_SUPPORT
756-
if (ssl_enabled_) {
757-
config.crt_bundle_attach = tofu_crt_bundle_attach;
758-
config.skip_cert_common_name_check = true;
759-
}
760-
#endif
761-
762-
esp_http_client_handle_t client = esp_http_client_init(&config);
763-
if (client == nullptr) {
764-
ESP_LOGE(__FILENAME__, "Failed to initialize HTTP client");
765-
set_connection_state(SKWSConnectionState::kSKWSDisconnected);
766-
return;
767-
}
768-
769-
esp_http_client_set_header(client, "Authorization", full_token.c_str());
770-
771-
// Use streaming API for GET request
772-
esp_err_t err = esp_http_client_open(client, 0);
773-
if (err != ESP_OK) {
774-
ESP_LOGE(__FILENAME__, "Failed to open HTTP connection: %s", esp_err_to_name(err));
775-
esp_http_client_cleanup(client);
776-
set_connection_state(SKWSConnectionState::kSKWSDisconnected);
777-
return;
778-
}
779-
780-
int content_length = esp_http_client_fetch_headers(client);
781-
int http_code = esp_http_client_get_status_code(client);
782-
783-
ESP_LOGD(__FILENAME__, "Testing resulted in http status %d", http_code);
784-
785-
// Read response body
786-
String payload;
787-
if (content_length > 0 && content_length < 4096) {
788-
char* buffer = new char[content_length + 1];
789-
int read_len = esp_http_client_read(client, buffer, content_length);
790-
buffer[read_len > 0 ? read_len : 0] = '\0';
791-
payload = String(buffer);
792-
delete[] buffer;
793-
} else {
794-
// Chunked encoding or unknown/large content length - read in chunks
795-
char buffer[512];
796-
int read_len;
797-
while ((read_len = esp_http_client_read(client, buffer, sizeof(buffer) - 1)) > 0) {
798-
buffer[read_len] = '\0';
799-
payload += String(buffer);
800-
if (payload.length() > 4096) break;
801-
}
802-
}
803-
804-
esp_http_client_close(client);
805-
esp_http_client_cleanup(client);
806-
807-
if (payload.length() > 0) {
808-
ESP_LOGD(__FILENAME__, "Returned payload (%d bytes): %s",
809-
payload.length(), payload.c_str());
810-
}
811-
812-
if (http_code == 426) {
813-
// HTTP status 426 is "Upgrade Required", which is the expected
814-
// response for a websocket connection.
815-
ESP_LOGD(__FILENAME__, "Attempting to connect to Signal K Websocket...");
816-
server_detected_ = true;
817-
token_test_success_ = true;
818-
this->connect_ws(server_address, server_port);
819-
} else if (http_code == 401) {
820-
// Token is invalid/expired - clear it and request new access
821-
// Keep client_id_ so we reuse the same device identity
822-
ESP_LOGW(__FILENAME__, "Token rejected (401), requesting new access");
823-
this->auth_token_ = NULL_AUTH_TOKEN;
824-
this->save();
825-
this->send_access_request(server_address, server_port);
826-
} else if (http_code > 0) {
827-
set_connection_state(SKWSConnectionState::kSKWSDisconnected);
828-
} else {
829-
ESP_LOGE(__FILENAME__, "HTTP request failed with code %d", http_code);
830-
set_connection_state(SKWSConnectionState::kSKWSDisconnected);
831-
}
832-
}
833-
834772
void SKWSClient::send_access_request(const String server_address,
835773
const uint16_t server_port) {
836774
ESP_LOGD(__FILENAME__, "Sending access request (client_id=%s, ssl=%d)",

src/sensesp/signalk/signalk_ws_client.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,9 @@ class SKWSClient : public FileSystemSaveable,
8989
// SKWSClient task methods
9090

9191
void on_disconnected();
92-
void on_error();
92+
// handshake_status is the HTTP status of a failed WebSocket upgrade (0 if not
93+
// applicable); a 401 means the auth token was rejected.
94+
void on_error(int handshake_status);
9395
void on_connected();
9496
void on_receive_delta(uint8_t* payload, size_t length);
9597
void on_receive_updates(JsonDocument& message);
@@ -295,7 +297,6 @@ class SKWSClient : public FileSystemSaveable,
295297
/////////////////////////////////////////////////////////
296298
// SKWSClient task methods
297299

298-
void test_token(const String host, const uint16_t port);
299300
void send_access_request(const String host, const uint16_t port);
300301
void poll_access_request(const String host, const uint16_t port,
301302
const String href);

0 commit comments

Comments
 (0)