Fix .acl lookup for paths with spaces

lib/handlers/allow.js

@@ -2,9 +2,9 @@ module.exports = allow
 
 var ACL = require('../acl-checker')
 var $rdf = require('rdflib')
-var url = require('url')
 var utils = require('../utils')
 var debug = require('../debug.js').ACL
+var LegacyResourceMapper = require('../legacy-resource-mapper')
 
 function allow (mode) {
   return function allowHandler (req, res, next) {
@@ -13,6 +13,14 @@ function allow (mode) {
       return next()
     }
 
+    // Set up URL to filesystem mapping
+    const rootUrl = utils.getBaseUri(req)
+    const mapper = new LegacyResourceMapper({
+      rootUrl,
+      rootPath: ldp.root,
+      includeHost: ldp.multiuser
+    })
+
     // Determine the actual path of the request
     var reqPath = res && res.locals && res.locals.path
       ? res.locals.path
@@ -27,11 +35,10 @@ function allow (mode) {
       }
 
       // Obtain and store the ACL of the requested resource
-      const baseUri = utils.getBaseUri(req)
-      req.acl = new ACL(baseUri + reqPath, {
+      req.acl = new ACL(rootUrl + reqPath, {
         origin: req.get('origin'),
         host: req.protocol + '://' + req.get('host'),
-        fetch: fetchDocument(req.hostname, ldp, baseUri),
+        fetch: fetchFromLdp(mapper, ldp),
         suffix: ldp.suffixAcl,
         strictOrigin: ldp.strictOrigin
       })
@@ -53,40 +60,23 @@ function allow (mode) {
  * The `fetch(uri, callback)` results in the callback, with either:
  *   - `callback(err, graph)` if any error is encountered, or
  *   - `callback(null, graph)` with the parsed RDF graph of the fetched resource
- * @method fetchDocument
- * @param host {string} req.hostname. Used in determining the location of the
- *   document on the file system (the root directory)
- * @param ldp {LDP} LDP instance
- * @param baseUri {string} Base URI of the solid server (including any root
- *   mount), for example `https://example.com/solid`
  * @return {Function} Returns a `fetch(uri, callback)` handler
  */
-function fetchDocument (host, ldp, baseUri) {
-  return function fetch (uri, callback) {
-    readFile(uri, host, ldp, baseUri).then(body => {
+function fetchFromLdp (mapper, ldp) {
+  return function fetch (url, callback) {
+    // Convert the URL into a filename
+    mapper.mapUrlToFile({ url })
+    // Read the file from disk
+    .then(({ path }) => new Promise((resolve, reject) => {
+      ldp.readFile(path, (e, c) => e ? reject(e) : resolve(c))
+    }))
+    // Parse the file as Turtle
+    .then(body => {
       const graph = $rdf.graph()
-      $rdf.parse(body, graph, uri, 'text/turtle')
+      $rdf.parse(body, graph, url, 'text/turtle')
       return graph
     })
+    // Return the ACL graph
     .then(graph => callback(null, graph), callback)
   }
 }
-
-// Reads the given file, returning its contents
-function readFile (uri, host, ldp, baseUri) {
-  return new Promise((resolve, reject) => {
-    // If local request, slice off the initial baseUri
-    // S(uri).chompLeft(baseUri).s
-    var newPath = uri.startsWith(baseUri)
-      ? uri.slice(baseUri.length)
-      : uri
-    // Determine the root file system folder to look in
-    // TODO prettify this
-    var root = !ldp.multiuser ? ldp.root : ldp.root + host + '/'
-    // Derive the file path for the resource
-    var documentPath = utils.uriToFilename(newPath, root)
-    var documentUri = url.parse(documentPath)
-    documentPath = documentUri.pathname
-    ldp.readFile(documentPath, (e, c) => e ? reject(e) : resolve(c))
-  })
-}

lib/legacy-resource-mapper.js

@@ -0,0 +1,21 @@
+const ResourceMapper = require('./resource-mapper')
+
+// A LegacyResourceMapper models the old mapping between HTTP URLs and server filenames,
+// and is intended to be replaced by ResourceMapper
+class LegacyResourceMapper extends ResourceMapper {
+  constructor (options) {
+    super(Object.assign({ defaultContentType: 'text/turtle' }, options))
+  }
+
+  // Maps the request for a given resource and representation format to a server file
+  async mapUrlToFile ({ url }) {
+    return { path: this._getFullPath(url), contentType: this._getContentTypeByExtension(url) }
+  }
+
+  // Preserve dollars in paths
+  _removeDollarExtension (path) {
+    return path
+  }
+}
+
+module.exports = LegacyResourceMapper

lib/resource-mapper.js

@@ -4,95 +4,103 @@ const { promisify } = require('util')
 const { types, extensions } = require('mime-types')
 const readdir = promisify(fs.readdir)
 
-const DEFAULT_CONTENTTYPE = 'application/octet-stream'
-
 // A ResourceMapper maintains the mapping between HTTP URLs and server filenames,
 // following the principles of the “sweet spot” discussed in
 // https://www.w3.org/DesignIssues/HTTPFilenameMapping.html
 class ResourceMapper {
-  constructor ({ rootUrl, rootPath, includeHost }) {
-    this._rootUrl = removeTrailingSlash(rootUrl)
-    this._rootPath = removeTrailingSlash(rootPath)
+  constructor ({ rootUrl, rootPath, includeHost, defaultContentType = 'application/octet-stream' }) {
+    this._rootUrl = this._removeTrailingSlash(rootUrl)
+    this._rootPath = this._removeTrailingSlash(rootPath)
     this._includeHost = includeHost
     this._readdir = readdir
+    this._defaultContentType = defaultContentType
 
     // If the host needs to be replaced on every call, pre-split the root URL
     if (includeHost) {
-      const { protocol, pathname } = URL.parse(rootUrl)
+      const { protocol, port, pathname } = URL.parse(rootUrl)
       this._protocol = protocol
-      this._rootUrl = removeTrailingSlash(pathname)
+      this._port = port === null ? '' : `:${port}`
+      this._rootUrl = this._removeTrailingSlash(pathname)
     }
   }
 
   // Maps the request for a given resource and representation format to a server file
   async mapUrlToFile ({ url, contentType, createIfNotExists }) {
-    // Split the URL into components
-    const { pathname, hostname } = typeof url === 'string' ? URL.parse(url) : url
-    if (pathname.indexOf('/..') >= 0) {
-      throw new Error('Disallowed /.. segment in URL')
-    }
-
+    const fullPath = this._getFullPath(url)
     let path
-    const basePath = this.getBasePath(hostname)
+
     // Create the path for a new file
     if (createIfNotExists) {
-      path = `${basePath}${pathname}`
+      path = fullPath
       // If the extension is not correct for the content type, append the correct extension
-      if (getContentType(pathname) !== contentType) {
+      if (this._getContentTypeByExtension(path) !== contentType) {
         path += contentType in extensions ? `$.${extensions[contentType][0]}` : '$.unknown'
       }
     // Determine the path of an existing file
     } else {
       // Read all files in the corresponding folder
-      const filename = /[^/]*$/.exec(pathname)[0]
-      const folder = `${basePath}${pathname.substr(0, pathname.length - filename.length)}`
+      const filename = fullPath.substr(fullPath.lastIndexOf('/') + 1)
+      const folder = fullPath.substr(0, fullPath.length - filename.length)
       const files = await this._readdir(folder)
 
       // Find a file with the same name (minus the dollar extension)
-      const match = files.find(f => removeDollarExtension(f) === filename)
+      const match = files.find(f => this._removeDollarExtension(f) === filename)
       if (!match) {
         throw new Error('File not found')
       }
       path = `${folder}${match}`
-      contentType = getContentType(match)
+      contentType = this._getContentTypeByExtension(match)
     }
 
-    return { path, contentType: contentType || DEFAULT_CONTENTTYPE }
+    return { path, contentType: contentType || this._defaultContentType }
   }
 
   // Maps a given server file to a URL
   async mapFileToUrl ({ path, hostname }) {
     // Determine the URL by chopping off everything after the dollar sign
-    const pathname = removeDollarExtension(path.substring(this._rootPath.length))
-    const url = `${this.getBaseUrl(hostname)}${pathname}`
-    return { url, contentType: getContentType(path) }
+    const pathname = this._removeDollarExtension(path.substring(this._rootPath.length))
+    const url = `${this.getBaseUrl(hostname)}${encodeURI(pathname)}`
+    return { url, contentType: this._getContentTypeByExtension(path) }
   }
 
   // Gets the base file path for the given hostname
   getBasePath (hostname) {
-    return this._includeHost ? `${this._rootPath}/${hostname}` : this._rootPath
+    return !this._includeHost ? this._rootPath : `${this._rootPath}/${hostname}`
   }
 
   // Gets the base URL for the given hostname
   getBaseUrl (hostname) {
-    return this._includeHost ? `${this._protocol}//${hostname}${this._rootUrl}` : this._rootUrl
+    return !this._includeHost ? this._rootUrl
+                              : `${this._protocol}//${hostname}${this._port}${this._rootUrl}`
   }
-}
 
-// Removes a possible trailing slash from a path
-function removeTrailingSlash (path) {
-  return path ? path.replace(/\/+$/, '') : ''
-}
+  // Determine the full file path corresponding to a URL
+  _getFullPath (url) {
+    const { pathname, hostname } = typeof url === 'string' ? URL.parse(url) : url
+    const fullPath = decodeURIComponent(`${this.getBasePath(hostname)}${pathname}`)
+    if (fullPath.indexOf('/..') >= 0) {
+      throw new Error('Disallowed /.. segment in URL')
+    }
+    return fullPath
+  }
 
-// Removes everything beyond the dollar sign from a path
-function removeDollarExtension (path) {
-  return path.replace(/\$.*/, '')
-}
+  // Gets the expected content type based on the extension of the path
+  _getContentTypeByExtension (path) {
+    const extension = /\.([^/.]+)$/.exec(path)
+    return extension && types[extension[1].toLowerCase()] || this._defaultContentType
+  }
+
+  // Removes a possible trailing slash from a path
+  _removeTrailingSlash (path) {
+    const lastPos = path.length - 1
+    return lastPos < 0 || path[lastPos] !== '/' ? path : path.substr(0, lastPos)
+  }
 
-// Gets the expected content type based on the extension of the path
-function getContentType (path) {
-  const extension = /\.([^/.]+)$/.exec(path)
-  return extension && types[extension[1].toLowerCase()] || DEFAULT_CONTENTTYPE
+  // Removes everything beyond the dollar sign from a path
+  _removeDollarExtension (path) {
+    const dollarPos = path.lastIndexOf('$')
+    return dollarPos < 0 ? path : path.substr(0, dollarPos)
+  }
 }
 
 module.exports = ResourceMapper