Merge branch 'master' into micah/35/sort

.travis.yml

@@ -8,10 +8,13 @@ node_js:
   - "3.3"
   - "4.9"
   - "5.12"
-  - "6.14"
+  - "6.17"
   - "7.10"
-  - "8.11"
+  - "8.16"
   - "9.11"
+  - "10.15"
+  - "11.15"
+  - "12.1"
 sudo: false
 cache:
   directories:

HISTORY.md

@@ -1,16 +1,21 @@
 unreleased
 ==========
 
+  * Fix text and json responses to match html sorting
   * Show font icon for more font types
-  * deps: accepts@~1.3.5
-    - deps: mime-types@~2.1.18
-  * deps: http-errors@~1.6.3
+  * Use 400 error on URI decode failure instead of 500
+  * deps: accepts@~1.3.7
+    - deps: mime-types@~2.1.24
+    - deps: [email protected]
+  * deps: http-errors@~1.7.2
+    - Set constructor name when possible
     - deps: depd@~1.1.2
-    - deps: [email protected].0
-    - deps: statuses@'>= 1.4.0 < 2'
-  * deps: mime-types@~2.1.18
+    - deps: [email protected].1
+    - deps: statuses@'>= 1.5.0 < 2'
+  * deps: mime-types@~2.1.24
     - Add new mime types
-    - deps: mime-db@~1.33.0
+    - deps: mime-db@~1.40.0
+  * deps: parseurl@~1.3.3
 
 1.9.1 / 2017-09-28
 ==================

appveyor.yml

@@ -8,14 +8,19 @@ environment:
     - nodejs_version: "3.3"
     - nodejs_version: "4.9"
     - nodejs_version: "5.12"
-    - nodejs_version: "6.14"
+    - nodejs_version: "6.17"
     - nodejs_version: "7.10"
-    - nodejs_version: "8.11"
+    - nodejs_version: "8.16"
     - nodejs_version: "9.11"
+    - nodejs_version: "10.15"
+    - nodejs_version: "11.15"
+    - nodejs_version: "12.1"
 cache:
   - node_modules
 install:
-  - ps: Install-Product node $env:nodejs_version
+  - ps: >-
+      try { Install-Product node $env:nodejs_version -ErrorAction Stop }
+      catch { Update-NodeJsInstallation (Get-NodeJsLatestBuild $env:nodejs_version) }
   - npm config set shrinkwrap false
   - if "%nodejs_version%" equ "0.8" npm config set strict-ssl false
   - if "%nodejs_version%" equ "0.8" npm rm --save-dev istanbul

index.js

@@ -107,10 +107,14 @@ function serveIndex(root, options) {
       return;
     }
 
+    // get dir
+    var dir = getRequestedDir(req)
+
+    // bad request
+    if (dir === null) return next(createError(400))
+
     // parse URLs
-    var url = parseUrl(req);
     var originalUrl = parseUrl.original(req);
-    var dir = decodeURIComponent(url.pathname);
     var originalDir = decodeURIComponent(originalUrl.pathname);
 
     // join / normalize from root dir
@@ -180,14 +184,9 @@ serveIndex.html = function _html(req, res, files, next, dir, showUp, icons, path
   }
 
   // stat all files
-  stat(path, files, function (err, stats) {
+  stat(path, files, function (err, fileList) {
     if (err) return next(err);
 
-    // combine the stats into the file list
-    var fileList = files.map(function (file, i) {
-      return { name: file, stat: stats[i] };
-    });
-
     // sort file list
     fileList.sort(sort);
 
@@ -218,16 +217,42 @@ serveIndex.html = function _html(req, res, files, next, dir, showUp, icons, path
  * Respond with application/json.
  */
 
-serveIndex.json = function _json(req, res, files) {
-  send(res, 'application/json', JSON.stringify(files))
+serveIndex.json = function _json (req, res, files, next, dir, showUp, icons, path) {
+  // stat all files
+  stat(path, files, function (err, fileList) {
+    if (err) return next(err)
+
+    // sort file list
+    fileList.sort(fileSort)
+
+    // serialize
+    var body = JSON.stringify(fileList.map(function (file) {
+      return file.name
+    }))
+
+    send(res, 'application/json', body)
+  })
 };
 
 /**
  * Respond with text/plain.
  */
 
-serveIndex.plain = function _plain(req, res, files) {
-  send(res, 'text/plain', (files.join('\n') + '\n'))
+serveIndex.plain = function _plain (req, res, files, next, dir, showUp, icons, path) {
+  // stat all files
+  stat(path, files, function (err, fileList) {
+    if (err) return next(err)
+
+    // sort file list
+    fileList.sort(fileSort)
+
+    // serialize
+    var body = fileList.map(function (file) {
+      return file.name
+    }).join('\n') + '\n'
+
+    send(res, 'text/plain', body)
+  })
 };
 
 /**
@@ -327,6 +352,22 @@ function fileSort(a, b) {
     String(a.name).toLocaleLowerCase().localeCompare(String(b.name).toLocaleLowerCase());
 }
 
+/**
+ * Get the requested directory from request.
+ *
+ * @param req
+ * @return {string}
+ * @api private
+ */
+
+function getRequestedDir (req) {
+  try {
+    return decodeURIComponent(parseUrl(req).pathname)
+  } catch (e) {
+    return null
+  }
+}
+
 /**
  * Map html `dir`, returning a linked path.
  */
@@ -507,8 +548,12 @@ function send (res, type, body) {
 }
 
 /**
- * Stat all files and return array of stat
- * in same order.
+ * Stat all files and return array of objects in the form
+ * `{ name, stat }`.
+ *
+ * @param {Array} files
+ * @return {Array}
+ * @api private
  */
 
 function stat(dir, files, cb) {
@@ -522,7 +567,10 @@ function stat(dir, files, cb) {
         if (err && err.code !== 'ENOENT') return done(err);
 
         // pass ENOENT as null stat, not error
-        done(null, stat || null);
+        done(null, {
+          name: file,
+          stat: stat || null
+        })
       });
     });
   });

package.json

@@ -6,18 +6,18 @@
   "license": "MIT",
   "repository": "expressjs/serve-index",
   "dependencies": {
-    "accepts": "~1.3.5",
+    "accepts": "~1.3.7",
     "batch": "0.6.1",
     "debug": "2.6.9",
     "escape-html": "~1.0.3",
-    "http-errors": "~1.6.3",
-    "mime-types": "~2.1.18",
-    "parseurl": "~1.3.2"
+    "http-errors": "~1.7.2",
+    "mime-types": "~2.1.24",
+    "parseurl": "~1.3.3"
   },
   "devDependencies": {
     "after": "0.8.2",
     "eslint": "3.19.0",
-    "eslint-plugin-markdown": "1.0.0-beta.6",
+    "eslint-plugin-markdown": "1.0.0-beta.8",
     "istanbul": "0.4.5",
     "mocha": "2.5.3",
     "supertest": "1.1.0"

test/test.js

@@ -76,6 +76,14 @@ describe('serveIndex(root)', function () {
     .expect(400, done)
   })
 
+  it('should deny path that does not decode', function (done) {
+    var server = createServer()
+
+    request(server)
+      .head('/%FF')
+      .expect(400, done)
+  })
+
   it('should deny path outside root', function (done) {
     var server = createServer()
 
@@ -93,11 +101,11 @@ describe('serveIndex(root)', function () {
   })
 
   it('should treat an ENAMETOOLONG as a 414', function (done) {
-    var path = Array(11000).join('foobar')
-    var server = createServer()
+    var dir = path.join(fixtures, Array(10000).join('/foobar'))
+    var server = createServer(dir)
 
     request(server)
-    .get('/' + path)
+    .get('/')
     .expect(414, done)
   })
 
@@ -136,6 +144,26 @@ describe('serveIndex(root)', function () {
         .expect('X-Content-Type-Options', 'nosniff')
         .expect(200, done)
       })
+
+      it('should sort folders first', function (done) {
+        request(createServer())
+          .get('/')
+          .set('Accept', 'application/json')
+          .expect(200)
+          .expect('Content-Type', 'application/json; charset=utf-8')
+          .expect([
+            '#directory',
+            'collect',
+            'g# %3 o & %2525 %37 dir',
+            'users',
+            'file #1.txt',
+            'foo & bar',
+            'nums',
+            'todo.txt',
+            'さくら.txt'
+          ])
+          .end(done)
+      })
     });
 
     describe('when Accept: text/html is given', function () {
@@ -233,6 +261,27 @@ describe('serveIndex(root)', function () {
         .expect('X-Content-Type-Options', 'nosniff')
         .expect(200, done)
       })
+
+      it('should sort folders first', function (done) {
+        request(createServer())
+          .get('/')
+          .set('Accept', 'text/plain')
+          .expect(200)
+          .expect('Content-Type', 'text/plain; charset=utf-8')
+          .expect([
+            '#directory',
+            'collect',
+            'g# %3 o & %2525 %37 dir',
+            'users',
+            'file #1.txt',
+            'foo & bar',
+            'nums',
+            'todo.txt',
+            'さくら.txt',
+            ''
+          ].join('\n'))
+          .end(done)
+      })
     });
 
     describe('when Accept: application/x-bogus is given', function () {