Properly escape file names in HTML

fixes expressjs#28

HISTORY.md

@@ -1,11 +1,13 @@
 unreleased
 ==========
 
+  * Properly escape file names in HTML
   * deps: accepts@~1.2.5
     - deps: mime-types@~2.0.10
   * deps: debug@~2.1.3
     - Fix high intensity foreground color for bold
     - deps: ms@0.7.0
+  * deps: escape-html@1.0.1
   * deps: mime-types@~2.0.10
     - Add new mime types
 

LICENSE

@@ -3,7 +3,7 @@
 Copyright (c) 2010 Sencha Inc.
 Copyright (c) 2011 LearnBoost
 Copyright (c) 2011 TJ Holowaychuk
-Copyright (c) 2014 Douglas Christopher Wilson
+Copyright (c) 2014-2015 Douglas Christopher Wilson
 
 Permission is hereby granted, free of charge, to any person obtaining
 a copy of this software and associated documentation files (the

index.js

@@ -2,7 +2,7 @@
  * serve-index
  * Copyright(c) 2011 Sencha Inc.
  * Copyright(c) 2011 TJ Holowaychuk
- * Copyright(c) 2014 Douglas Christopher Wilson
+ * Copyright(c) 2014-2015 Douglas Christopher Wilson
  * MIT Licensed
  */
 
@@ -11,11 +11,13 @@
 
 /**
  * Module dependencies.
+ * @private
  */
 
 var accepts = require('accepts');
 var createError = require('http-errors');
 var debug = require('debug')('serve-index');
+var escapeHtml = require('escape-html');
 var fs = require('fs')
   , path = require('path')
   , normalize = path.normalize
@@ -175,7 +177,7 @@ exports.html = function(req, res, files, next, dir, showUp, icons, path, view, t
         str = str
           .replace(/\{style\}/g, style.concat(iconStyle(files, icons)))
           .replace(/\{files\}/g, html(files, dir, icons, view))
-          .replace(/\{directory\}/g, dir)
+          .replace(/\{directory\}/g, escapeHtml(dir))
           .replace(/\{linked-path\}/g, htmlPath(dir));
 
         var buf = new Buffer(str, 'utf8');
@@ -227,11 +229,19 @@ function fileSort(a, b) {
  */
 
 function htmlPath(dir) {
-  var curr = [];
-  return dir.split('/').map(function(part){
-    curr.push(encodeURIComponent(part));
-    return part ? '<a href="' + curr.join('/') + '">' + part + '</a>' : '';
-  }).join(' / ');
+  var parts = dir.split('/');
+  var crumb = new Array(parts.length);
+
+  for (var i = 0; i < parts.length; i++) {
+    var part = parts[i];
+
+    if (part) {
+      parts[i] = encodeURIComponent(part);
+      crumb[i] = '<a href="' + escapeHtml(parts.slice(0, i + 1).join('/')) + '">' + escapeHtml(part) + '</a>';
+    }
+  }
+
+  return crumb.join(' / ');
 }
 
 /**
@@ -342,7 +352,7 @@ function iconStyle (files, useIcons) {
  */
 
 function html(files, dir, useIcons, view) {
-  return '<ul id="files" class="view-' + view + '">'
+  return '<ul id="files" class="view-' + escapeHtml(view) + '">'
     + (view == 'details' ? (
       '<li class="header">'
       + '<span class="name">Name</span>'
@@ -382,13 +392,12 @@ function html(files, dir, useIcons, view) {
       : '';
 
     return '<li><a href="'
-      + normalizeSlashes(normalize(path.join('/')))
-      + '" class="'
-      + classes.join(' ') + '"'
-      + ' title="' + file.name + '">'
-      + '<span class="name">'+file.name+'</span>'
-      + '<span class="size">'+size+'</span>'
-      + '<span class="date">'+date+'</span>'
+      + escapeHtml(normalizeSlashes(normalize(path.join('/'))))
+      + '" class="' + escapeHtml(classes.join(' ')) + '"'
+      + ' title="' + escapeHtml(file.name) + '">'
+      + '<span class="name">' + escapeHtml(file.name) + '</span>'
+      + '<span class="size">' + escapeHtml(size) + '</span>'
+      + '<span class="date">' + escapeHtml(date) + '</span>'
       + '</a></li>';
 
   }).join('\n') + '</ul>';

package.json

@@ -9,6 +9,7 @@
     "accepts": "~1.2.5",
     "batch": "0.5.2",
     "debug": "~2.1.3",
+    "escape-html": "1.0.1",
     "http-errors": "~1.3.1",
     "mime-types": "~2.0.10",
     "parseurl": "~1.3.0"

test/test.js

@@ -109,7 +109,7 @@ describe('serveIndex(root)', function () {
         .get('/')
         .set('Accept', 'application/json')
         .expect('Content-Type', /json/)
-        .expect(/g# %3 o %2525 %37 dir/)
+        .expect(/g# %3 o & %2525 %37 dir/)
         .expect(/users/)
         .expect(/file #1\.txt/)
         .expect(/nums/)
@@ -128,14 +128,28 @@ describe('serveIndex(root)', function () {
         .set('Accept', 'text/html')
         .expect(200)
         .expect('Content-Type', 'text/html; charset=utf-8')
-        .expect(/<a href="\/g%23%20%253%20o%20%252525%20%2537%20dir"/)
+        .expect(/<a href="\/g%23%20%253%20o%20%26%20%252525%20%2537%20dir"/)
         .expect(/<a href="\/users"/)
         .expect(/<a href="\/file%20%231.txt"/)
         .expect(/<a href="\/todo.txt"/)
         .expect(/<a href="\/%E3%81%95%E3%81%8F%E3%82%89\.txt"/)
         .end(done);
       });
 
+      it('should property escape file names', function (done) {
+        var server = createServer()
+
+        request(server)
+        .get('/')
+        .set('Accept', 'text/html')
+        .expect(200)
+        .expect('Content-Type', 'text/html; charset=utf-8')
+        .expect(/<a href="\/foo%20%26%20bar"/)
+        .expect(/foo &amp; bar/)
+        .expect(bodyDoesNotContain('foo & bar'))
+        .end(done);
+      });
+
       it('should sort folders first', function (done) {
         var server = createServer()
 
@@ -151,10 +165,10 @@ describe('serveIndex(root)', function () {
           assert.deepEqual(urls, [
             '/%23directory',
             '/collect',
-            '/g%23%20%253%20o%20%252525%20%2537%20dir',
+            '/g%23%20%253%20o%20%26%20%252525%20%2537%20dir',
             '/users',
             '/file%20%231.txt',
-            '/foo%20bar',
+            '/foo%20%26%20bar',
             '/nums',
             '/todo.txt',
             '/%E3%81%95%E3%81%8F%E3%82%89.txt'
@@ -174,7 +188,7 @@ describe('serveIndex(root)', function () {
         .expect(200)
         .expect('Content-Type', 'text/plain; charset=utf-8')
         .expect(/users/)
-        .expect(/g# %3 o %2525 %37 dir/)
+        .expect(/g# %3 o & %2525 %37 dir/)
         .expect(/file #1.txt/)
         .expect(/todo.txt/)
         .expect(/さくら\.txt/)
@@ -454,12 +468,26 @@ describe('serveIndex(root)', function () {
       var server = createServer()
 
       request(server)
-      .get('/g%23%20%253%20o%20%252525%20%2537%20dir/')
+      .get('/g%23%20%253%20o%20%26%20%252525%20%2537%20dir/')
+      .set('Accept', 'text/html')
+      .expect(200)
+      .expect('Content-Type', 'text/html; charset=utf-8')
+      .expect(/<a href="\/g%23%20%253%20o%20%26%20%252525%20%2537%20dir"/)
+      .expect(/<a href="\/g%23%20%253%20o%20%26%20%252525%20%2537%20dir\/empty.txt"/)
+      .end(done);
+    });
+
+    it('should property escape directory names', function (done) {
+      var server = createServer()
+
+      request(server)
+      .get('/g%23%20%253%20o%20%26%20%252525%20%2537%20dir/')
       .set('Accept', 'text/html')
       .expect(200)
       .expect('Content-Type', 'text/html; charset=utf-8')
-      .expect(/<a href="\/g%23%20%253%20o%20%252525%20%2537%20dir"/)
-      .expect(/<a href="\/g%23%20%253%20o%20%252525%20%2537%20dir\/empty.txt"/)
+      .expect(/<a href="\/g%23%20%253%20o%20%26%20%252525%20%2537%20dir"/)
+      .expect(/g# %3 o &amp; %2525 %37 dir/)
+      .expect(bodyDoesNotContain('g# %3 o & %2525 %37 dir'))
       .end(done);
     });
 
@@ -483,7 +511,7 @@ describe('serveIndex(root)', function () {
       request(server)
       .get('/')
       .set('Accept', 'text/html')
-      .expect(/<a href="\/g%23%20%253%20o%20%252525%20%2537%20dir"/)
+      .expect(/<a href="\/g%23%20%253%20o%20%26%20%252525%20%2537%20dir"/)
       .expect(/<a href="\/users"/)
       .expect(/<a href="\/file%20%231.txt"/)
       .expect(/<a href="\/todo.txt"/)