-
Notifications
You must be signed in to change notification settings - Fork 135
API-compatibility with WebServer.h? #64
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
Comments
I'm not a heavy user of WebServer.h, but, to me, the core of the APIs seems to be similar within a certain degree: You have the server instance and you can define handler functions. The main difference with this implementation is that the server can handle requests in parallel, so it passes request and response objects to the handler function. This is somewhat inspired by the way it is done in JS frameworks like Express (which also holds for things like middleware etc.). However, this prevents a direct mapping from WebServer.h code to this server, as reading requests and writing responses has to be done on the request-specific objects, not on the webserver itself. And this lib is a bit more oriented in the direction of the C++ standard library (usage of std::string etc.) than to some Arduino classes. I think it could be possible to write a thin wrapper around the library, that does the aforementioned mapping of types and function calls. But I would put that wrapper code into a second library, so that your code can depend on that library, which again depends on the one in this repo. That way, one could switch between WebServer.h and the wrapper transparently, while the more powerful API of this library is still accessible to all users needing it.
Correct. The main goal when I started this project was to have an HTTPS server on one of Espressif's chips, and the ESP32 has the advantage of having hardware support for AES, SHA and multiplication of big numbers, which is handy for the required asymmetric crypto (RSA, DH). In fact, in the beginning the server was HTTPS-only, and as the TLS-support for the ESP8266 was still quite experimental and the performance isn't very well on that chip either, I didn't consider it as target platform. So the TLS part is what makes this library specific to the ESP32. If you throw out everything that has "HTTPS" or "SSL" in its filename, the plain HTTP part should be working or need only very few modifications. At least, if the lwip network stack does not differ too much between both platforms. And having a compile-time switch to disable HTTPS completely has been on my list for some time now anyway. So if one would want to proceed in that direction, there are two main tasks: ESP8266 compatibility and wrapping the API. To achieve compatibility, I'd first add that HTTP-only flag, then get HTTP running on the 8266, and finally create the platform-specific code to deal with TLS on the 8266. The wrapper project could be started directly and should work on the ESP8266 once the compatibility has been reached in the main code base. What do you think of that approach? |
To make it a bit more precise, that wrapper layer could look like this. The hello server example is working on the ESP32 with HTTP (for now without parsing the args on the 404 page). When finished, that setup allows exposing the underlying instance of this web server while providing API compatibility with WebServer.h. So one could easily migrate to this server, and then make use of its additional functionality. |
Frank, thanks a lot for the clarification! I'll think about it for a while, but it may be better to do the compatibility the other way around (i.e. build a wrapper around |
I think the other way round (Having the esp32_https_server API but using Webserver.h to manage the traffic) will be hardly possible, at least not without some limitations. Creating the request and response objects could be tricky without a connection context. From what I've seen when creating the other repo, it should be possible to achieve nearly complete compatibility to the Webserver.h API with an underlying esp32_https_server. By adding functions like If the server implementation should remain interchangable, these additional functions must not be used. |
Good point. I'm making some progress (I forked your esp32_https_server_compat repo), enough progress that I haven't given up yet:-), but I'm running into the first issue, and that is that the compat wrapper needs access to things that aren't exported by esp32_https_server. Specifically, the I guess this means I have to fork esp32_https_server too, unless you have an idea about how I could work around this (and similar issues that will come up, undoubtedly) without forking... |
Yes, #65 would solve the issue at hand. I'll wait for that to be merged. In the mean time I'll continue with the other functionality that I'll need (such as upload, which I'm really looking forward to:-). |
Turns out I also need to iterate over the headers (in the request). So can I ask for either (a) a similar interface to #65 for iterating over the headers, or (b) a method |
I decided on the latter. I've created pull request #66 which implements this. |
And for the sake of completeness of inter-issue references: I also need #29 in some form to implement WebServer upload handlers. |
While the latter is easier, I think exposing an iterator interface would be a more consistent way of providing access to the headers. Also, the
There is the highly outdated bodyparser branch, which you might already have found through the PR. In case of file uploads, only multipart bodies have to be considered, so we could focus on that first and do not need to add url-encoded forms directly. However, I think maybe the code in that branch could also be changed to provide an interator-like interface, for sake of consistency. I think parsing the body elements can only be done iteratively to avoid storing the whole request body in memory. So a random access method like |
Hmm, you're right. But practically speaking I presume people will understand that the |
Got back to this after a good month of being busy with other stuff. I've come across various issues with form handling, and filed pull request #69 to handle one of them. I'm now going to have a look at the bodyparser branch, because that's not only needed for file uploads but also for forms with |
If you need any support with the compatibility layer, just let me know. I just didn't want to edit my repo because of the amount of commits that your fork was ahead already. |
The compatibility layer is coming along fine. I've given you access to my fork of the repo, just in case. I think the last bit of work is really getting the bodyparser branch to work, possibly after thinking a bit about the API. It's just slow going because I'm working on far too many projects, and this one isn't really in the critical path for any work-related ones. |
One last issue: is there a way I can easily get multiple path components when I create a |
That sounds great, I just feel a bit reserved about merging things back as you've clearly done most of the work.
At the moment there is none, maybe something like ending a path with |
I thought about that, but my feeling is that you're better suited as the primary maintainer than me, because future changes will probably be mostly due to changes in esp32_https_server. And I don't need the GitHub geek points anyway given my history:-). But: I'm not trying to push work over the fence, so I'd be perfectly happy to (co)maintain.
I was thinking of something along those lines. I didn't bother yet (unsure how much of an issue it really is, for the compatibility layer) but I could give it a try and create a patch. Let me know. |
@fhessel shall we try and close this one? The main issue is deciding whether you want to host esp32_https_server_compat (possibly with me as a co-maintainer, if you want), or if you would prefer not to and then I'll host it (with the caveats given above). I'd like to make some progress with this, because I'm getting ready for a new release of https://github.com/cwi-dis/iotsa but I'd really like the dependencies like esp32_https_server_compat to be in their final resting place with the platformio and arduino library URLs pointing to the right location before I do so... |
Sorry for the lack of feedback from my side. Then let's do this. I will need a moment to check the code in the compat repository, but I'll start now. If you want to have a fully working build process and resolve dependencies by version number, that means that I need to draft a release for this repo as well, right? Currently that would be a major release as the API to request and path parameters has changed. That will take a moment but as there are a lot of pending changes which aren't released yet, 1.0.0 is overdue anyway. From then on I would keep the version of both repos in sync, with an exception to the patch level, and use So what I'll do then:
Once PlatformIO did pick it up, you can esp32_https_server_compat=1.0.0 as dependency without referring to the repository and you'll get a stable build. I can maintain both repos, but I'll only be able to keep the compat layer up to this lib from a functional perspective. Adding the actions from this repo to build at least the examples for every push will help by providing a basic sanity check. |
The new library has appeared in the platformio registry. I think we can close this one. Thanks a million! |
Thanks to you too for all the work on getting the body parsing done and implementing the compatibility layer! I'll close this for now and keep an eye on the Arduino registry. |
Is there a chance of getting API-compatibility with
WebServer.h
?Or, alternatively, could you guide me in the direction of what would be needed to do that (so I could do it myself and share the code, if you're interested in accepting it)?
I think your API is probably superior to the
WebServer.h
API, but it has the disadvantage that it's only available for ESP32 (correct?).I maintain a framework that runs both on ESP32 and ESP8266, and it can be built for either http or https support, and for 3 out of the 4 combinations I can use the
WebServer.h
API (or the very similarESP8266WebServer.h
orESP8266WebServerSecure.h
APIs).The text was updated successfully, but these errors were encountered: