Kitura-NIO icon indicating copy to clipboard operation
Kitura-NIO copied to clipboard

Connection limiting doesn't do what it says.

Open Davidde94 opened this issue 5 years ago • 3 comments

If I create a HTTPServer with a connection limit of 2, then I would expect only 2 connections to be made. However, the current implementation allows essentially an unlimited number of connections, subject only to OS limits.

This appears to be because the connection limit logic occurs inside HTTPRequestHandler, an InboundChannelHandler. By definition, the connection must exist for the execution to reach this point. Correct me if I'm wrong, but what I think you're trying to achieve is to limit the number of concurrent requests, not literal connections?

The limiting logic also appears to be evaluated at the end of a HTTP request. This is a fairly major issue, as it allows me to dump massive requests before you try and stop me. At the very least it would be a good idea to move the logic to the very start of "channelRead" to shut the connection down before data is sent.

Hope that helps, me and the rest of the NIO are more than happy to help at any time :)

Davidde94 avatar Sep 15 '20 12:09 Davidde94

Thanks for your comments @Davidde94 ! Is this PR what you mentioned by moving it to the start of the logic? An additional question: is channelInactive called when I close the connection? Eg. do I need to decrease manually the counter everywhere context.close() is called or is it automatic?

Thanks.

mbarnach avatar Sep 28 '20 11:09 mbarnach

@mbarnach that's right! channelInactive will be called automatically

Davidde94 avatar Sep 30 '20 12:09 Davidde94

Thanks! Then, I think the PR should be OK.

mbarnach avatar Sep 30 '20 12:09 mbarnach