websockets / ws

Simple to use, blazing fast and thoroughly tested WebSocket client and server for Node.js
MIT License
21.34k stars 2.3k forks source link

terminate() doesnt terminate instantly #2209

Closed cenullum closed 3 months ago

cenullum commented 3 months ago

Is there an existing issue for this?

Description

I'm wanted to add rate limiter with https://www.npmjs.com/package/rate-limiter-flexible then terminate function seems doesnt terminate ws instantly then made my own rate limiter however same result. After limit request of client reached server still can receives request from client.

ws version

7.5.9

Node.js Version

v16.16.0

System

OS: Windows 10 10.0.19045
CPU: (8) x64 AMD Ryzen 5 3550H with Radeon Vega Mobile Gfx
Memory: 19.62 GB / 29.88 GB

Expected result

I expected to not receive another requests after 50 limit in the video. So I wanted dos2 result but got dos

Actual result

After terminate() I'm getting 300 more requests from client. I recorded a video and made a sample project

Attachments

resim when cpu isnt that much busy, invalid requests count increase while recording it was less when delaying request it instantly terminates (which is dos2 in the video) https://youtu.be/4f6QQEHaHB8 sample.zip

lpinca commented 3 months ago

websocket.terminate() destroys the underlying socket. If the there is buffered data, that data is read and then the 'close' event is emitted. Buffered data is not discarded by design.

cenullum commented 3 months ago

Is there a way to delete buffered data manually or can limit buffered data count or longer polling interval?

lpinca commented 3 months ago

Is there a way to delete buffered data manually or can limit buffered data count or longer polling interval?

No, the amount of buffered data is defined by https://nodejs.org/api/stream.html#readablereadablehighwatermark but you can't discard it.

cenullum commented 3 months ago

@lpinca Is there a way to artificially delay adding data to the buffer? If there is any other recommendation I'm listening

lpinca commented 3 months ago

No, that's how Node.js streams work. If you don't care about the the messages received after calling websocket.terminate(), you can remove the listener for the 'message' event.

lpinca commented 3 months ago

I'm closing this as answered.