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

Submitted by:  Sakari Kapanen <flannelhead>
Submitted on:  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 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>
Project 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>
Project 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>
Project 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>
Project 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>
Project Administrator
Mon 29 Jan 2018 05:39:39 AM UTC, comment #3: 

Simon is the httpd guy :-)

Dirk Ziegelmeier <dziegel>
Project 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>
Project 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>

 

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 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)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 1 encouragement so far.

    Only logged-in users can vote.

     

     

     

    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.5