add regex support for websocket controller #1777 #1778 #1779

TYUTthelastone · 2023-09-15T10:04:02Z

刚用github，不太会提代码，点半天不知道在点啥，最后只能删了重来...💀

WS_PATH_LIST_BEGIN
    WS_PATH_ADD("/chat", Get);
    WS_PATH_ADD_REGEX("/[^/]*", Get, Post, Options);
WS_PATH_LIST_END

add additional regular regex strings support for the WebSocket controller.

an-tao · 2023-09-15T10:59:56Z

drogon/lib/tests/integration_test/server/WebSocketTest.h

Line 20 in cedeeb5

WS_PATH_ADD("/chat", "drogon::LocalHostFilter", Get);

请在服务端和客户端添加适当的测试代码，谢谢~

ken-matsui · 2023-09-15T22:02:48Z

@an-tao Sorry. I have been so busy that I cannot review this PR. Can you assign another reviewer?

an-tao · 2023-09-16T02:21:04Z

@an-tao Sorry. I have been so busy that I cannot review this PR. Can you assign another reviewer?

Okey, thanks.

Mis1eader-dev · 2023-09-19T07:42:45Z

Causes a segmentation fault when matches the regex:

let ws = new WebSocket("ws://127.0.0.1:8848/chat"); // fine
let ws = new WebSocket("ws://127.0.0.1:8848/chatting"); // segfault
let ws = new WebSocket("ws://127.0.0.1:8848"); // segfault

Mis1eader-dev · 2023-09-19T07:49:51Z

One thing to note, according to the WebSocket specification websockets should accept websocket requests using GET method only, I suggest we remove support for allowing methods to be passed in to WS_PATH_ADD and WS_PATH_ADD_REGEX, because in one of the previous commits any websocket request that is not a GET request has been disallowed.

Mis1eader-dev · 2023-09-20T04:40:28Z

lib/src/WebsocketControllersRouter.cc

@@ -184,16 +244,35 @@ void WebsocketControllersRouter::route(
 std::string wsKey = req->getHeaderBy("sec-websocket-key");
 if (!wsKey.empty())
 {
+ std::smatch result;


Prefer putting this in its respective scope, within if (iter == wsCtrlMap_.end()) block just before the for loop

ken-matsui · 2024-01-31T14:25:40Z

examples/websocket_client/WebSocketClient.cc

+ std::string messageType = "Unknown";
+ if (type == WebSocketMessageType::Text)
+ messageType = "text";
+ else if (type == WebSocketMessageType::Pong)
+ messageType = "pong";
+ else if (type == WebSocketMessageType::Ping)
+ messageType = "ping";
+ else if (type == WebSocketMessageType::Binary)
+ messageType = "binary";
+ else if (type == WebSocketMessageType::Close)
+ messageType = "Close";


I think we can use std::string_view instead to avoid heap allocation.

Mis1eader-dev · 2024-03-29T08:07:05Z

Seems we have diverged in files, resolving this merge conflict requires moving functions around

Mis1eader-dev · 2024-05-30T15:18:26Z

I have now come to a point where I also need this feature, any chance for us to revive this PR?

TYUTthelastone · 2024-05-31T00:58:58Z

I have now come to a point where I also need this feature, any chance for us to revive this PR?

I notice that this code has been removed in the last version, I'll find time to push my code again later.

an-tao · 2024-05-31T02:49:11Z

@TYUTthelastone Thanks so much. please pay attention to the questions I reminded in this PR.

Mis1eader-dev · 2024-06-01T05:44:33Z

This needs a rebase, it has pulled many other commits with itself.

Mis1eader-dev · 2024-06-01T17:37:50Z

I tested it out, throws a segfault @TYUTthelastone, can you please check if it works properly?

20240601 17:36:41.431205 UTC 8794 TRACE [run] Start to run... - HttpAppFrameworkImpl.cc:521
20240601 17:36:41.433687 UTC 8794 TRACE [createListeners] thread num=1 - ListenerManager.cc:84
20240601 17:36:41.433923 UTC 8794 TRACE [createNonblockingSocketOrDie] sock=11 - Socket.h:46
20240601 17:36:41.433984 UTC 8794 TRACE [IgnoreSigPipe] Ignore SIGPIPE - TcpServer.h:298
20240601 17:36:41.433997 UTC 8794 TRACE [~TcpServer] TcpServer::~TcpServer [drogonPortTest] destructing - TcpServer.cc:47
20240601 17:36:41.434042 UTC 8794 TRACE [~Socket] Socket deconstructed:11 - Socket.cc:245
20240601 17:36:41.434093 UTC 8794 TRACE [createNonblockingSocketOrDie] sock=10 - Socket.h:46
20240601 17:36:41.434130 UTC 8794 TRACE [IgnoreSigPipe] Ignore SIGPIPE - TcpServer.h:298
20240601 17:36:41.434647 UTC 8794 TRACE [start] HttpServer[drogon] starts listening on 0.0.0.0:8080 - HttpServer.cc:88
20240601 17:36:41.434869 UTC 8796 TRACE [operator()] map size=1 - TcpServer.cc:131
20240601 17:36:44.046592 UTC 8796 TRACE [newConnection] new connection:fd=11 address=127.0.0.1:45580 - TcpServer.cc:62
20240601 17:36:44.046656 UTC 8796 TRACE [TcpConnectionImpl] new connection:127.0.0.1:45580->127.0.0.1:8080 - TcpConnectionImpl.cc:78
20240601 17:36:44.046702 UTC 8796 TRACE [operator()] connectEstablished - TcpConnectionImpl.cc:262
20240601 17:36:44.046776 UTC 8796 TRACE [addHeader] cookies!!!:null; - HttpRequestImpl.cc:424
20240601 17:36:44.046822 UTC 8796 TRACE [isWebSocket] new websocket request - HttpServer.cc:1045

Mis1eader-dev

@an-tao looks good to me and works properly.

Mis1eader-dev · 2024-06-06T12:44:26Z

examples/websocket_server/WebSocketServer.cc

@@ -14,6 +14,7 @@ class WebSocketChat : public drogon::WebSocketController<WebSocketChat>
 const WebSocketConnectionPtr &) override;
 WS_PATH_LIST_BEGIN
 WS_PATH_ADD("/chat", Get);
+ WS_ADD_PATH_VIA_REGEX("/[^/]*", Get);


@an-tao the naming feels strange on the macro, I say we make it have the same prefix as the original and add "_VIA_REGEX" to it:

WS_PATH_ADD_VIA_REGEX

What do you say?

What about WS_PATH_REGEX_ADD? This corresponds to the style of WS_PATH_LIST_BEGIN as well, i.e., WS_ + Noun + Verb.

I use the same format as the http using

Damn, and making naming changes to macros will break compatibility.

We can stick to one standard naming convention, and keep the old ones for compatibility purposes.

My suggestion is to use scoped naming convention:

PATH_ADD PATH_ADD_VIA_REGEX METHOD_ADD METHOD_ADD_TO METHOD_ADD_VIA_REGEX WS_ADD WS_ADD_VIA_REGEX

TYUTthelastone changed the title ~~add regex support for websocket controller #1777~~ add regex support for websocket controller #1777 #1778 Sep 15, 2023

an-tao requested a review from ken-matsui September 15, 2023 10:50

ken-matsui removed their request for review September 15, 2023 22:02

an-tao requested review from Greisby and Mis1eader-dev September 16, 2023 02:21

Mis1eader-dev reviewed Sep 20, 2023

View reviewed changes

ken-matsui reviewed Jan 31, 2024

View reviewed changes

TYUTthelastone force-pushed the regex branch from ebb5874 to f37a1d0 Compare May 31, 2024 12:02

add regExp for websocket

e3cbfd7

TYUTthelastone changed the base branch from cpp14 to master June 1, 2024 07:31

TYUTthelastone added 2 commits June 2, 2024 17:13

fix a ws route error, add websocket server regex path example

ddeeac2

format

a3cafeb

an-tao requested review from Mis1eader-dev and ken-matsui June 3, 2024 03:14

Mis1eader-dev approved these changes Jun 3, 2024

View reviewed changes

an-tao merged commit 9a96a20 into drogonframework:master Jun 4, 2024
34 checks passed

Mis1eader-dev reviewed Jun 6, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add regex support for websocket controller #1777 #1778 #1779

add regex support for websocket controller #1777 #1778 #1779

TYUTthelastone commented Sep 15, 2023

an-tao commented Sep 15, 2023

ken-matsui commented Sep 15, 2023

an-tao commented Sep 16, 2023

Mis1eader-dev commented Sep 19, 2023

Mis1eader-dev commented Sep 19, 2023

Mis1eader-dev Sep 20, 2023

ken-matsui Jan 31, 2024

Mis1eader-dev commented Mar 29, 2024

Mis1eader-dev commented May 30, 2024

TYUTthelastone commented May 31, 2024

an-tao commented May 31, 2024

Mis1eader-dev commented Jun 1, 2024

Mis1eader-dev commented Jun 1, 2024

Mis1eader-dev left a comment

Mis1eader-dev Jun 6, 2024

ken-matsui Jun 6, 2024

TYUTthelastone Jun 6, 2024

Mis1eader-dev Jun 6, 2024 •

edited

add regex support for websocket controller #1777 #1778 #1779

add regex support for websocket controller #1777 #1778 #1779

Conversation

TYUTthelastone commented Sep 15, 2023

an-tao commented Sep 15, 2023

ken-matsui commented Sep 15, 2023

an-tao commented Sep 16, 2023

Mis1eader-dev commented Sep 19, 2023

Mis1eader-dev commented Sep 19, 2023

Mis1eader-dev Sep 20, 2023

Choose a reason for hiding this comment

ken-matsui Jan 31, 2024

Choose a reason for hiding this comment

Mis1eader-dev commented Mar 29, 2024

Mis1eader-dev commented May 30, 2024

TYUTthelastone commented May 31, 2024

an-tao commented May 31, 2024

Mis1eader-dev commented Jun 1, 2024

Mis1eader-dev commented Jun 1, 2024

Mis1eader-dev left a comment

Choose a reason for hiding this comment

Mis1eader-dev Jun 6, 2024

Choose a reason for hiding this comment

ken-matsui Jun 6, 2024

Choose a reason for hiding this comment

TYUTthelastone Jun 6, 2024

Choose a reason for hiding this comment

Mis1eader-dev Jun 6, 2024 • edited

Choose a reason for hiding this comment

Mis1eader-dev Jun 6, 2024 •

edited