-
Notifications
You must be signed in to change notification settings - Fork 7.8k
WebServer: Use client() instead of _currentClient #11945
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
WebServer: Use client() instead of _currentClient #11945
Conversation
CLA assistant check
All committers have signed the CLA.
👋 Hello hurricanefrog, we appreciate your contribution to this project! 📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more. 🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project. Click to see more instructions ...
Review and merge process you can expect ...
|
8cf7d81 to
aa8c053
Compare
hurricanefrog
commented
Oct 24, 2025
Should I really lowercase the WebServer component to fix(webserver) in the commit message?
me-no-dev
commented
Oct 24, 2025
Should I really lowercase the WebServer component to fix(webserver) in the commit message?
It's not a big deal, but it's nice to have consistency. Will not influence wether this gets merged or or not :)
this makes it possible to create derived classes properly.
aa8c053 to
c80e972
Compare
me-no-dev
commented
Oct 24, 2025
I haven't done any tests. I expect no impact on the function of the code. I haven't been able to create a HTTPS server with the changes yet, to confirm it leads to my goal, this takes some additional time.
Lets keep this open then (I'll convert it to Draft) until you can test it, in case other changes also need to be added. Please let us know when you have done that :)
Test Results
76 files 76 suites 15m 57s ⏱️
38 tests 38 ✅ 0 💤 0 ❌
241 runs 241 ✅ 0 💤 0 ❌
Results for commit c80e972.
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.
Click to expand the detailed deltas report [usage change in BYTES]
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This makes it possible to create derived classes properly.
Description of Change
This change replaces all uses of
_currentClientin theWebServerclass by calling the virtual methodclient().(Except in
handleClient, which makes to sense to touch, I think).This makes it possible to create derived classes, e.g to create TLS-protected web servers.
This was not possible previously due to the hardcoded "plain"
_currentClient.Derived classes can now simply override
client(), e.g. to return the TLS protected socket, and continue using the rest of the class (i.e. all of the HTTP parsing).Test Scenarios
I haven't done any tests. I expect no impact on the function of the code. I haven't been able to create a HTTPS server with the changes yet, to confirm it leads to my goal, this takes some additional time.
Related links
closes #11944