fix: serve and list files with # or ? in their names (#616)

* fix: serve and list files with # or ? in their names

* test: cover ? in file names and # or ? in directory names in listings

Author: lprnmns

index.js

@@ -23,6 +23,7 @@ const endForwardSlashRegex = /\/$/u
 const asteriskRegex = /\*/gu
 const dotDotSegmentRegex = /(?:^|[\\/])\.\.(?:[\\/]|$)/u
 const leadingDotDotSegmentRegex = /^\/?\.\.(?:[\\/]|$)/u
+const encodedHashOrQuestionMarkRegex = /%(?:23|3f)/giu
 
 const supportedEncodings = ['br', 'gzip', 'deflate']
 send.mime.default_type = 'application/octet-stream'
@@ -959,8 +960,8 @@ function getPathnameForSend (url, matchRoutePrefix) {
   }
 
   try {
-    const decodedUrlPathname = decodeURI(url.slice(0, pathnameEnd))
-    const decodedPathname = decodeURI(pathname)
+    const decodedUrlPathname = decodePathname(url.slice(0, pathnameEnd))
+    const decodedPathname = decodePathname(pathname)
 
     // Check the full raw URL path, because route params can consume a
     // dot-dot segment before the suffix is passed to @fastify/send.
@@ -982,6 +983,17 @@ function getPathnameForSend (url, matchRoutePrefix) {
   }
 }
 
+/**
+ * decodeURI() keeps `#` and `?` encoded, so a file with one of them in its
+ * name could never be served. Decode those two first: neither is `%`, so this
+ * cannot create a new escape sequence and the input is still decoded once.
+ * @param {string} pathname
+ * @returns {string}
+ */
+function decodePathname (pathname) {
+  return decodeURI(pathname.replace(encodedHashOrQuestionMarkRegex, decodeURIComponent))
+}
+
 /**
  * @param {string} url
  * @return {string}

lib/dirList.js

@@ -10,6 +10,7 @@ const {
   FST_STATIC_MULTIROOT_LIST_CONFLICT
 } = require('./errors.js')
 const fastqConcurrency = Math.max(1, os.cpus().length - 1)
+const URL_DELIMITERS_REG = /[#?]/g
 
 function isPathInsideRoot (root, target) {
   const relativePath = path.relative(root, target)
@@ -164,7 +165,8 @@ const dirList = {
       route = path.posix.normalize(path.posix.join(route, '..'))
     }
     return {
-      href: encodeURI(path.posix.join(prefix, route, entry.name)),
+      // encodeURI leaves `#` and `?` as is, which would cut the href short
+      href: encodeURI(path.posix.join(prefix, route, entry.name)).replace(URL_DELIMITERS_REG, encodeURIComponent),
       name: entry.name,
       stats: entry.stats,
       extendedInfo: entry.extendedInfo

test/dir-list.test.js

@@ -4,6 +4,7 @@
 
 const fs = require('node:fs')
 const http = require('node:http')
+const os = require('node:os')
 const path = require('node:path')
 const { test } = require('node:test')
 const Fastify = require('fastify')
@@ -369,6 +370,63 @@ test('dir list href nested structure', async t => {
   })
 })
 
+test('dir list html format - href encodes # and ? in file and directory names', async t => {
+  const root = fs.mkdtempSync(path.join(os.tmpdir(), 'fastify-static-dir-list-hash-'))
+  t.after(() => fs.rmSync(root, { recursive: true, force: true }))
+  // `?` is not allowed in Windows file names
+  const posix = process.platform !== 'win32'
+  const files = { 'a#b.txt': '/public/a%23b.txt' }
+  if (posix) {
+    files['a?b.txt'] = '/public/a%3Fb.txt'
+  }
+  const dir = posix ? 'd#?x' : 'd#x'
+  const dirHref = posix ? '/public/d%23%3Fx' : '/public/d%23x'
+  for (const name of Object.keys(files)) {
+    fs.writeFileSync(path.join(root, name), name)
+  }
+  fs.mkdirSync(path.join(root, dir))
+  fs.writeFileSync(path.join(root, dir, 'c#d.txt'), 'nested')
+
+  const options = {
+    root,
+    prefix: '/public',
+    index: false,
+    list: {
+      format: 'html',
+      names: ['index'],
+      render (dirs, files) {
+        return JSON.stringify({
+          dirs: dirs.map(entry => entry.href),
+          files: files.map(entry => entry.href).sort()
+        })
+      }
+    }
+  }
+
+  await helper.arrange(t, options, async (url) => {
+    const response = await fetch(url + '/public/index')
+    t.assert.deepStrictEqual(response.status, 200)
+    const listing = JSON.parse(await response.text())
+    t.assert.deepStrictEqual(listing.dirs, [dirHref])
+    t.assert.deepStrictEqual(listing.files, Object.values(files).sort())
+
+    for (const [name, href] of Object.entries(files)) {
+      const file = await fetch(url + href)
+      t.assert.deepStrictEqual(file.status, 200)
+      t.assert.deepStrictEqual(await file.text(), name)
+    }
+
+    const nested = await fetch(url + dirHref + '/index')
+    t.assert.deepStrictEqual(nested.status, 200)
+    const nestedListing = JSON.parse(await nested.text())
+    t.assert.deepStrictEqual(nestedListing.files, [dirHref + '/c%23d.txt'])
+
+    const file = await fetch(url + nestedListing.files[0])
+    t.assert.deepStrictEqual(file.status, 200)
+    t.assert.deepStrictEqual(await file.text(), 'nested')
+  })
+})
+
 test('dir list html format - stats', async t => {
   t.plan(6)
 

test/static.test.js

@@ -5353,3 +5353,32 @@ test('register with wildcard false and globIgnore', async t => {
     await response.text()
   })
 })
+
+test('serves files with # or ? in their names', async (t) => {
+  const root = fs.mkdtempSync(path.join(os.tmpdir(), 'fastify-static-reserved-'))
+  t.after(() => fs.rmSync(root, { recursive: true, force: true }))
+  const files = { 'a#b.txt': '/a%23b.txt', '100%#.txt': '/100%25%23.txt' }
+  // `?` is not allowed in Windows file names
+  if (process.platform !== 'win32') {
+    files['a?b.txt'] = '/a%3Fb.txt'
+  }
+  for (const name of Object.keys(files)) {
+    fs.writeFileSync(path.join(root, name), name)
+  }
+  fs.writeFileSync(path.join(root, 'a%23b.txt'), 'literal')
+
+  const fastify = Fastify()
+  t.after(() => fastify.close())
+  fastify.register(fastifyStatic, { root, prefix: '/public' })
+
+  for (const [name, urlPath] of Object.entries(files)) {
+    const response = await fastify.inject('/public' + urlPath)
+    t.assert.deepStrictEqual(response.statusCode, 200)
+    t.assert.deepStrictEqual(response.body, name)
+  }
+
+  // `%25` still decodes to a literal `%`, and only once
+  const literal = await fastify.inject('/public/a%2523b.txt')
+  t.assert.deepStrictEqual(literal.statusCode, 200)
+  t.assert.deepStrictEqual(literal.body, 'literal')
+})