6
\$\begingroup\$

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);
 });
200_success
146k22 gold badges190 silver badges479 bronze badges
asked Jan 28, 2015 at 10:33
\$\endgroup\$

3 Answers 3

3
\$\begingroup\$

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.

answered Jan 28, 2015 at 13:15
\$\endgroup\$
0
1
\$\begingroup\$

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);
 });
answered Jan 28, 2015 at 21:13
\$\endgroup\$
1
  • \$\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\$ Commented Jan 29, 2015 at 13:10
1
\$\begingroup\$

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);
answered Jan 29, 2015 at 0:20
\$\endgroup\$
1
  • \$\begingroup\$ I propose to edit your other anser, instead of creating a new answer for each code improvement. \$\endgroup\$ Commented Jan 29, 2015 at 12:10

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.