Mirroristas/CommunitySolidServer
2
0
Fork 0
mirror of https://github.com/CommunitySolidServer/CommunitySolidServer.git synced 2024-10-03 14:55:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

Correctly handle slugs in POST requests

Browse Source 
* bug: correctly handle slug in POST request

* bug: disallow slashes in slug + modified tests

* fix: fixed tests to work with PUT instead of POST+slug

* fix: fixed tests failing in ci

* fix: adapted to reviews

* fix: adapted to review
This commit is contained in:
Arthur Joppart 2021-02-24 12:03:41 +01:00 committed by GitHub
parent 894d4589d9
commit 28c0eb7e88
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 154 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/storage/DataAccessorBasedStore.ts
View File

@ -22,6 +22,7 @@ import {
isContainerIdentifier,
isContainerPath,
trimTrailingSlashes,
toCanonicalUriPath,
} from '../util/PathUtil';
import { parseQuads } from '../util/QuadUtil';
import { generateResourceQuads } from '../util/ResourceUtil';
@ -347,8 +348,22 @@ export class DataAccessorBasedStore implements ResourceStore {
* @param slug - Slug to use for the new URI.
*/
protected createURI(container: ResourceIdentifier, isContainer: boolean, slug?: string): ResourceIdentifier {
return { path:
`${ensureTrailingSlash(container.path)}${slug ? trimTrailingSlashes(slug) : uuid()}${isContainer ? '/' : ''}` };
const base = ensureTrailingSlash(container.path);
const name = (slug && this.cleanSlug(slug)) ?? uuid();
const suffix = isContainer ? '/' : '';
return { path: `${base}${name}${suffix}` };
}
/**
* Clean http Slug to be compatible with the server. Makes sure there are no unwanted characters
* e.g.: cleanslug('&%26') returns '%26%26'
* @param slug - the slug to clean
*/
protected cleanSlug(slug: string): string {
if (/\/[^/]/u.test(slug)) {
throw new BadRequestHttpError('Slugs should not contain slashes');
}
return toCanonicalUriPath(trimTrailingSlashes(slug));
}
/**

39
test/integration/LdpHandlerWithAuth.test.ts
View File

@ -64,22 +64,23 @@ describe.each(stores)('An LDP handler with auth using %s', (name, { storeUrn, te
await aclHelper.setSimpleAcl({ read: true, write: true, append: true, control: false }, 'agent');
// Create file
const filePath = 'testfile2.txt';
const fileUrl = `${BASE}/${filePath}`;
let response = await resourceHelper.createResource(
'../assets/testfile2.txt', 'testfile2.txt', 'text/plain',
'../assets/testfile2.txt', filePath, 'text/plain',
);
const id = response._getHeaders().location;
// Get file
response = await resourceHelper.getResource(id);
response = await resourceHelper.getResource(fileUrl);
expect(response.statusCode).toBe(200);
expect(response._getBuffer().toString()).toContain('TESTFILE2');
expect(response.getHeaders().link).toContain(`<${LDP.Resource}>; rel="type"`);
expect(response.getHeaders().link).toContain(`<${id}.acl>; rel="acl"`);
expect(response.getHeaders().link).toContain(`<${fileUrl}.acl>; rel="acl"`);
expect(response.getHeaders()['wac-allow']).toBe('user="read write append",public="read write append"');
// DELETE file
await resourceHelper.deleteResource(id);
await resourceHelper.shouldNotExist(id);
await resourceHelper.deleteResource(fileUrl);
await resourceHelper.shouldNotExist(fileUrl);
});
it('can not add a file to the store if not allowed.', async(): Promise<void> => {
@ -87,8 +88,9 @@ describe.each(stores)('An LDP handler with auth using %s', (name, { storeUrn, te
await aclHelper.setSimpleAcl({ read: true, write: true, append: true, control: false }, 'authenticated');
// Try to create file
const filePath = 'testfile2.txt';
const response = await resourceHelper.createResource(
'../assets/testfile2.txt', 'testfile2.txt', 'text/plain', true,
'../assets/testfile2.txt', filePath, 'text/plain', true,
);
expect(response.statusCode).toBe(401);
});
@ -98,8 +100,9 @@ describe.each(stores)('An LDP handler with auth using %s', (name, { storeUrn, te
await aclHelper.setSimpleAcl({ read: true, write: false, append: false, control: false }, 'agent');
// Try to create file
const filePath = 'testfile2.txt';
let response = await resourceHelper.createResource(
'../assets/testfile2.txt', 'testfile2.txt', 'text/plain', true,
'../assets/testfile2.txt', filePath, 'text/plain', true,
);
expect(response.statusCode).toBe(401);
@ -120,17 +123,21 @@ describe.each(stores)('An LDP handler with auth using %s', (name, { storeUrn, te
await aclHelper.setSimpleAcl({ read: true, write: false, append: true, control: false }, 'agent');
// Add a file
let response = await resourceHelper.createResource(
'../assets/testfile2.txt', 'testfile2.txt', 'text/plain', true,
const filePath = 'testfile2.txt';
let response = await resourceHelper.performRequestWithBody(
new URL(`${BASE}/`),
'POST',
{
'content-type': 'text/plain',
'transfer-encoding': 'chunked',
slug: filePath,
},
Buffer.from('data'),
);
expect(response.statusCode).toBe(201);
const id = response._getHeaders().location;
response = await resourceHelper.performRequestWithBody(
new URL(id),
'PUT',
{ 'content-type': 'text/plain', 'transfer-encoding': 'chunked' },
Buffer.from('data'),
response = await resourceHelper.createResource(
'../assets/testfile2.txt', filePath, 'text/plain', true,
);
expect(response.statusCode).toBe(401);
});

154
test/integration/LdpHandlerWithoutAuth.test.ts
View File

@ -90,187 +90,203 @@ describe.each(stores)('An LDP handler without auth using %s', (name, { storeUrn,
it('can add a file to the store, read it and delete it.', async():
Promise<void> => {
// POST
const filePath = 'testfile0.txt';
const fileUrl = `${BASE}/${filePath}`;
// PUT
let response = await resourceHelper.createResource(
'../assets/testfile0.txt', 'testfile0.txt', 'text/plain',
'../assets/testfile0.txt', filePath, 'text/plain',
);
const id = response._getHeaders().location;
// GET
response = await resourceHelper.getResource(id);
response = await resourceHelper.getResource(fileUrl);
expect(response.statusCode).toBe(200);
expect(response._getBuffer().toString()).toContain('TESTFILE0');
expect(response.getHeaders().link).toContain(`<${LDP.Resource}>; rel="type"`);
expect(response.getHeaders().link).toContain(`<${id}.acl>; rel="acl"`);
expect(response.getHeaders().link).toContain(`<${fileUrl}.acl>; rel="acl"`);
expect(response.getHeaders()['accept-patch']).toBe('application/sparql-update');
expect(response.getHeaders()['ms-author-via']).toBe('SPARQL');
// DELETE
await resourceHelper.deleteResource(id);
await resourceHelper.shouldNotExist(id);
await resourceHelper.deleteResource(fileUrl);
await resourceHelper.shouldNotExist(fileUrl);
});
it('can add and overwrite a file.', async(): Promise<void> => {
const filePath = 'file.txt';
const fileUrl = `${BASE}/${filePath}`;
// PUT
let response = await resourceHelper.createResource(
'../assets/testfile0.txt', 'file.txt', 'text/plain',
'../assets/testfile0.txt', filePath, 'text/plain',
);
const id = response._getHeaders().location;
// GET
response = await resourceHelper.getResource(id);
response = await resourceHelper.getResource(fileUrl);
expect(response.statusCode).toBe(200);
expect(response._getBuffer().toString()).toContain('TESTFILE0');
expect(response.getHeaders().link).toContain(`<${LDP.Resource}>; rel="type"`);
expect(response.getHeaders().link).toContain(`<${id}.acl>; rel="acl"`);
expect(response.getHeaders().link).toContain(`<${fileUrl}.acl>; rel="acl"`);
// PUT
response = await resourceHelper.replaceResource(
'../assets/testfile1.txt', id, 'text/plain',
'../assets/testfile1.txt', fileUrl, 'text/plain',
);
// GET
response = await resourceHelper.getResource(id);
response = await resourceHelper.getResource(fileUrl);
expect(response.statusCode).toBe(200);
expect(response._getBuffer().toString()).toContain('TESTFILE1');
expect(response.getHeaders().link).toContain(`<${LDP.Resource}>; rel="type"`);
expect(response.getHeaders().link).toContain(`<${id}.acl>; rel="acl"`);
expect(response.getHeaders().link).toContain(`<${fileUrl}.acl>; rel="acl"`);
// DELETE
await resourceHelper.deleteResource(id);
await resourceHelper.shouldNotExist(id);
await resourceHelper.deleteResource(fileUrl);
await resourceHelper.shouldNotExist(fileUrl);
});
it('can create a folder and delete it.', async(): Promise<void> => {
// POST
let response = await resourceHelper.createContainer('secondfolder/');
const id = response._getHeaders().location;
const containerPath = 'secondfolder/';
const containerUrl = `${BASE}/${containerPath}`;
// PUT
let response = await resourceHelper.createContainer(containerPath);
// GET
response = await resourceHelper.getContainer(id);
response = await resourceHelper.getContainer(containerUrl);
expect(response.statusCode).toBe(200);
expect(response.getHeaders().link).toContain(`<${LDP.Container}>; rel="type"`);
expect(response.getHeaders().link).toContain(`<${LDP.BasicContainer}>; rel="type"`);
expect(response.getHeaders().link).toContain(`<${LDP.Resource}>; rel="type"`);
expect(response.getHeaders().link).toContain(`<${id}.acl>; rel="acl"`);
expect(response.getHeaders().link).toContain(`<${containerUrl}.acl>; rel="acl"`);
// DELETE
await resourceHelper.deleteResource(id);
await resourceHelper.shouldNotExist(id);
await resourceHelper.deleteResource(containerUrl);
await resourceHelper.shouldNotExist(containerUrl);
});
it('can make a folder and put a file in it.', async(): Promise<void> => {
// Create folder
await resourceHelper.createContainer('testfolder0/');
const containerPath = 'testfolder0/';
const containerUrl = `${BASE}/${containerPath}`;
await resourceHelper.createContainer(containerPath);
// Create file
const filePath = 'testfolder0/testfile0.txt';
const fileUrl = `${BASE}/${filePath}`;
let response = await resourceHelper.createResource(
'../assets/testfile0.txt', 'testfolder0/testfile0.txt', 'text/plain',
'../assets/testfile0.txt', filePath, 'text/plain',
);
const id = response._getHeaders().location;
// GET File
response = await resourceHelper.getResource(id);
response = await resourceHelper.getResource(fileUrl);
expect(response.statusCode).toBe(200);
expect(response.getHeaders().link).toContain(`<${LDP.Resource}>; rel="type"`);
expect(response.getHeaders().link).toContain(`<${id}.acl>; rel="acl"`);
expect(response.getHeaders().link).toContain(`<${fileUrl}.acl>; rel="acl"`);
// DELETE
await resourceHelper.deleteResource(id);
await resourceHelper.shouldNotExist(id);
await resourceHelper.deleteResource('http://test.com/testfolder0/');
await resourceHelper.shouldNotExist('http://test.com/testfolder0/');
await resourceHelper.deleteResource(fileUrl);
await resourceHelper.shouldNotExist(fileUrl);
await resourceHelper.deleteResource(containerUrl);
await resourceHelper.shouldNotExist(containerUrl);
});
it('cannot remove a folder when the folder contains a file.', async(): Promise<void> => {
// Create folder
let response = await resourceHelper.createContainer('testfolder1/');
const folderId = response._getHeaders().location;
const containerPath = 'testfolder1/';
const containerUrl = `${BASE}/${containerPath}`;
let response = await resourceHelper.createContainer(containerPath);
// Create file
const filePath = 'testfolder1/testfile0.txt';
const fileUrl = `${BASE}/${filePath}`;
await resourceHelper.createResource(
'../assets/testfile0.txt', 'testfolder1/testfile0.txt', 'text/plain',
'../assets/testfile0.txt', filePath, 'text/plain',
);
// Try DELETE folder
response = await resourceHelper.performRequest(new URL(folderId), 'DELETE', {});
response = await resourceHelper.performRequest(new URL(containerUrl), 'DELETE', {});
expect(response.statusCode).toBe(409);
expect(response._getData()).toContain('ConflictHttpError: Can only delete empty containers.');
// DELETE
await resourceHelper.deleteResource('http://test.com/testfolder1/testfile0.txt');
await resourceHelper.shouldNotExist('http://test.com/testfolder1/testfile0.txt');
await resourceHelper.deleteResource(folderId);
await resourceHelper.shouldNotExist(folderId);
await resourceHelper.deleteResource(fileUrl);
await resourceHelper.shouldNotExist(fileUrl);
await resourceHelper.deleteResource(containerUrl);
await resourceHelper.shouldNotExist(containerUrl);
});
it('cannot remove a folder when the folder contains a subfolder.', async(): Promise<void> => {
// Create folder
let response = await resourceHelper.createContainer('testfolder2/');
const folderId = response._getHeaders().location;
const containerPath = 'testfolder2/';
const containerUrl = `${BASE}/${containerPath}`;
let response = await resourceHelper.createContainer(containerPath);
// Create subfolder
response = await resourceHelper.createContainer('testfolder2/subfolder0');
const subFolderId = response._getHeaders().location;
const subContainerPath = `${containerPath}subfolder0/`;
const subContainerUrl = `${BASE}/${subContainerPath}`;
response = await resourceHelper.createContainer(subContainerPath);
// Try DELETE folder
response = await resourceHelper.performRequest(new URL(folderId), 'DELETE', {});
response = await resourceHelper.performRequest(new URL(containerUrl), 'DELETE', {});
expect(response.statusCode).toBe(409);
expect(response._getData()).toContain('ConflictHttpError: Can only delete empty containers.');
// DELETE
await resourceHelper.deleteResource(subFolderId);
await resourceHelper.shouldNotExist(subFolderId);
await resourceHelper.deleteResource(folderId);
await resourceHelper.shouldNotExist(folderId);
await resourceHelper.deleteResource(subContainerUrl);
await resourceHelper.shouldNotExist(subContainerUrl);
await resourceHelper.deleteResource(containerUrl);
await resourceHelper.shouldNotExist(containerUrl);
});
it('can read the contents of a folder.', async(): Promise<void> => {
// Create folder
let response = await resourceHelper.createContainer('testfolder3/');
const folderId = response._getHeaders().location;
const containerPath = 'testfolder3/';
const containerUrl = `${BASE}/${containerPath}`;
let response = await resourceHelper.createContainer(containerPath);
// Create subfolder
const subContainerPath = `${containerPath}subfolder0/`;
const subContainerUrl = `${BASE}/${subContainerPath}`;
response = await resourceHelper.createContainer('testfolder3/subfolder0/');
const subFolderId = response._getHeaders().location;
// Create file
const filePath = `${containerPath}testfile0.txt`;
const fileUrl = `${BASE}/${filePath}`;
response = await resourceHelper.createResource(
'../assets/testfile0.txt', 'testfolder3/testfile0.txt', 'text/plain',
'../assets/testfile0.txt', filePath, 'text/plain',
);
const fileId = response._getHeaders().location;
response = await resourceHelper.getContainer(folderId);
response = await resourceHelper.getContainer(containerUrl);
expect(response.statusCode).toBe(200);
expect(response._getData()).toContain('<http://www.w3.org/ns/ldp#contains> <http://test.com/testfolder3/subfolder0/> .');
expect(response._getData()).toContain('<http://www.w3.org/ns/ldp#contains> <http://test.com/testfolder3/testfile0.txt> .');
expect(response._getData()).toContain(`<http://www.w3.org/ns/ldp#contains> <${subContainerUrl}> .`);
expect(response._getData()).toContain(`<http://www.w3.org/ns/ldp#contains> <${fileUrl}> .`);
expect(response.getHeaders().link).toContain(`<${LDP.Container}>; rel="type"`);
expect(response.getHeaders().link).toContain(`<${LDP.BasicContainer}>; rel="type"`);
expect(response.getHeaders().link).toContain(`<${LDP.Resource}>; rel="type"`);
expect(response.getHeaders().link).toContain(`<${folderId}.acl>; rel="acl"`);
expect(response.getHeaders().link).toContain(`<${containerUrl}.acl>; rel="acl"`);
// DELETE
await resourceHelper.deleteResource(fileId);
await resourceHelper.shouldNotExist(fileId);
await resourceHelper.deleteResource(subFolderId);
await resourceHelper.shouldNotExist(subFolderId);
await resourceHelper.deleteResource(folderId);
await resourceHelper.shouldNotExist(folderId);
await resourceHelper.deleteResource(fileUrl);
await resourceHelper.shouldNotExist(fileUrl);
await resourceHelper.deleteResource(subContainerUrl);
await resourceHelper.shouldNotExist(subContainerUrl);
await resourceHelper.deleteResource(containerUrl);
await resourceHelper.shouldNotExist(containerUrl);
});
it('can upload and delete a image.', async(): Promise<void> => {
const filePath = 'image.png';
const fileUrl = `${BASE}/${filePath}`;
let response = await resourceHelper.createResource(
'../assets/testimage.png', 'image.png', 'image/png',
'../assets/testimage.png', filePath, 'image/png',
);
const fileId = response._getHeaders().location;
// GET
response = await resourceHelper.getResource(fileId);
response = await resourceHelper.getResource(fileUrl);
expect(response.statusCode).toBe(200);
expect(response._getHeaders()['content-type']).toBe('image/png');
// DELETE
await resourceHelper.deleteResource(fileId);
await resourceHelper.shouldNotExist(fileId);
await resourceHelper.deleteResource(fileUrl);
await resourceHelper.shouldNotExist(fileUrl);
});
it('can create a container with a diamond identifier in the data.', async(): Promise<void> => {

5
test/integration/SparqlStorage.test.ts
View File

@ -35,9 +35,8 @@ describeIf('docker', 'A server with a SPARQL endpoint as storage', (): void => {
it('can add a Turtle file to the store.', async():
Promise<void> => {
// POST
// PUT
const response = await resourceHelper.createResource('../assets/person.ttl', 'person', 'text/turtle');
const id = response._getHeaders().location;
expect(id).toBeTruthy();
expect(response).toBeTruthy();
});
});

19
test/unit/storage/DataAccessorBasedStore.test.ts
View File

@ -291,6 +291,24 @@ describe('A DataAccessorBasedStore', (): void => {
});
});
it('generates http://test.com/%26%26 when slug is &%26.', async(): Promise<void> => {
const resourceID = { path: root };
representation.metadata.removeAll(RDF.type);
representation.metadata.add(HTTP.slug, '&%26');
const result = await store.addResource(resourceID, representation);
expect(result).toEqual({ path: `${root}%26%26` });
});
it('errors if the slug contains a slash.', async(): Promise<void> => {
const resourceID = { path: root };
representation.metadata.removeAll(RDF.type);
representation.data = guardedStreamFrom([ `` ]);
representation.metadata.add(HTTP.slug, 'sla/sh/es');
const result = store.addResource(resourceID, representation);
await expect(result).rejects.toThrow(BadRequestHttpError);
await expect(result).rejects.toThrow('Slugs should not contain slashes');
});
it('errors if the slug would cause an auxiliary resource URI to be generated.', async(): Promise<void> => {
const resourceID = { path: root };
representation.metadata.removeAll(RDF.type);
@ -559,3 +577,4 @@ describe('A DataAccessorBasedStore', (): void => {
});
});
});

21
test/util/TestHelpers.ts
View File

@ -2,7 +2,6 @@ import { EventEmitter } from 'events';
import { promises as fs } from 'fs';
import type { IncomingHttpHeaders } from 'http';
import { Readable } from 'stream';
import * as url from 'url';
import type { MockResponse } from 'node-mocks-http';
import { createResponse } from 'node-mocks-http';
import type { ResourceStore, PermissionSet, HttpHandler, HttpRequest } from '../../src/';
@ -93,24 +92,22 @@ export class ResourceHelper {
return response;
}
public async createResource(fileLocation: string, slug: string, contentType: string, mayFail = false):
public async createResource(fileLocation: string, path: string, contentType: string, mayFail = false):
Promise<MockResponse<any>> {
const fileData = await fs.readFile(
joinFilePath(__dirname, fileLocation),
);
const response: MockResponse<any> = await this.performRequestWithBody(
this.baseUrl,
'POST',
new URL(path, this.baseUrl),
'PUT',
{ 'content-type': contentType,
slug,
'transfer-encoding': 'chunked' },
fileData,
);
if (!mayFail) {
expect(response.statusCode).toBe(201);
expect(response.statusCode).toBe(205);
expect(response._getData()).toHaveLength(0);
expect(response._getHeaders().location).toContain(url.format(this.baseUrl));
}
return response;
}
@ -153,20 +150,18 @@ export class ResourceHelper {
return response;
}
public async createContainer(slug: string): Promise<MockResponse<any>> {
public async createContainer(path: string): Promise<MockResponse<any>> {
const response: MockResponse<any> = await this.performRequest(
this.baseUrl,
'POST',
new URL(path, this.baseUrl),
'PUT',
{
slug,
link: '<http://www.w3.org/ns/ldp#Container>; rel="type"',
'content-type': 'text/turtle',
'transfer-encoding': 'chunked',
},
);
expect(response.statusCode).toBe(201);
expect(response.statusCode).toBe(205);
expect(response._getData()).toHaveLength(0);
expect(response._getHeaders().location).toContain(url.format(this.baseUrl));
return response;
}