patchlwIP - A Lightweight TCP/IP stack - Patches: patch #9525, httpd: add Websocket support

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #9525: httpd: add Websocket support

Submitter:  Sakari Kapanen <flannelhead>
Submitted:  Thu 21 Dec 2017 09:12:19 PM UTC
Votes: 1
 
Category:  apps Priority:  5 - Normal
Status:  None Privacy:  Public
Assigned to:  None Open/Closed:  Open
Planned Release:  None

Jump to the original submission

Fri 06 May 2022 07:57:50 AM UTC, comment #17: 

Hello Savannah Community,

In the last 6 months we (a team of 6 students from the Baden-Wuerttemberg Cooperative State University) have been working on lwIP and on a successful implementation of Sakari Kapanen’s WebSocket patch under lwIP.

In the process we had to clear some obstacles, which we want to outline briefly at this point to help other developers who will probably have similar problems.

Our project goal was:
Implement the WebSocket patch for lwIP on Windows 10. From this task we derived the following subgoals:
• Deploy lwIP on Windows 10
• Rework the patch from Sakari Kapanen
• Publish the implementation on GitHub/Savannah.

lwIP on Windows 10
This subgoal represented our first hurdle. The installation guides available on the internet for lwIP on Windows are all outdated and not applicable to the 2.1.0 patch. More up-to-date installation guides exist mainly for lwIP under Linux or for specific embedded systems.

After a brief exchange with Simon Goldschmidt over email in early 2022, we were able to set up lwIP on Windows 10 with his input. As a result of this process, we created an installation guide that we are hosting on GitHub for all to see: Installation Guide lwIP on Windows 10.

The patch by Sakari Kapanen
In December 2017, Sakari Kapanen released a patch for lwIP that implements the WebSocket protocol: Patch #9525. This patch was not implemented in the main branch of lwIP because it contains several bugs (e.g. missing software licenses for third party code, memory leak).

Our project task was to customize the WebSocket patch and successfully implement it in the Main Branch of lwIP.
The WebSocket patch by Sakari Kapanen does not work in our Windows development environment due to that opening a websocket connection inevitably leads to a "Run-Time Check Failure #2". This could be because of missing dependencies in the Windows environment.

Release of the implementation
Due to the problems presented here, we decided to change the project goal as we wanted to make our small contribution to the lwIP project. Instead of implementing the WebSocket patch, we extended lwIP with an OpenAPI specification for REST APIs. lwIP sends back a JSON string containing the lwIP server’s key data when connecting to the lwIP server. This is displayed via the http web server interface in the endpoint identification.

We hope that this small insight into our work of the last 6 months could make a small contribution to the further development of lwIP.

Benjamin Esenwein <besenwein>
Wed 09 Mar 2022 10:16:28 AM UTC, comment #16: 

I tested the patch and I found a memory leak. http_state_init(hs) can be called with hs->ws_state pointing to an allocated struct and if hs is resetted, this struct is never freed.

This patch can fix the leak :


Date:   Wed Mar 9 10:45:32 2022 +0100

    WebSocket : Fix a memory leak (httpd.c)

diff --git a/src/apps/httpd/httpd.c b/src/apps/httpd/httpd.c
index 96434a3d..0eef09ff 100644
--- a/src/apps/httpd/httpd.c
+++ b/src/apps/httpd/httpd.c
@@ -734,6 +734,9 @@ http_eof(struct altcp_pcb *pcb, struct http_state *hs)
 #if LWIP_HTTPD_SUPPORT_WEBSOCKET
   if (hs->is_websocket) {
     http_state_eof(hs);
+    if (NULL != hs->ws_state) {
+      HTTP_FREE_WEBSOCKET_STATE(hs->ws_state);
+    }
     http_state_init(hs);
     hs->is_websocket = 1;
   } else


Jonathan D <jona>
Fri 06 Sep 2019 01:52:24 PM UTC, comment #15: 

Hi, my apologies for the absence of reply. I unfortunately have not had sufficient time to work on this and the situation is probably not going to change any time soon (even if I would have liked to). So, if anyone desires to continue improving upon this patch, please feel free to pick it up.

Personally, I've previously used this implementation on ESP8266, but I'm moving on to an RTOS SDK where it would be possible to utilize libwebsockets (https://libwebsockets.org/) which is a more advanced implementation and could serve future needs better.

Again, apologies for leaving this patch just floating here.

Sakari Kapanen <flannelhead>
Wed 09 Jan 2019 12:41:46 PM UTC, comment #14: 

Hi Sakari, any progress on the WebSocket support? Or, do you have any example full project that you can share? I am very interested in it.

Afdhal Atiff Tan <afdhalatifftan>
Mon 01 Oct 2018 01:16:24 PM UTC, comment #13: 

If you want something that resembles the raw tcp API, please create a new altcp layer.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 01 Oct 2018 01:14:57 PM UTC, comment #12: 

Should the websocket API not resemble the Raw TCP API? I mean stuff like:

ws_recv(ws_pcb *, ...)
ws_close(...)
ws_sent(...)

And a way to detect closing of a socket, like calling the recv callback with NULL pbuf.

I used the proposed API some time ago, and if I remember correctly it had a single, shared, receive callback for all open sockets. Also it gave no indication that a socket was closed for inactivity or remote request, making user's cleanup hard.

Lucas Hartmann <lhartmann>
Mon 01 Oct 2018 07:14:09 AM UTC, comment #11: 

Hello Sakari,

may I ask how the work is going with adding websockets? I would be interested in the outcome.

Adam Baron <vysocan>
Wed 28 Feb 2018 03:57:36 PM UTC, comment #10: 

Adjusted the summary as Dirk said he already pushed the strnicmp thing

Simon Goldschmidt <goldsimon>
Group administrator
Mon 29 Jan 2018 09:13:56 AM UTC, comment #9: 

Thanks, that seems like a fair assumption with good reasoning and indeed it makes maintenance easier.

Sakari Kapanen <flannelhead>
Mon 29 Jan 2018 09:12:14 AM UTC, comment #8: 

The best thing I can come up with is to just expect mbedtls is present. They provide everything we need and we don't need to implement things twice.

The code copy from PolarSSL was at a time where that library had moved to GPL and the copy we have was the last BSD version. With mbedtls having a compatible license, I don't thing it's good to stay with the code copy.

The downside is of course that we make non-secure things depend on mbedtls, but I'm willing to take that for not having to maintain more code :-)

Simon Goldschmidt <goldsimon>
Group administrator
Mon 29 Jan 2018 09:02:36 AM UTC, comment #7: 

Good to hear that. I'll follow up with a refactored patch and let's discuss it further then. :)

Still, could you please give a suggestion where should I relocate the base64 and SHA1 functions decoupled from SMTP and PPP, respectively?

Sakari Kapanen <flannelhead>
Mon 29 Jan 2018 08:39:47 AM UTC, comment #6: 


> With the mentioned changes in place, would you then more likely
> to consider this for merging?


I don't know that yet. With the mentioned changes, I'd be willing to take a look at it in depth. I'm not at all opposed to having web socket support in a separate file. And if the diff to httpd.c is really minimal, I guess this might go in, yes.

If that initial work is done, maybe we might be able to better separate the SSI code as well...

Simon Goldschmidt <goldsimon>
Group administrator
Mon 29 Jan 2018 08:33:09 AM UTC, comment #5: 

Simon, thank you for the feedback.

As for the SHA1 and base64 code, the origin is here:
https://github.com/B-Con/crypto-algorithms
They are released into the public domain and there's even no license file in the origin repo.

However, I agree with your point about avoiding duplication of the SHA1 and base64 functions. I had a look at the SMTP code, and the base64 function seems easy to split out of that code. What is, in your opinion, the best place in the code tree to locate the standalone base64 function?

About SHA1: I can change that to use PolarSSL/mbedTLS conditionally (I think you're referring to pppcrypt.h). In their current state, the PolarSSL files seem to be quite coupled to the PPP code, having includes of ppp_opts.h and conditional compilation based on whether PPP support is selected. I think they would have to be split out as well. What's your opinion about this? Where should the files be relocated?

Finally, I completely agree that httpd.c is huge, and I'll do my best to refactor the WebSocket support out of the main file. It will indeed be much more maintainable that way.

With the mentioned changes in place, would you then more likely to consider this for merging? Also, are there any other things I would have to consider? It seems you're moving more and more towards pool-based memory allocation, so it would seem preferable to account for this in the WebSocket implementation (like I've done to some extent already in the patch).

Sakari Kapanen <flannelhead>
Mon 29 Jan 2018 06:35:24 AM UTC, comment #4: 

I really don't know if this fits. Anyway, a few generic comments:
- don't add third party code without a license that fits. I can't even see a license at all for base64 and sha1
- when using sha1, use it like ppp does (polarssl/mbedtls)
- base64 encoding is used in smtp, too. We need to combine this instead of adding it a second time. Again, watch out for licensing issues here!

As to the actual web socket implementation: I won't add it like this to httpd.c. That file is too big and unreadable like it is now, adding yet more code would be bad. You might try to add a httpd_websockets.c file and come off with minimal changes to integrate this into httpd.c

Simon Goldschmidt <goldsimon>
Group administrator
Mon 29 Jan 2018 05:39:39 AM UTC, comment #3: 

Simon is the httpd guy :-)

Dirk Ziegelmeier <dziegel>
Group administrator
Sun 28 Jan 2018 10:01:29 PM UTC, comment #2: 

dziegel: thanks for merging the fix. Have you yet had time to review the patch? Would you consider this feature appropriate for merging to upstream httpd? If there is anything I could do to help this get merged, I'm willing to do so. :)

Sakari Kapanen <flannelhead>
Sun 24 Dec 2017 12:14:10 PM UTC, comment #1: 

Pushed the fix for strnicmp in a modified version without side-effect in boolean expression

Dirk Ziegelmeier <dziegel>
Group administrator
Thu 21 Dec 2017 09:12:19 PM UTC, original submission:  

This patch adds Websocket support to the httpd. The original work was done based on older lwIP by lujji: https://github.com/lujji/esp-httpd

I integrated his work into the current upstream httpd code. Broadcast support was also added in the process.

Also, this patch fixes the lwip_strnicmp implementation which I found to be faulty when I needed to implement strnistr to be used in the Websocket implementation.

Sakari Kapanen <flannelhead>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by besenwein (Posted a comment)
  • -email is unavailable- added by jona (Posted a comment)
  • -email is unavailable- added by afdhalatifftan (Posted a comment)
  • -email is unavailable- added by lhartmann (Posted a comment)
  • -email is unavailable- added by vysocan (Posted a comment)
  • -email is unavailable- added by vysocan (Voted in favor of this item)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by dziegel (Posted a comment)
  • -email is unavailable- added by flannelhead (Submitted the item)
  •  

    There is 1 vote so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-10-01 vysocan Carbon-Copy- Added vysocan
    2018-02-28 goldsimon Summaryhttpd: add Websocket support, fix lwip_strnicmp httpd: add Websocket support
    2018-02-03 goldsimon Assigned togoldsimon None
    2018-01-29 dziegel Assigned toNone goldsimon
    2017-12-21 flannelhead Attached File- Added 0001-httpd-integrate-Websocket-support.patch, #42700

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code