Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(webserver): Middleware with default middleware for cors, authc, curl-like logging #10750

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mathieucarbou
Copy link
Contributor

For history, please see: #10186


This PR improves WebServer with the following additions:

  • Add Middleware support (aka Expressif) with some built-in middlewares like cors, authc and logging (curl-like)
  • Add ability to collect all incoming request headers
  • Added response code and info on server

Copy link
Contributor

github-actions bot commented Dec 18, 2024

Messages
📖 You might consider squashing your 4 commits (simplifying branch history).

👋 Hello mathieucarbou, we appreciate your contribution to this project!


Click to see more instructions ...


This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Addressing info messages (📖) is strongly recommended; they're less critical but valuable.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...


We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against f977d60

@mathieucarbou
Copy link
Contributor Author

Note:

I kept using chained pointers, I don't know why STL lists are not used across the code, if it is because Arduino Core has to support AVR-like boards where C++ STL is not available ?

Because if it is the case, then why not creating a sort of ArduinoList (like it was done with String) that could be used everywhere, instead of repeating the CRUD logic to manage a list everywhere ? Maybe these structures exist already ?

Copy link
Contributor

github-actions bot commented Dec 18, 2024

Test Results

 62 files   62 suites   5m 41s ⏱️
 21 tests  21 ✅ 0 💤 0 ❌
154 runs  154 ✅ 0 💤 0 ❌

Results for commit f977d60.

♻️ This comment has been updated with latest results.

@me-no-dev
Copy link
Member

It's been on my backlog to create something that is thread-safe, but so is a nice Task class... maybe we will find time in near future to do it. In order to have real impact and because the core is partially in C, it would be nice to have a C version that would be implemented in the CPP as well. Then we can go around and replace all lists with that

Copy link
Contributor

github-actions bot commented Dec 18, 2024

Memory usage test (comparing PR against master branch)

The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.

MemoryFLASH [bytes]FLASH [%]RAM [bytes]RAM [%]
TargetDECINCDECINCDECINCDECINC
ESP32P40⚠️ +20020.00⚠️ +0.280⚠️ +160.00⚠️ +0.06
ESP32S30‼️ +3K0.00⚠️ +0.34000.000.00
ESP32S20‼️ +3K0.00⚠️ +0.35000.000.00
ESP32C30⚠️ +20000.00⚠️ +0.200⚠️ +160.00⚠️ +0.04
ESP32C60⚠️ +20000.00⚠️ +0.200⚠️ +160.00⚠️ +0.02
ESP320‼️ +3K0.00⚠️ +0.32000.000.00
Click to expand the detailed deltas report [usage change in BYTES]
TargetESP32P4ESP32S3ESP32S2ESP32C3ESP32C6ESP32
ExampleFLASHRAMFLASHRAMFLASHRAMFLASHRAMFLASHRAMFLASHRAM
WebServer/examples/AdvancedWebServer⚠️ +18940‼️ +3K0‼️ +3K0⚠️ +18980⚠️ +18940‼️ +3K0
WebServer/examples/FSBrowser⚠️ +18900‼️ +3K0‼️ +3K0⚠️ +1890⚠️ +16⚠️ +18900‼️ +3K0
WebServer/examples/Filters⚠️ +18940‼️ +3K0‼️ +3K0⚠️ +18980⚠️ +18940‼️ +3K0
WebServer/examples/HelloServer⚠️ +18940‼️ +3K0‼️ +3K0⚠️ +18980⚠️ +18960‼️ +3K0
WebServer/examples/HttpAdvancedAuth⚠️ +1882⚠️ +16‼️ +3K0‼️ +3K0⚠️ +18780⚠️ +1882⚠️ +16‼️ +3K0
WebServer/examples/HttpAuthCallback⚠️ +1882⚠️ +16‼️ +3K0‼️ +3K0⚠️ +18800⚠️ +1882⚠️ +16‼️ +3K0
WebServer/examples/HttpAuthCallbackInline⚠️ +1882⚠️ +16‼️ +3K0‼️ +3K0⚠️ +18800⚠️ +1882⚠️ +16‼️ +3K0
WebServer/examples/HttpBasicAuth⚠️ +1882⚠️ +16‼️ +3K0‼️ +3K0⚠️ +18780⚠️ +1882⚠️ +16‼️ +3K0
WebServer/examples/HttpBasicAuthSHA1⚠️ +1882⚠️ +16‼️ +3K0‼️ +3K0⚠️ +18780⚠️ +1882⚠️ +16‼️ +3K0
WebServer/examples/HttpBasicAuthSHA1orBearerToken⚠️ +18820‼️ +3K0‼️ +3K0⚠️ +1878⚠️ +16⚠️ +18860‼️ +3K0
WebServer/examples/MultiHomedServers⚠️ +18820‼️ +3K0‼️ +3K0⚠️ +18860⚠️ +18840‼️ +3K0
WebServer/examples/PathArgServer⚠️ +18820‼️ +3K0‼️ +3K0⚠️ +18860⚠️ +18820‼️ +3K0
WebServer/examples/SDWebServer⚠️ +1894⚠️ +16‼️ +3K0‼️ +3K0⚠️ +1894⚠️ +16⚠️ +1894⚠️ +16‼️ +3K0
WebServer/examples/SimpleAuthentification⚠️ +18600‼️ +3K0‼️ +3K0⚠️ +18640⚠️ +18600‼️ +3K0
WebServer/examples/UploadHugeFile⚠️ +18980‼️ +3K0‼️ +3K0⚠️ +18980⚠️ +18980‼️ +3K0
WebServer/examples/WebServer⚠️ +2002⚠️ +16‼️ +3K0‼️ +3K0⚠️ +20000⚠️ +2000⚠️ +16‼️ +3K0
WebServer/examples/WebUpdate⚠️ +18820‼️ +3K0‼️ +3K0⚠️ +18820⚠️ +18820‼️ +3K0
WebServer/examples/Middleware------------

@lucasssvaz lucasssvaz added the Area: Libraries Issue is related to Library support. label Dec 18, 2024
@SuGlider SuGlider added this to the 3.1.0 milestone Dec 19, 2024
@SuGlider SuGlider added the Status: Pending Merge Pull Request is ready to be merged label Dec 19, 2024
@SuGlider SuGlider modified the milestones: 3.1.0, 3.1.1 Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Libraries Issue is related to Library support. Status: Pending Merge Pull Request is ready to be merged
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

4 participants