fhessel / esp32_https_server

Alternative ESP32 Webserver implementation for the ESP32 Arduino Core, supporting HTTPS and HTTP.
MIT License
341 stars 124 forks source link

Web Socket Support #13

Open fhessel opened 6 years ago

fhessel commented 6 years ago

The server should support web sockets (see #9 for reference).

squonk11 commented 6 years ago

So if you wanted to enable websockets, you would need to check the header values in state HEADERS_FINISHED, and check if the client requested an Upgrade: websocket.

I think it would make more sense if I check for a websocket connection in the end of the state: STATE_REQUEST_FINISHED, right?

squonk11 commented 6 years ago

After thinking again about the right approach I think it would be correct if I check in the end of the state "STATE_REQUEST_FINISHED" for the prerequisites of a websocket and set an internal flag (e.g.: HttpsConnection._isWebsocket). In the state "HEADERS_FINISHED" I can invoke the callback of the matching node and do activate the websocket handler. For the websocket handler I was thinking about using the second core of the ESP32 because I am planning to have lots of fast traffic on the websocket connection. Does this approach make sense to you?

fhessel commented 6 years ago

Yes your are right. By "in state HEADERS_FINISHED" I meant to do it at the beginning of that state. Checking for the websocket handshake at the end of STATE_REQUEST_FINISHED should work as well. According to the RFC handshakes should be bodyless GET-requests, so we need no classic body parsing there anyway. If HttpsConnection._connectionState is set to STATE_WEBSOCKET, the additional flag for websockets should be redundant as well.

Regarding the handling on the second core: I would prefer to let the user of the library decide on where to handle the traffic, as the separation might not be ideal for every user and not every ESP32 module supports two cores (as far as I know). Do you have something in mind how to achieve it?

squonk11 commented 6 years ago

Until now I did not deeply think about how to implement it. But as you suggested I will identify the websocket initiation in the state "STATE_REQUEST_FINISHED" and then set the state "STATE_WEBSOCKET". In the state "STATE_WEBSOCKET" I will do the websocket initiation handshake and then launch a separate FreeRTOS task (using "xTaskCreatePinnedToCore") which reads from and writes to the created websocket. Here I need to take care about the websocket frame and I will have to handle some opcodes like close, ping, pong, continue and text (or binary). Until now I have never done this before but I would like to try if I can succeed. Do you see somewhere any specific pitfalls I should take care of? Do you have specific requirements concerning the coding?

fhessel commented 6 years ago

I think that's a feasible way to implement the web sockets initially, but on long term I would make the task separation optional or let the library user do it, as new tasks means to reserve more memory for the task's stack etc. – I think that depending on the use case, waiting a bit longer for a response but having more common memory available might be the better side of this trade-off. However, if you'd like to work on that feature and that's the best option for your scenario, go for it. I might generalize that later on.

At the moment I don't have any coding requirements, especially I did not use any coding style guide etc. for myself by now and I doubt that there would be one matching the existing code... I can't think of any specific pitfalls by now. If you put every handler function in a separate task, you should not run into problems like #3 which would affect every handler function of the server. However, running in a different task means that you need to be careful when accessing server's resources from the handler task. Pointers may point to the stack of the server's task and so they can by invalid, depending on the server's state.

squonk11 commented 6 years ago

I think you are right - my use case is quite special and the https server library should be more general. So, I will first implement it using your loop functionality. Maybe I can find a soulution so that the user is able to plugin or launch his own handler function in a separate task on a selectable core.

fhessel commented 6 years ago

I'm currently implementing support for middleware functions (see #10) because I need them for one project I'm working on. The way I do it is to use the functional part of the standard library. I still need to check the impact of using it on the size of the binary, but if everything turns out well that might also be an option to achieve variation of the handler function. One could define a default behavior but allow the user to override it by defining his own way of handling websockets, eg. creating the new task. That would allow for most flexibility, if the impact on performance is not too big.

squonk11 commented 6 years ago

I am now starting to implement the websocket functionality but I am already stuck at the very beginning: when I start a websocket connection from the browser e.g. using var ws = new WebSocket("wss://192.168.137.139/echo"); the size of the generated headers of the request is bigger than the first chunk of data read into _parserLine. The updateBuffer() function obviously does not read the remaining part of the headers and the loop() function gots stuck in the state "STATE_REQUEST_FINISHED". In updateBuffer() I see: if (FD_ISSET(_socket, &sockfds)) { // FIXME: Is SSL_pending(_ssl) > 0 required here? Maybe here is the issue? Unfortunately I am not familiar with FD_ISSET. Are you able to fix it?

fhessel commented 6 years ago

That was a part I changed for #17, because there are differences in the API for checking if there are input bytes remaining depending of the protocol you use (plain sockets vs. ssl sockets). I did not encounter any problem without that call, so I dropped it to make the code less complex. But I can integrate it again, so that you can try if that fixes your problem.

Currently I'm working on restructuring the repository so that it can directly be used as an Arduino library (mostly renaming directories and creating example sketches). If you wouldn't mind, I'd like to finish that first and then fix this bug.

squonk11 commented 6 years ago

I am not in a hurry. Just inform me when you are ready.

fhessel commented 6 years ago

I added the call to SSL_pending() again.

Furthermore, if you run into size limits while parsing the headers, have a look at HTTPSServerConstants.hpp, especially at the HTTPS_REQUEST_MAX... definitions. You can increase the amount of headers, the length of one header line and of the request string there. The main reason for those limits is to protect the server against malicious clients that try to consume all the server's memory by sending an infinite amount of headers. But the values that are defined right now are merely a guess on what could work.

squonk11 commented 6 years ago

Your correction concerning SSL_pending() works. Thanks for that. I did the first step of the websocket implementation: detect the websocket and do the handshake. The next step will be to implement a reader function which invokes a user function. But currently I have another problem with the wifi conneciton: The wifi connection gets lost some time after the last TCP connection has closed. In this case I need to reset the ESP32 in order to get a new wifi connection. I think this problem is more related to the Arduino wifi code rather than your webseerver code. Probably I need to set some timeout somewhere. Do you have a hint for me for that?

fhessel commented 6 years ago

The examples in the repository only include a very basic code for the wifi handling, to focus on the server itself. So the connection is made once, but not restored if it gets lost.

Basically you have something like this in the setup() function:

WiFi.begin(WIFI_SSID, WIFI_PSK);
while (WiFi.status() != WL_CONNECTED) {
  Serial.print(".");
  delay(500);
}

In addition to that, you would have to check for WiFi.status() != WL_CONNECTED in the loop() and then try to reconnect, like this:

if (WiFi.status() != WL_CONNECTED) {
  WiFi.disconnect();
  Serial.println("Reconnecting WiFi");
  WiFi.begin(WIFI_SSID, WIFI_PSK);
  while(WiFi.status() != WL_CONNECTED) {
    Serial.println(".");
    delay(100);
  }
}

I'm facing the same problem since recently, but as it occurs for other sketches as well and it correlates with a recent router update, I thought that the update was causing the problem.

squonk11 commented 6 years ago

Maybe this is a stupid question: in the constructor for ResourceNode you scan for URL parameters starting with "/*". According to my understanding URL parameters start with "?". Or am I mixing something, e.g. with queries?

fhessel commented 6 years ago

It's maybe a naming issue, as there's no official name for the URL wildcards. Within the library, the following convention should be used consistently: URL parameters name those wildcard parts of the URL (see this function in the parameters example), and the part after the question mark goes as request parameters (see this function in the parameters example).

squonk11 commented 6 years ago

Unfortunately I am currently running into a strange compilation error:

E:\msys32\opt\devel\Arduino\WebServer\libraries\HttpsServer\src/ResolvedResource.hpp:14:23: error: 'ResourceNode' has not been declared

Where ResourceNode.hpp and ResolvedResource.hpp are not modified by me - so, it should compile like normal. I think that maybe I am running into this problem: https://github.com/Sloeber/arduino-eclipse-plugin/issues/705 There seems to be an issue in Sloeber with a long path for the location of the arduinoPlugin. The mentioned workaround using path substitution (using 'subst') does not work for me (for whatever reason). So, I got stuck with the Sloeber Arduino IDE. Do you know which other IDE could make sense? maybe VSCode?

fhessel commented 6 years ago

If I understand the Sloeber issue correctly, it's caused by Windows file system limitations which makes it hard for me to reproduce, but I experienced similar problems with Eclipse in the past. The only other thing that comes to my mind regarding the ResourceNode class is the following: You could check if your ResourceNode.hpp contains the changes from the latest commit. without it, Sloeber might run into troubles as it sets -Werror=reorder and I had the member order wrong in the first commit. This would lead to ResourceNode not being compiled (you should see an additional error for that in the log, though) and so no being available for later inclusion. The Arduino IDE seems to treat that only as a warning, which made me overlook that problem at first.

VSCode works for me as an alternative IDE. Combining it with Platform IO might be a good idea for you as it supports both, Arduino builds and those for plain esp-idf and makes it easy to setup build paths etc. Issue #8 has some info on how to use the lib with Platform IO and ESP32/Arduino as target, most of the refactoring by Thorsten is already part of this repo, and you might use his library.json to complete it.

squonk11 commented 6 years ago

Now I synced again against your latest sources. The files ResourceNode.hpp and ResolvedResource.hpp are still the same as in your repository. But I still get the same error message:

In file included from E:\msys32\opt\devel\Arduino\WebServer\libraries\HttpsServer\src/ResourceResolver.hpp:12:0, from E:\msys32\opt\devel\Arduino\WebServer\libraries\HttpsServer\src/HTTPConnection.hpp:17, from E:\msys32\opt\devel\Arduino\WebServer\libraries\HttpsServer\src/Websocket.hpp:18, from E:\msys32\opt\devel\Arduino\WebServer\libraries\HttpsServer\src/ConnectionContext.hpp:9, from E:\msys32\opt\devel\Arduino\WebServer\libraries\HttpsServer\src/HTTPRequest.hpp:9, from E:\msys32\opt\devel\Arduino\WebServer\libraries\HttpsServer\src/HTTPSCallbackFunction.hpp:4, from E:\msys32\opt\devel\Arduino\WebServer\libraries\HttpsServer\src/ResourceNode.hpp:6, from E:\msys32\opt\devel\Arduino\WebServer\libraries\HttpsServer\src/HTTPServer.hpp:20, from E:\msys32\opt\devel\Arduino\WebServer\libraries\HttpsServer\src/HTTPSServer.hpp:15, from ..\WebServerArduino.cpp:23: E:\msys32\opt\devel\Arduino\WebServer\libraries\HttpsServer\src/ResolvedResource.hpp:14:23: error: 'ResourceNode' has not been declared void setMatchingNode(ResourceNode * node);

But now I see that in eclipse the contents of the file ResolvedResource.hpp has a grey background like when it is excluded from compile by an #ifnedf statement. All other .hpp files don't have a grey background. Do you have an explanation for that?

fhessel commented 6 years ago

I would expect that only if SRC_RESOLVEDRESOURCE_HPP_ has been defined somewhere else, but the only #define statement I was able to find was the one below that #ifndef SRC_RESOLVEDRESOURCE_HPP_ statement.

Did you run Project → Clean and deleted the Release folder to make sure everything is build from scratch?

Otherwise, hovering an #ifdef or #ifndef line should show you the corresponding #define statement that causes the code block to be included or not. Maybe that's a starting point to track down the error?

squonk11 commented 6 years ago

Running Project → Clean and delete the Release folder did not help. Now I removed all references to my Webserver class and compiled again - this works. Tomorrow I will successively insert the Webserver class again and check when and where the problem starts to appear...

squonk11 commented 6 years ago

I see that I get the error even with an almost empty Websocket.hpp file being included in the main application file. Could you do me a favour and try to compile my Websocket.hpp and Websocket.cpp into one of your example projects by including #include "Websocket.hpp" into your main application file? How can I send you the files? Attaching them as zip to this message is not possible.

fhessel commented 6 years ago

I can give it a try. If attaching the files does not work and they are too long to put them inline, you may send them to frank@fhessel.de.