Here's my code for a consise node.js HTTP server. It does work as intended, but I'd like to ask if there's any glitch or to be improved.
var httpServer = function(dir)
{
var component = require('http')
.createServer(function(req, res)
{
var fs = require('fs');
var path = require("path");
var url = require('url');
var mimeTypes = {
"html": "text/html",
"jpeg": "image/jpeg",
"jpg": "image/jpeg",
"png": "image/png",
"js": "text/javascript",
"css": "text/css"
};
var uri = url.parse(req.url)
.pathname;
var filename = path.join(dir, unescape(uri));
var indexFilename = path.join(dir, unescape('index.html'));
var stats;
console.log(filename);
try
{
stats = fs.lstatSync(filename); // throws if path doesn't exist
}
catch (e)
{
res.writeHead(404,
{
'Content-Type': 'text/plain'
});
res.write('404 Not Found\n');
res.end();
return;
}
if (stats.isFile())
{
// path exists, is a file
var mimeType = mimeTypes[path.extname(filename)
.split(".")[1]];
res.writeHead(200,
{
'Content-Type': mimeType
});
var fileStream =
fs.createReadStream(filename)
.pipe(res);
}
else if (stats.isDirectory())
{
// path exists, is a directory
res.writeHead(200,
{
'Content-Type': "text/html"
});
var fileStream =
fs.createReadStream(indexFilename)
.pipe(res);
}
else
{
// Symbolic link, other?
// TODO: follow symlinks? security?
res.writeHead(500,
{
'Content-Type': 'text/plain'
});
res.write('500 Internal server error\n');
res.end();
}
});
return component;
};
var port = 9999;
var dir = 'www';
var HTTPserver =
httpServer(require('path')
.join(__dirname, dir))
.listen(port, function()
{
console.log('HTTP listening ' + port);
});
3 Answers 3
You should move all require(...)
calls to the very beginning of your code, instead of running require
on each place locally.
In its current form, you won't see require
errors until your server runs and receives its first requests. This is unfortunate. You usually want a program to fail as early as possible if it clearly won't work.
Rule of Repair (TAOUP): Repair what you can — but when you must fail, fail noisily and as soon as possible.
I modified as follows, in addition to ensure require error possibility checking, structurally better I guess. Plus, I modified to full async where it should be.
var httpServer = function(dir)
{
var fs = require('fs');
var path = require("path");
var url = require('url');
var mimeTypes = {
"html": "text/html",
"js": "text/javascript",
"css": "text/css",
"jpeg": "image/jpeg",
"jpg": "image/jpeg",
"png": "image/png",
"gif": "image/gif",
"svg": "image/svg"
// more
};
var component = require('http')
.createServer(function(req, res)
{
var uri = url.parse(req.url).pathname;
var filepath = path.join(dir, unescape(uri));
var indexfilepath = path.join(dir, unescape('index.html'));
console.info('filepath', filepath);
var f = function(err, stats)
{
if (stats === undefined) // path does not exit 404
{
res.writeHead(404,
{
'Content-Type': 'text/plain'
});
res.write('404 Not Found\n');
res.end();
return;
}
else if (stats.isFile()) // path exists, is a file
{
var mimeType = mimeTypes[path.extname(filepath).split(".")[1]];
res
.writeHead(200,
{
'Content-Type': mimeType
});
var fileStream =
fs
.createReadStream(filepath)
.pipe(res);
return;
}
else if (stats.isDirectory()) // path exists, is a directory
{
res
.writeHead(200,
{
'Content-Type': "text/html"
});
var fileStream =
fs
.createReadStream(indexfilepath)
.pipe(res);
return;
}
else
{
// Symbolic link, other?
// TODO: follow symlinks? security?
res
.writeHead(500,
{
'Content-Type': 'text/plain'
})
.write('500 Internal server error\n')
.end();
return;
}
};
fs.stat(filepath, f);
});
return component;
};
var port = 9999;
var dir = 'www';
var HTTPserver =
httpServer(require('path')
.join(__dirname, dir))
.listen(port, function()
{
console.info('HTTP listening', port);
});
-
\$\begingroup\$ What exactly were you simplifying? Could you explain rather than just showing? A whole new version of the code won't create much learning effect, especially since there is no easy diff mechanism here. \$\endgroup\$vog– vog2015年01月29日 13:10:47 +00:00Commented Jan 29, 2015 at 13:10
simpler
var port = 9999;
var directory = 'www';
var http = require('http');
var url = require('url');
var path = require("path");
var fs = require('fs');
var mimeTypes = {
"html": "text/html",
"js": "text/javascript",
"css": "text/css",
"jpeg": "image/jpeg",
"jpg": "image/jpeg",
"png": "image/png",
"gif": "image/gif",
"svg": "image/svg"
// more
};
var request = function(req, res)
{
var uri = url.parse(req.url).pathname;
var dir = path.join(__dirname, directory);
var filepath = path.join(dir, unescape(uri));
var indexfilepath = path.join(dir, unescape('index.html'));
console.info('filepath', filepath);
var f = function(err, stats)
{
if (stats === undefined) // path does not exit 404
{
res.writeHead(404,
{
'Content-Type': 'text/plain'
});
res.write('404 Not Found\n');
res.end();
return;
}
else if (stats.isFile()) // path exists, is a file
{
var mimeType = mimeTypes[path.extname(filepath).split(".")[1]];
res
.writeHead(200,
{
'Content-Type': mimeType
});
var fileStream =
fs
.createReadStream(filepath)
.pipe(res);
return;
}
else if (stats.isDirectory()) // path exists, is a directory
{
res
.writeHead(200,
{
'Content-Type': "text/html"
});
var fileStream =
fs
.createReadStream(indexfilepath)
.pipe(res);
return;
}
else
{
// Symbolic link, other?
// TODO: follow symlinks? security?
res
.writeHead(500,
{
'Content-Type': 'text/plain'
})
.write('500 Internal server error\n')
.end();
return;
}
};
fs.stat(filepath, f);
return;
};
var serverUp = function()
{
console.info('HTTP server listening', port);
return;
};
var component = http
.createServer(request)
.listen(port, serverUp);
-
\$\begingroup\$ I propose to edit your other anser, instead of creating a new answer for each code improvement. \$\endgroup\$vog– vog2015年01月29日 12:10:09 +00:00Commented Jan 29, 2015 at 12:10