Fix handling/url-encoding of container names

lib/handlers/post.js

@@ -82,9 +82,6 @@ function handler (req, res, next) {
     debug('Receving one file')
     var linkHeader = header.parseMetadataFromHeader(req.get('Link'))
     var slug = req.get('Slug')
-    if (slug) {
-      slug = slug.replace('/')
-    }
     ldp.post(
       req.hostname,
       containerPath,

lib/ldp.js

@@ -182,30 +182,25 @@ class LDP {
 
   post (hostname, containerPath, slug, stream, container, callback) {
     var ldp = this
-
     debug.handlers('POST -- On parent: ' + containerPath)
-
     // prepare slug
-    if (container && !slug.endsWith('/')) {
-      slug += '/'
+    if (slug) {
+      slug = encodeURIComponent(slug)
     }
-
     // TODO: possibly package this in ldp.post
     ldp.getAvailablePath(hostname, containerPath, slug, function (resourcePath) {
       debug.handlers('POST -- Will create at: ' + resourcePath)
-      var meta = ''
-
+      let originalPath = resourcePath
       if (container) {
-        if (resourcePath[ resourcePath.length - 1 ] !== '/') {
-          resourcePath += '/'
+        // Create directory by an LDP PUT to the container's .meta resource
+        resourcePath = path.join(originalPath, ldp.suffixMeta)
+        if (originalPath && !originalPath.endsWith('/')) {
+          originalPath += '/'
         }
-        meta = ldp.suffixMeta
       }
-
-      ldp.put(hostname, resourcePath + meta, stream, function (err) {
+      ldp.put(hostname, resourcePath, stream, function (err) {
         if (err) callback(err)
-
-        callback(null, resourcePath)
+        callback(null, originalPath)
       })
     })
   }
@@ -216,17 +211,19 @@ class LDP {
     var filePath = utils.uriToFilename(resourcePath, root, host)
 
     // PUT requests not supported on containers. Use POST instead
-    if (filePath[ filePath.length - 1 ] === '/') {
+    if (filePath.endsWith('/')) {
       return callback(error(409,
         'PUT not supported on containers, use POST instead'))
     }
-
-    mkdirp(path.dirname(filePath), function (err) {
+    // First, create the enclosing directory, if necessary
+    var dirName = path.dirname(filePath)
+    mkdirp(dirName, (err) => {
       if (err) {
         debug.handlers('PUT -- Error creating directory: ' + err)
         return callback(error(err,
           'Failed to create the path to the new resource'))
       }
+      // Directory created, now write the file
       var file = stream.pipe(fs.createWriteStream(filePath))
       file.on('error', function () {
         callback(error(500, 'Error writing data'))

lib/utils.js

@@ -33,16 +33,15 @@ function debrack (s) {
 }
 
 function uriToFilename (uri, base) {
-  uri = decodeURIComponent(uri)
   var filename = path.join(base, uri)
   // Make sure filename ends with '/'  if filename exists and is a directory.
   // TODO this sync operation can be avoided and can be left
   // to do, to other components, see `ldp.get`
   try {
     var fileStats = fs.statSync(filename)
-    if (fileStats.isDirectory() && !S(filename).endsWith('/')) {
+    if (fileStats.isDirectory() && !filename.endsWith('/')) {
       filename += '/'
-    } else if (fileStats.isFile() && S(filename).endsWith('/')) {
+    } else if (fileStats.isFile() && filename.endsWith('/')) {
       filename = S(filename).chompRight('/').s
     }
   } catch (err) {}
@@ -89,9 +88,9 @@ function hasSuffix (path, suffixes) {
 
 function getResourceLink (filename, uri, base, suffix, otherSuffix) {
   var link = filenameToBaseUri(filename, uri, base)
-  if (S(link).endsWith(suffix)) {
+  if (link.endsWith(suffix)) {
     return link
-  } else if (S(link).endsWith(otherSuffix)) {
+  } else if (link.endsWith(otherSuffix)) {
     return S(link).chompRight(otherSuffix).s + suffix
   } else {
     return link + suffix

test/http.js

@@ -13,6 +13,7 @@ var ldpServer = ldnode.createServer({
   root: path.join(__dirname, '/resources')
 })
 var server = supertest(ldpServer)
+var assert = require('chai').assert
 
 /**
  * Creates a new test basic container via an LDP POST
@@ -534,7 +535,7 @@ describe('HTTP APIs', function () {
           if (err) return done(err)
           var stats = fs.statSync(path.join(__dirname, '/resources/post-tests/loans/'))
           if (!stats.isDirectory()) {
-            return done(new Error('Cannot read file just created'))
+            return done(new Error('Cannot read container just created'))
           }
           done()
         })
@@ -544,6 +545,36 @@ describe('HTTP APIs', function () {
         .expect('content-type', /text\/turtle/)
         .expect(200, done)
     })
+
+    it('should create a container with a url name', (done) => {
+      let containerName = 'https://example.com/page'
+      let expectedDirName = '/post-tests/https%3A%2F%2Fexample.com%2Fpage/'
+      server.post('/post-tests/')
+        .set('slug', containerName)
+        .set('content-type', 'text/turtle')
+        .set('link', '<http://www.w3.org/ns/ldp#BasicContainer>; rel="type"')
+        .expect(201)
+        .end((err, res) => {
+          if (err) return done(err)
+          try {
+            assert.equal(res.headers.location, expectedDirName,
+              'Uri container names should be encoded')
+            let createdDir = fs.statSync(path.join(__dirname, 'resources', expectedDirName))
+            assert(createdDir.isDirectory(), 'Container should have been created')
+          } catch (err) {
+            return done(err)
+          }
+          done()
+        })
+    })
+
+    it('should be able to access new url-named container', (done) => {
+      let containerUrl = '/post-tests/https%3A%2F%2Fexample.com%2Fpage/'
+      server.get(containerUrl)
+        .expect('content-type', /text\/turtle/)
+        .expect(200, done)
+    })
+
     after(function () {
       // Clean up after POST API tests
       return Promise.all([

test/utils.js

@@ -19,8 +19,8 @@ describe('Utility functions', function () {
     it('should return empty as relative path for undefined path', function () {
       assert.equal(utils.pathBasename(undefined), '')
     })
-    it('should properly decode a uri', function () {
-      assert.equal(utils.uriToFilename('uri%20', 'base/'), 'base/uri ')
+    it('should not decode uris', function () {
+      assert.equal(utils.uriToFilename('uri%20', 'base/'), 'base/uri%20')
     })
   })
 })