4 ms·
A largely incomplete list. One of the most important items would be: handle errors correctly and make sure errors do not result into any sort of resource leak o
by partycoder 9y ago
A largely incomplete list. One of the most important items would be: handle errors correctly and make sure errors do not result into any sort of resource leak or sensitive information disclosure.
For example, this code was from a guy in Stack Overflow:
... function (req, res, next) {
if (err) return console.log(err)
What will that piece of code do? Leak the request object, the response object, including the underlying client connection... keep the connection open, leak memory... and at scale, make your server run out of sockets and memory. Then, when memory is low, swapping kicks in, overloading your CPU as well. In short, kills your machine with only a little 1 line of code mistake.
There are less trivial ways of running into the same situation, but the lesson is that node.js is not a babyproofed technology and needs to be used with care.
This piece of advice is one of the most important, and the most overlooked/ignored in the node community.
- Kiro 9y ago> make your server run out of sockets and memory Do you mind explaining why the code causes this?
- partycoder 9y agoBecause when you handle a request, you need either respond to it or close it. If you don't close it (e.g: res.end, res.send, ...) the connection remains open. Also all the memory allocated for that request stays around without being collected.
- aeronautic 9y agoAgree with the point, but remember normal HTTP/1.1 /2 keep alive should close that connection in 15-30 seconds for all modern browsers. It will give rise to a DOS vulnerability as you point out.
- partycoder 9y agoBut all the objects remain leaked even if the request connection ends. And if the request stream got piped into another stream the problem can become more severe. Usually error events propagate through piped streams, but that largely depends on the order in which the event handlers were defined.
- aeronautic 9y agoAre you sure the objects remain leaked when a connection disconnect comes through? I don't have hard data on that. Depending on the node web framework (express, ...) they may handle that differently and may have timeouts to cleanup. I'd love to get a firm answer on this one.
- partycoder 9y agoYes, I am certain of that. I've seen it in production environments and verified it by looking at heap dumps.
- brlewis 9y agoI'd love a firm answer too, but I'm too lazy to write up code that puts request objects in a WeakSet and checks for when the set gets smaller. I hope someone else does it. :-)
- aeronautic 9y agoAh, node. That is a whole post for itself and a really good idea. Perhaps I'll tackle that next. Coupled with async wait issues -- there is plenty to talk about there. But good point. How would you describe this item on error handling? How would you state it for the check list?
- partycoder 9y agoMake sure error conditions are handled gracefully, without leaking resources, and producing errors that do not contain any sensitive information.
- aeronautic 9y agoThanks perfect.
- slackingoff2017 9y agoWell, I've avoided the node hype because of people warning about bad debugging experience. With that, I'm going to try to avoid touching it ever. I've used many web stacks over the years and not a single one defaults to blowing up like that. They all follow proper modularization... The framework cleans up objects that the framework creates, you deal with yours. In any other framework forgetting to end the request would result in the request automatically ending when your code finishes. This includes Vert.x and Undertow which are just as asynchronous as Node(but also multithreaded). Also how the hell does that leak memory? Is JS reference counting that bad? I've done some really stupid things in Java and C# but never leaked memory enough that it mattered.
- partycoder 9y agoIt's not a problem with the garbage collector, just how the objects are referenced. Basically you need to ensure the request reaches a terminal state where you either close the connection (ServerResponse.end) or send a response (ServerResponse.send and similar). All the cleanup happens after you do that.
- slackingoff2017 9y agoIs there not an OnDispose or some kind of hook to detect when requests can end? To end a request in every framework I've used you just return from your code back to the framework. You shouldn't have to worry about a framework object like request state.
- partycoder 9y agoThere are ways to detect this, but since express declares itself as an "minimalistic, non-opinionated framework" it's up to you to do it and do it correctly, or have correctly implemented logic that does not run into this issue.