ESP32 Fix #14

opensprinklershop · 2025-12-24T09:26:40Z

This change adds SSL support for ESP32 and fixes a bug that causes crashes on some situations.
Also cleaned some code

…-accelerated cipher suites, and improved memory usage by storing certificates in PROGMEM.

…SSL context configuration for ESP32

Copilot

Pull request overview

This pull request adds SSL/TLS support for ESP32 devices and includes bug fixes and code cleanup. The main addition is a comprehensive HTTPS server implementation using mbedTLS for ESP32, along with self-signed certificates for local SSL connections.

Changes:

Added SSL/TLS support for ESP32 with mbedTLS-based HTTPS server implementation
Fixed memory management bug in StringBuilder (delete vs delete[])
Refactored HTTP method enums to avoid naming conflicts (HTTP_* to OTF_HTTP_*)

Reviewed changes

Copilot reviewed 23 out of 25 changed files in this pull request and generated 15 comments.

Show a summary per file

File	Description
cert.h	New header containing self-signed SSL certificate and private key in DER format for HTTPS
Esp32LocalServer.cpp/h	Major implementation of SSL/TLS server with mbedTLS, separate HTTP and HTTPS client classes
StringBuilder.cpp	Fixed memory leak (delete vs delete[]), added appendStr methods, improved buffer handling
Request.cpp/h	Renamed HTTP method enums to OTF_HTTP_* to avoid conflicts
OpenThingsFramework.cpp/h	Added HTTPS support, pollCloud method, constructor changes for dual HTTP/HTTPS ports
Response.cpp/h	Removed writeBodyChunk methods, updated writeBodyData to use appendStr
Websocket.cpp	Added ESP32 SSL support with insecure mode for websocket connections
Esp8266LocalServer.cpp	Changed from available() to accept() for WiFiClient retrieval
library.json	Version bump to 0.2.1, changed WebSockets dependency to opensprinklershop fork
.vscode/*	IDE configuration files with user-specific paths

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-11T22:03:56Z

cert.h

+  0x32, 0x35, 0x31, 0x32, 0x33, 0x31, 0x31, 0x35, 0x31, 0x33, 0x35, 0x31,
+  0x5a, 0x17, 0x0d, 0x33, 0x35, 0x31, 0x32, 0x33, 0x31, 0x31, 0x35, 0x31,


The SSL certificate has a validity period starting from December 31, 2025 (notBefore: 251231151351Z). Since the current date is January 11, 2026, this certificate is already valid. However, certificates with future start dates can cause issues with clients that have incorrect system clocks set to dates before the certificate's validity period. Consider using a certificate with a start date in the past or documenting this requirement.

Copilot · 2026-01-11T22:03:57Z

Esp32LocalServer.cpp

+  static const int hw_accelerated_ciphersuites[] = {
+    // TLS 1.2 cipher suites with hardware acceleration (ONLY these 3)
+    0xC02B,  // TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256 (HW AES + HW ECC + HW SHA-256)
+    0xC02C,  // TLS-ECDHE-ECDSA-WITH-AES-256-GCM-SHA384 (HW AES + HW ECC + HW SHA-384)
+    0xC023,  // TLS-ECDHE-ECDSA-WITH-AES-128-CBC-SHA256 (Fallback, HW AES + HW ECC)
+
+    0        // Terminator
+  };


The cipher suite identifiers are defined as hexadecimal literals (0xC02B, 0xC02C, 0xC023) without the MBEDTLS_ prefix. While these values are correct for the named cipher suites, using the symbolic constants from mbedTLS headers (e.g., MBEDTLS_TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256) would be more maintainable and self-documenting, reducing the risk of errors if these values change in future mbedTLS versions.

Copilot · 2026-01-11T22:03:57Z

Esp32LocalServer.cpp

+  // Cleanup previous client to free memory before accepting new connections
+  // This reduces heap fragmentation by ensuring SSL resources are freed
+  if (activeClient != nullptr) {
+    delete activeClient;
+    activeClient = nullptr;
+  }


The deletion of the activeClient happens before accepting a new client, which means if the new client acceptance fails, there's no activeClient to track. While this appears to be intentional for memory management, consider whether this could cause issues if client cleanup is needed after a failed accept.

Copilot · 2026-01-11T22:03:57Z

Esp8266LocalServer.cpp

  }

-  WiFiClient wiFiClient = server.available();
+  WiFiClient wiFiClient = server.accept();


The change from server.available() to server.accept() is significant. The available() method typically returns a client if one is available without removing it from the queue, while accept() accepts and removes the connection. Verify that this change is compatible with the ESP8266 WiFiServer API, as this could cause compilation errors or unexpected behavior if the method doesn't exist or has different semantics.

Suggested change

WiFiClient wiFiClient = server.accept();

WiFiClient wiFiClient = server.available();

Copilot · 2026-01-11T22:03:57Z

Esp32LocalServer.cpp

+void Esp32HttpsClient::print(const __FlashStringHelper *data) {
+  const char* p = reinterpret_cast<const char*>(data);
+  mbedtls_ssl_write(ssl, (const unsigned char*)p, strlen(p));
+}


The __FlashStringHelper pointer is cast to char* and then passed to strlen and mbedtls_ssl_write. On ESP32, PROGMEM data should be read using strlen_P and similar functions. This could cause incorrect behavior or crashes when reading from flash memory.

Copilot · 2026-01-11T22:03:59Z

StringBuilder.cpp

+  size_t res = strlen(str);
+  strncpy(buffer+length, str, maxLength - length);
+
+  if (streaming && ((res >= maxLength) || (length + res >= maxLength))) {
+    // If in streaming mode flush the buffer and continue writing if the data doesn't fit.
+    stream_write(buffer, length, streaming);
+    first_message = false;
+    stream_flush();
+    clear();
+    strncpy(buffer+length, str, maxLength - length);
+  }
+
+  totalLength += res;
+  length += res;
+
+  // The builder is invalid if the string fits perfectly in the buffer since there wouldn't be room for the null terminator.
+  if (length >= maxLength) {
+    // snprintf will not allow more than the specified number of characters to be written to the buffer, so the length will be the buffer size.
+    length = maxLength;
+    valid = false;
+  }


The appendStr function uses strncpy which does not guarantee null-termination if the source is longer than the destination buffer size. Additionally, the logic for handling buffer overflow is flawed: the function checks if res >= maxLength or length + res >= maxLength after already copying data, but it should check before copying. If the string doesn't fit, it flushes and copies again, but the buffer state after the initial failed copy is undefined. The function should calculate if the string fits before calling strncpy, and ensure null-termination after the copy.

Suggested change

size_t res = strlen(str);

strncpy(buffer+length, str, maxLength - length);

if (streaming && ((res >= maxLength) || (length + res >= maxLength))) {

// If in streaming mode flush the buffer and continue writing if the data doesn't fit.

stream_write(buffer, length, streaming);

first_message = false;

stream_flush();

clear();

strncpy(buffer+length, str, maxLength - length);

}

totalLength += res;

length += res;

// The builder is invalid if the string fits perfectly in the buffer since there wouldn't be room for the null terminator.

if (length >= maxLength) {

// snprintf will not allow more than the specified number of characters to be written to the buffer, so the length will be the buffer size.

length = maxLength;

valid = false;

}

size_t res = strlen(str);

// Delegate to _write, which correctly handles buffer limits, streaming,

// null-termination, and validity.

size_t written = _write(str, res, false);

(void)written; // Suppress unused variable warning if callers ignore the result.

Copilot · 2026-01-11T22:03:59Z

OpenThingsFramework.cpp

  // Try to reconnect to the websocket if the connection is lost.
  webSocket->setReconnectInterval(WEBSOCKET_RECONNECT_INTERVAL);
-  // Ping the server every 15 seconds with a timeout of 5 seconds, and treat 1 missed ping as a lost connection.
+  // Ping the server every 30 seconds with a timeout of 2 seconds, and treat 3 missed ping as a lost connection.


The comment states "Ping the server every 30 seconds with a timeout of 2 seconds, and treat 3 missed ping as a lost connection" but the actual function call is enableHeartbeat(15000, 5000, 1) which means 15 seconds interval, 5 seconds timeout, and 1 missed ping. The comment does not match the code implementation.

Suggested change

// Ping the server every 30 seconds with a timeout of 2 seconds, and treat 3 missed ping as a lost connection.

// Ping the server every 15 seconds with a timeout of 5 seconds, and treat 1 missed ping as a lost connection.

Copilot · 2026-01-11T22:03:59Z

Esp32LocalServer.cpp

+  size_t index = 0;
+  while (index < length) {
+    char c;
+    int bytesRead = mbedtls_ssl_read(ssl, (unsigned char*)&c, 1);
+    if (bytesRead <= 0) break;
+    if (c == terminator) break;
+    buffer[index++] = c;
+  }


The ReadBytesUntil implementation for HTTPS reads one byte at a time via mbedtls_ssl_read, which is inefficient and could impact performance. Consider using a buffered approach or reading larger chunks when possible.

Suggested change

size_t index = 0;

while (index < length) {

char c;

int bytesRead = mbedtls_ssl_read(ssl, (unsigned char*)&c, 1);

if (bytesRead <= 0) break;

if (c == terminator) break;

buffer[index++] = c;

}

// Buffered implementation to avoid per-byte mbedtls_ssl_read calls

static unsigned char sslReadBuffer[256];

static size_t sslReadBufferLen = 0;

static size_t sslReadBufferPos = 0;

size_t index = 0;

while (index < length) {

// Refill buffer if exhausted

if (sslReadBufferPos >= sslReadBufferLen) {

int bytesRead = mbedtls_ssl_read(ssl, sslReadBuffer, sizeof(sslReadBuffer));

if (bytesRead <= 0) {

// No more data or error

break;

}

sslReadBufferLen = static_cast<size_t>(bytesRead);

sslReadBufferPos = 0;

}

char c = static_cast<char>(sslReadBuffer[sslReadBufferPos++]);

if (c == terminator) {

// Stop before writing the terminator into the destination buffer

break;

}

buffer[index++] = c;

}

Copilot · 2026-01-11T22:04:00Z

Websocket.cpp

+    WebSocketsClient::beginSSL(host.c_str(), port, path.c_str());
+
+    // Note: arduinoWebSockets library will create WiFiClientSecure internally
+    // and call setInsecure() is done via the SSL_AXTLS mode which doesn't validate certs


The comment on line 40 states "and call setInsecure() is done via the SSL_AXTLS mode" which contains a grammatical error. It should read "and setInsecure() is called via the SSL_AXTLS mode" or similar. Additionally, the comment mentions SSL_AXTLS mode but the code doesn't explicitly set this mode, making the comment potentially misleading.

Suggested change

// and call setInsecure() is done via the SSL_AXTLS mode which doesn't validate certs

// and may call setInsecure() internally when using insecure SSL modes that don't validate certs

Copilot · 2026-01-11T22:04:00Z

OpenThingsFramework.cpp

 using namespace OTF;

-OpenThingsFramework::OpenThingsFramework(uint16_t webServerPort, char *hdBuffer, int hdBufferSize) : localServer(webServerPort) {
+OpenThingsFramework::OpenThingsFramework(uint16_t webServerPort, char *hdBuffer, int hdBufferSize) : localServer(webServerPort, webServerPort + 363) {


The constructor signature changed from Esp32LocalServer(uint16_t port) to Esp32LocalServer(uint16_t port = 80, uint16_t httpsPort = 443), adding a default HTTPS port parameter. However, in OpenThingsFramework.cpp line 14, the constructor is called with localServer(webServerPort, webServerPort + 363). This uses port 443 (80 + 363) for HTTPS, which conflicts with the default parameter value of 443. The hardcoded offset of 363 seems arbitrary and should be documented or made configurable.

Stefan Schmaltz and others added 19 commits September 12, 2024 22:49

Fix delete buffer

1391411

Merge branch 'OpenThingsIO:master' into master

4527df2

Added ESP32 SSL support (port=443)

3acbb3f

Fixt crash for OTC connect on ESP32

169407c

TryFix OTC connect on ESP32

b800bd9

changed ArduinoWebSockets dependencies to own fork

5e3c62b

ESP32 adjustments

f362f69

https for esp32

2193d92

LocalServer Fix http/https

f4ef90a

Updated ESP32 https implementation

5355504

ESP32 https modifications

c4c770e

writeBodyChunk replaced by writeBodyData

1a7af67

fixed compile errors for esp8266

e3640ca

Fixt https server implementation (for ESP32)

265dfde

Remoted ssl optimizations, just caused connection problems.

6f194a2

Update server certificate with memory optimization

48edd5e

Optimized SSL/TLS configuration for ESP32, added support for hardware…

5928e56

…-accelerated cipher suites, and improved memory usage by storing certificates in PROGMEM.

Refactor HTTP method handling to use OTFHTTPMethod enum and optimize …

064e8af

…SSL context configuration for ESP32

changed the logic on receiving data

da937fa

Copilot AI review requested due to automatic review settings January 11, 2026 21:57

Copilot started reviewing on behalf of opensprinklershop January 11, 2026 21:57 View session

Copilot AI reviewed Jan 11, 2026

View reviewed changes

Stefan Schmaltz added 2 commits January 11, 2026 23:18

Timeout/SSL fix

dbe9f25

https fixes

6650d74

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ESP32 Fix #14

ESP32 Fix #14

opensprinklershop commented Dec 24, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Copilot AI Jan 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		0x32, 0x35, 0x31, 0x32, 0x33, 0x31, 0x31, 0x35, 0x31, 0x33, 0x35, 0x31,
		0x5a, 0x17, 0x0d, 0x33, 0x35, 0x31, 0x32, 0x33, 0x31, 0x31, 0x35, 0x31,

	WiFiClient wiFiClient = server.accept();
	WiFiClient wiFiClient = server.available();

	// Ping the server every 30 seconds with a timeout of 2 seconds, and treat 3 missed ping as a lost connection.
	// Ping the server every 15 seconds with a timeout of 5 seconds, and treat 1 missed ping as a lost connection.

-  size_t index = 0;
-  while (index < length) {
-    char c;
-    int bytesRead = mbedtls_ssl_read(ssl, (unsigned char*)&c, 1);
-    if (bytesRead <= 0) break;
-    if (c == terminator) break;
-    buffer[index++] = c;
-  }
+  // Buffered implementation to avoid per-byte mbedtls_ssl_read calls
+  static unsigned char sslReadBuffer[256];
+  static size_t sslReadBufferLen = 0;
+  static size_t sslReadBufferPos = 0;
+  size_t index = 0;
+  while (index < length) {
+    // Refill buffer if exhausted
+    if (sslReadBufferPos >= sslReadBufferLen) {
+      int bytesRead = mbedtls_ssl_read(ssl, sslReadBuffer, sizeof(sslReadBuffer));
+      if (bytesRead <= 0) {
+        // No more data or error
+        break;
+      }
+      sslReadBufferLen = static_cast<size_t>(bytesRead);
+      sslReadBufferPos = 0;
+    }
+    char c = static_cast<char>(sslReadBuffer[sslReadBufferPos++]);
+    if (c == terminator) {
+      // Stop before writing the terminator into the destination buffer
+      break;
+    }
+    buffer[index++] = c;
+  }

	// and call setInsecure() is done via the SSL_AXTLS mode which doesn't validate certs
	// and may call setInsecure() internally when using insecure SSL modes that don't validate certs

ESP32 Fix #14

Are you sure you want to change the base?

ESP32 Fix #14

Conversation

opensprinklershop commented Dec 24, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant