5
\$\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);
      });
\$\endgroup\$
2
\$\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.

\$\endgroup\$
0
\$\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);
  });
\$\endgroup\$
  • \$\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 Jan 29 '15 at 13:10
0
\$\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);
\$\endgroup\$
  • \$\begingroup\$ I propose to edit your other anser, instead of creating a new answer for each code improvement. \$\endgroup\$ – vog Jan 29 '15 at 12:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.