Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/issue#1758 server shall exclude dot files #1760

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/ldp-container.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ async function addContainerStats (ldp, reqUri, filename, resourceGraph) {
}

async function addFile (ldp, resourceGraph, containerUri, reqUri, container, file) {
// Skip .meta and .acl
if (file.endsWith(ldp.suffixMeta) || file.endsWith(ldp.suffixAcl)) {
// Skip .meta and .acl and any dot file
if (ldp.isAuxResource(file) || file.startsWith('.')) {
return null
}

Expand Down
6 changes: 3 additions & 3 deletions lib/ldp.js
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ class LDP {
({ path, contentType } = await this.resourceMapper.mapUrlToFile({ url: options, searchIndex }))
stats = await this.stat(path)
} catch (err) {
throw error(404, 'Can\'t find file requested: ' + options)
throw error(err.status || 500, err.message)
}

// Just return, since resource exists
Expand Down Expand Up @@ -545,8 +545,8 @@ class LDP {
throw error(404, 'The container does not exist')
}

// Ensure the container is empty (we ignore .meta and .acl)
if (list.some(file => !file.endsWith(this.suffixMeta) && !file.endsWith(this.suffixAcl))) {
// Ensure the container is empty (we ignore .meta, .acl and dot files)
if (list.some(file => !file.startsWith('.') && !file.endsWith(this.suffixMeta) && !file.endsWith(this.suffixAcl))) {
throw error(409, 'Container is not empty')
}

Expand Down
16 changes: 12 additions & 4 deletions lib/resource-mapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,14 @@ class ResourceMapper {
if (filePath.indexOf('/..') >= 0) {
throw new Error('Disallowed /.. segment in URL')
}
let filename
const isFolder = filePath.endsWith('/')
if (!isFolder) {
filename = filePath.split('/').pop()
if (filename.startsWith('.') && !(filename.endsWith('.acl') || filename.endsWith('.meta'))) {
throw new HTTPError(403, `filename not allowed by server: ${filename}`)
}
}
const isIndex = searchIndex && filePath.endsWith('/')

// Create the path for a new resource
Expand All @@ -120,12 +127,11 @@ class ResourceMapper {
// Determine the path of an existing file
} else {
// Read all files in the corresponding folder
const filename = filePath.substr(filePath.lastIndexOf('/') + 1)
filename = filePath.substr(filePath.lastIndexOf('/') + 1)
const folder = filePath.substr(0, filePath.length - filename.length)

// Find a file with the same name (minus the dollar extension)
let match = ''
if (match === '') { // always true to keep indentation
try {
const files = await this._readdir(folder)
// Search for files with the same name (disregarding a dollar extension)
if (!isFolder) {
Expand All @@ -134,13 +140,15 @@ class ResourceMapper {
} else if (searchIndex && files.includes(this._indexFilename)) {
match = this._indexFilename
}
} catch (err) {
throw new HTTPError(404, `${filePath} Resource not found`)
}
// Error if no match was found (unless URL ends with '/', then fall back to the folder)
if (match === undefined) {
if (isIndex) {
match = ''
} else {
throw new HTTPError(404, `Resource not found: ${pathname}`)
throw new HTTPError(404, `${pathname} Resource not found`)
}
}
path = `${folder}${match}`
Expand Down
23 changes: 22 additions & 1 deletion test/integration/http-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -335,11 +335,19 @@ describe('HTTP APIs', function () {
server.get('/invalidfile.foo')
.expect(404, done)
})
it('should return 404 for non-existent container', function (done) { // alain
it('should return 404 for non-existent container', function (done) {
server.get('/inexistant/')
.expect('Accept-Put', 'text/turtle')
.expect(404, done)
})
it('should return 403 for existing dot filename', function (done) {
server.get('/sampleContainer/.tmp')
.expect(403, done)
})
it('should return 403 for non existing dot filename', function (done) {
server.get('/.foo')
.expect(403, done)
})
it('should return basic container link for directories', function (done) {
server.get('/')
.expect('Link', /http:\/\/www.w3.org\/ns\/ldp#BasicContainer/)
Expand Down Expand Up @@ -561,6 +569,18 @@ describe('HTTP APIs', function () {
.set('content-type', '')
.expect(400, done)
})
it('should fail with 403 for existing dot file', function (done) {
server.put('/sampleContainer/.tmp')
.send(putRequestBody)
.set('content-type', 'text/turtle')
.expect(403, done)
})
it('should fail with 403 for non existing dot file', function (done) {
server.put('/.foo')
.send(putRequestBody)
.set('content-type', 'text/turtle')
.expect(403, done)
})
it('should create new resource and delete old path if different', function (done) {
server.put('/put-resource-1.ttl')
.send(putRequestBody)
Expand Down Expand Up @@ -684,6 +704,7 @@ describe('HTTP APIs', function () {
createTestResource('/.acl'),
createTestResource('/profile/card'),
createTestResource('/delete-test-empty-container/.meta.acl'),
createTestResource('/delete-test-empty-container/.tmp'),
createTestResource('/put-resource-1.ttl'),
createTestResource('/put-resource-with-acl.ttl'),
createTestResource('/put-resource-with-acl.ttl.acl'),
Expand Down
4 changes: 3 additions & 1 deletion test/integration/ldp-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ describe('LDP', function () {
})
})

it('should ldp:contains the same files in dir', () => {
it('should ldp:contains the same files in dir (excluding dot files)', () => {
ldp.listContainer(path.join(__dirname, '../resources/sampleContainer/'), 'https://server.tld/resources/sampleContainer/', '', 'server.tld')
.then(data => {
fs.readdir(path.join(__dirname, '../resources/sampleContainer/'), function (err, expectedFiles) {
Expand All @@ -385,6 +385,8 @@ describe('LDP', function () {

files.sort()
expectedFiles.sort()
const firstItem = expectedFiles.shift()
assert.deepEqual(firstItem, '.tmp')
assert.deepEqual(files, expectedFiles)
})
})
Expand Down
10 changes: 10 additions & 0 deletions test/integration/patch-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,16 @@ describe('PATCH through text/n3', () => {
result: '@prefix : </new.ttl#>.\n@prefix tim: </>.\n\ntim:x tim:y tim:z.\n\n'
}))

describe('on a non-existing dot file', describePatch({
path: '/.ttl',
exists: false,
patch: `<> a solid:InsertDeletePatch;
solid:inserts { <x> <y> <z>. }.`
}, { // expected:
status: 403,
text: 'GlobalDashboard'
}))

describe('on a non-existent JSON-LD file', describePatch({
path: '/new.jsonld',
exists: false,
Expand Down
13 changes: 13 additions & 0 deletions test/resources/.ttl
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
@prefix ldp: <http://www.w3.org/ns/ldp#>.
@prefix o: <http://example.org/ontology#>.

<http://example.org/netWorth/nw1/>
a o:NetWorth;
o:netWorthOf <http://example.org/users/JohnZSmith>;
o:asset
<assets/a1>,
<assets/a2>;
o:liability
<liabilities/l1>,
<liabilities/l2>,
<liabilities/l3>.
Empty file.
20 changes: 19 additions & 1 deletion test/unit/resource-mapper-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,19 @@ describe('ResourceMapper', () => {

// GET/HEAD/POST/DELETE/PATCH base cases

itMapsUrl(mapper, 'a URL of a non-existing folder',
{
url: 'http://localhost/space/foo/'
},
[/* no files */],
new Error('/space/foo/ Resource not found'))

itMapsUrl(mapper, 'a URL of a non-existing file',
{
url: 'http://localhost/space/foo.html'
},
[/* no files */],
new Error('Resource not found: /space/foo.html'))
new Error('/space/foo.html Resource not found'))

itMapsUrl(mapper, 'a URL of an existing file with extension',
{
Expand Down Expand Up @@ -328,6 +335,9 @@ describe('ResourceMapper', () => {
{
url: 'http://localhost/space/'
},
[
`${rootPath}space/.tmp` // fs.readdir mock needs one file
],
{
path: `${rootPath}space/`,
contentType: 'text/turtle'
Expand Down Expand Up @@ -570,6 +580,13 @@ describe('ResourceMapper', () => {
url: 'http://example.org/space/foo.html',
contentType: 'text/html'
})

itMapsUrl(mapper, 'a URL of a non-existing dot file',
{
url: 'http://localhost/space/.foo'
},
[/* no files */],
new Error('filename not allowed by server: .foo'))
})

describe('A ResourceMapper instance for a multi-host setup with a subfolder root URL', () => {
Expand Down Expand Up @@ -673,6 +690,7 @@ function mapsUrl (it, mapper, label, options, files, expected) {
function mockReaddir () {
mapper._readdir = async (path) => {
expect(path.startsWith(`${rootPath}space/`)).to.equal(true)
if (!files.length) return
return files.map(f => f.replace(/.*\//, ''))
}
}
Expand Down
Loading