Skip to content

Added a few more ESP32-ism files used by ESPAsyncWebServer#17

Merged
pschatzmann merged 8 commits intopschatzmann:mainfrom
MitchBradley:b64_md5_cbuf
Apr 2, 2026
Merged

Added a few more ESP32-ism files used by ESPAsyncWebServer#17
pschatzmann merged 8 commits intopschatzmann:mainfrom
MitchBradley:b64_md5_cbuf

Conversation

@MitchBradley
Copy link
Copy Markdown
Contributor

(Sorry about the ifdef screwup with my last PR)

ESPAsyncWebServer needs a few extra things that are present in the ESP32 Arduino framework, namely

  • libb64 - It is possible to get this from linux and macos host packages, but the ESP Arduino version of libb64 has a few tweaks that make it very cumbersome to use the standard libb64 - plus you have to make sure to install the host packages and include the libs on the linker. It is far easier to just include libb64/ here.
  • md5 - similarly, you can get it from openssl, but it is similarly cumbersome, and quite easy to just drop MD5Builder here
  • cbuf - yet another (small) ESP-ism that ESPAsyncTCP relies on

I think (hope) that this is the last set of changes related to this.

Copilot AI review requested due to automatic review settings March 31, 2026 02:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds several ESP32/ESP8266-compatibility source files into ArduinoCore-Linux/cores/arduino to satisfy dependencies required by ESPAsyncWebServer / ESPAsyncTCP (base64, MD5, and a circular buffer utility).

Changes:

  • Added an MD5Builder implementation backed by BearSSL MD5.
  • Vendored libb64 (base64 encode/decode) sources and license metadata.
  • Added cbuf circular buffer implementation used by async TCP/webserver stacks.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
ArduinoCore-Linux/cores/arduino/MD5Builder.h Declares MD5Builder API used by higher-level networking/web libs
ArduinoCore-Linux/cores/arduino/MD5Builder.cpp Implements MD5 hashing and stream ingestion
ArduinoCore-Linux/cores/arduino/libb64/LICENSE Adds libb64 licensing text
ArduinoCore-Linux/cores/arduino/libb64/AUTHORS Adds libb64 author attribution
ArduinoCore-Linux/cores/arduino/libb64/cencode.h Base64 encoder public header
ArduinoCore-Linux/cores/arduino/libb64/cencode.c Base64 encoder implementation
ArduinoCore-Linux/cores/arduino/libb64/cdecode.h Base64 decoder public header
ArduinoCore-Linux/cores/arduino/libb64/cdecode.c Base64 decoder implementation
ArduinoCore-Linux/cores/arduino/cbuf.h Declares circular buffer class used by async networking code
ArduinoCore-Linux/cores/arduino/cbuf.cpp Implements circular buffer operations (read/write/resize)

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

Comment on lines +60 to +63
bool MD5Builder::addStream(Stream &stream, const size_t maxLen) {
const int buf_size = 512;
int maxLengthLeft = maxLen;

Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addStream(), maxLen is a size_t but it is immediately narrowed into int maxLengthLeft = maxLen;. On 64-bit hosts or large inputs this can overflow/truncate and cause incorrect loop behavior. Use size_t for the remaining length (and adjust comparisons/casts around stream.available() / readBytes) to avoid narrowing.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed; maxLengthLeft is now size_t

Comment on lines +25 to +40
#pragma once

#include <api/String.h>
#include <Stream.h>
#include <bearssl/bearssl_hash.h>

class MD5Builder {
private:
br_md5_context _ctx;
uint8_t _buf[16];
public:
void begin(void);
void add(const uint8_t * data, const uint16_t len);
void add(const char * data) {
add((const uint8_t*)data, strlen(data));
}
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MD5Builder.h uses strlen() in inline methods but does not include <string.h>/<cstring>. This makes the header non-self-contained and can fail to compile depending on include order. Please include the proper header (and/or avoid calling strlen in the header).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added "include "

newSize += 1;
// not lose any data
// if data can be lost use remove or flush before resize
if((newSize < bytes_available) || (newSize == _size)) {
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In cbuf::resize(), after newSize += 1 (to account for the sentinel), the check if ((newSize < bytes_available) ...) is too weak. When newSize == bytes_available the code will set _end = newbuf + bytes_available, which equals _bufend and later writes will dereference _end out of bounds. The condition should ensure the internal buffer size is strictly greater than the number of stored bytes (e.g., newSize <= bytes_available should reject), accounting for the sentinel slot.

Suggested change
if((newSize < bytes_available) || (newSize == _size)) {
if((newSize <= bytes_available) || (newSize == _size)) {

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above

@MitchBradley
Copy link
Copy Markdown
Contributor Author

Yuck, the bearssl dependency. The ESP32 version of MD5Builder depends on the ESP32's ROM implementation of md5. This version that I included is cribbed from earlephilhower's ArduinoPico framework. Looks like I have some more work to do to clean this up.

* Eliminated MD5Builder's dependency on BearSSL by copying
  publicly-available md5 code directly into MD5Builder and
  adjusting it to use class variables instead of a separate
  context structure.
* Fixed the well-known off-by-one size bug in the cbuf code
  that was copied from the ESP32 Arduino code.  The sizes
  returned by all methods now reflect that actual data capacity
  of the buffer, without counting the sentinel byte that should
  be hidden.  I asked for multiple AI reviews of the new code and
  also verified that it is compatible with the one use of cbuf in
  ESPAsyncWebServer
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.


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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.


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

MitchBradley and others added 2 commits April 1, 2026 15:18
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.


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

MitchBradley and others added 2 commits April 1, 2026 15:36
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@MitchBradley
Copy link
Copy Markdown
Contributor Author

I think this is ready, after several rounds of CoPilot review.

@pschatzmann
Copy link
Copy Markdown
Owner

Did you test this with the ESPAsyncServer ?

I suggest that you provide a simple example in the examples directory...

@MitchBradley
Copy link
Copy Markdown
Contributor Author

Yes, I tested with ESPAsyncWebServer. I am making https://github.com/bdring/FluidNC truly portable, including running it under Linux, MacOS, and Windows. (The use cases for hosted builds include demonstration, rapid development, and integration testing). At present, it is working on ESP32 (the original target), ESP32-S3, RP2040, RP2350, Linux, MacOS, and (partially) Windows, with networking support across the board. It originally used synchronous networking, but we recently switched to ESPAsyncWebServer for performance and (hopefully) reliability reasons.

Making it work on Linux and MacOS required changes to Arduino-Emulator, ESPAsyncWebServer (mostly the addition of "|| defined (HOST}" clauses, and the creation of a Posix version of the AsyncTCP package in addition to the preexisting ESP and RP AsyncTCP packages.

Deploying this is tedious exercise since I have to PR against existing packages in addition to publishing a new one, on top of the changes to the application itself. I am trying to do the PRs in stages so I can focus.

I can create an example, but the example will not work until the other packages are ready - and the creation thereof would delay the PR activity on those packages. What do you think is the best way forward?

@pschatzmann pschatzmann merged commit 9c7eb76 into pschatzmann:main Apr 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants