Skip to content

Commit 5bba6c9

Browse files
timbldmitrizagidulin
timbl
authored and
dmitrizagidulin
committed
Fix URI encoding in file listing and decoding to get file names (nodeSolidServer#471)
* Fix URI encoding in file listing and decoding to get file names, and improve debug a little * Fix bug causing .ttl files in a directory to be parsed when directory is listed, taking ages. ferossified * Fix url decodeing of POST slug, add test for bad characters, add test for decoding.
1 parent 5b5b7f2 commit 5bba6c9

7 files changed

Lines changed: 53 additions & 14 deletions

File tree

‎lib/acl-checker.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,9 @@ class ACLChecker {
117117
debug(`${mode} access permitted to ${user}`)
118118
return callback()
119119
} else {
120-
debug(`${mode} access not permitted to ${user}`)
121-
return callback(new Error('Acl found but policy not found'))
120+
debug(`${mode} access NOT permitted to ${user}` +
121+
aclOptions.strictOrigin ? ` and origin ${options.origin}` : '')
122+
return callback(new Error('ACL file found but no matching policy found'))
122123
}
123124
})
124125
.catch(err => {

‎lib/ldp-container.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ var error = require('./http-error')
1010
var fs = require('fs')
1111
var ns = require('solid-namespace')($rdf)
1212
var S = require('string')
13-
var turtleExtension = '.ttl'
13+
// var turtleExtension = '.ttl'
1414
var mime = require('mime-types')
1515

1616
function addContainerStats (ldp, reqUri, filename, resourceGraph, next) {
@@ -58,9 +58,9 @@ function addFile (ldp, resourceGraph, containerUri, reqUri, uri, container, file
5858
resourceGraph.sym(memberUri))
5959

6060
// Set up a metaFile path
61-
var metaFile = container + file +
62-
(stats.isDirectory() ? '/' : '') +
63-
(S(file).endsWith(turtleExtension) ? '' : ldp.suffixMeta)
61+
// Earlier code used a .ttl file as its own meta file, which
62+
// caused massive data files to parsed as part of deirectory listings just looking for type triples
63+
var metaFile = container + file + ldp.suffixMeta
6464

6565
getMetadataGraph(ldp, metaFile, memberUri, function (err, metadataGraph) {
6666
if (err) {
@@ -147,7 +147,7 @@ function readdir (filename, callback) {
147147
return callback(error(err, 'Can\'t read container'))
148148
}
149149

150-
debug.handlers('Files in directory: ' + files)
150+
debug.handlers('Files in directory: ' + files.toString().slice(0, 100))
151151
return callback(null, files)
152152
})
153153
}

‎lib/ldp.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ class LDP {
157157
async.each(
158158
files,
159159
function (file, cb) {
160-
let fileUri = reqUri + file
160+
let fileUri = reqUri + encodeURIComponent(file)
161161
ldpContainer.addFile(ldp, resourceGraph, reqUri, fileUri, uri,
162162
filename, file, cb)
163163
},
@@ -185,7 +185,11 @@ class LDP {
185185
debug.handlers('POST -- On parent: ' + containerPath)
186186
// prepare slug
187187
if (slug) {
188-
slug = encodeURIComponent(slug)
188+
slug = decodeURIComponent(slug)
189+
if (slug.match(/\/|\||:/)) {
190+
callback(error(400, 'The name of new file POSTed may not contain : | or /'))
191+
return
192+
}
189193
}
190194
// TODO: possibly package this in ldp.post
191195
ldp.getAvailablePath(hostname, containerPath, slug, function (resourcePath) {
@@ -293,7 +297,7 @@ class LDP {
293297
ldp.stat(filename, function (err, stats) {
294298
// File does not exist
295299
if (err) {
296-
return callback(error(err, "Can't find resource requested"))
300+
return callback(error(err, 'Can\'t find file requested: ' + filename))
297301
}
298302

299303
// Just return, since resource exists

‎lib/utils.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ function debrack (s) {
3333
}
3434

3535
function uriToFilename (uri, base) {
36-
var filename = path.join(base, uri)
36+
var decoded = uri.split('/').map(decodeURIComponent).join('/')
37+
var filename = path.join(base, decoded)
3738
// Make sure filename ends with '/' if filename exists and is a directory.
3839
// TODO this sync operation can be avoided and can be left
3940
// to do, to other components, see `ldp.get`

‎test/http.js‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,29 @@ describe('HTTP APIs', function () {
546546
.expect(200, done)
547547
})
548548

549+
it('should create a container with a name hex decoded from the slug', (done) => {
550+
let containerName = 'Film%4011'
551+
let expectedDirName = '/post-tests/Film@11/'
552+
server.post('/post-tests/')
553+
.set('slug', containerName)
554+
.set('content-type', 'text/turtle')
555+
.set('link', '<http://www.w3.org/ns/ldp#BasicContainer>; rel="type"')
556+
.expect(201)
557+
.end((err, res) => {
558+
if (err) return done(err)
559+
try {
560+
assert.equal(res.headers.location, expectedDirName,
561+
'Uri container names should be encoded')
562+
let createdDir = fs.statSync(path.join(__dirname, 'resources', expectedDirName))
563+
assert(createdDir.isDirectory(), 'Container should have been created')
564+
} catch (err) {
565+
return done(err)
566+
}
567+
done()
568+
})
569+
})
570+
571+
/* No, URLs are NOT ex-encoded to make filenames -- the other way around.
549572
it('should create a container with a url name', (done) => {
550573
let containerName = 'https://example.com/page'
551574
let expectedDirName = '/post-tests/https%3A%2F%2Fexample.com%2Fpage/'
@@ -574,6 +597,7 @@ describe('HTTP APIs', function () {
574597
.expect('content-type', /text\/turtle/)
575598
.expect(200, done)
576599
})
600+
*/
577601

578602
after(function () {
579603
// Clean up after POST API tests

‎test/ldp.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ describe('LDP', function () {
109109
})
110110
})
111111
describe('listContainer', function () {
112+
/*
112113
it('should inherit type if file is .ttl', function (done) {
113114
write('@prefix dcterms: <http://purl.org/dc/terms/>.' +
114115
'@prefix o: <http://example.org/ontology>.' +
@@ -145,7 +146,7 @@ describe('LDP', function () {
145146
done()
146147
})
147148
})
148-
149+
*/
149150
it('should not inherit type of BasicContainer/Container if type is File', function (done) {
150151
write('@prefix dcterms: <http://purl.org/dc/terms/>.' +
151152
'@prefix o: <http://example.org/ontology>.' +

‎test/utils.js‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,16 @@ describe('Utility functions', function () {
1919
it('should return empty as relative path for undefined path', function () {
2020
assert.equal(utils.pathBasename(undefined), '')
2121
})
22-
it('should not decode uris', function () {
23-
assert.equal(utils.uriToFilename('uri%20', 'base/'), 'base/uri%20')
22+
})
23+
describe('uriToFilename', function () {
24+
it('should decode hex-encoded space', function () {
25+
assert.equal(utils.uriToFilename('uri%20', 'base/'), 'base/uri ')
26+
})
27+
it('should decode hex-encoded at sign', function () {
28+
assert.equal(utils.uriToFilename('film%4011', 'base/'), 'base/film@11')
29+
})
30+
it('should decode hex-encoded single quote', function () {
31+
assert.equal(utils.uriToFilename('quote%27', 'base/'), 'base/quote\'')
2432
})
2533
})
2634
})

0 commit comments

Comments
 (0)