diff --git a/src/storage/DataAccessorBasedStore.ts b/src/storage/DataAccessorBasedStore.ts index fab78e3725..570e9c6744 100644 --- a/src/storage/DataAccessorBasedStore.ts +++ b/src/storage/DataAccessorBasedStore.ts @@ -196,10 +196,10 @@ export class DataAccessorBasedStore implements ResourceStore { throw new ConflictHttpError(`${identifier.path} conflicts with existing path ${oldMetadata.identifier.value}`); } - const isContainer = this.isNewContainer(representation.metadata, identifier.path); // Solid, ยง3.1: "Paths ending with a slash denote a container resource." // https://solid.github.io/specification/protocol#uri-slash-semantics - if (isContainer !== isContainerIdentifier(identifier)) { + const isContainer = isContainerIdentifier(identifier); + if (!isContainer && this.isContainerType(representation.metadata)) { throw new BadRequestHttpError('Containers should have a `/` at the end of their path, resources should not.'); } @@ -473,12 +473,25 @@ export class DataAccessorBasedStore implements ResourceStore { * @param slug - Slug to use for the new URI. */ protected createURI(container: ResourceIdentifier, isContainer: boolean, slug?: string): ResourceIdentifier { + this.validateSlug(isContainer, slug); const base = ensureTrailingSlash(container.path); const name = (slug && this.cleanSlug(slug)) ?? uuid(); const suffix = isContainer ? '/' : ''; return { path: `${base}${name}${suffix}` }; } + /** + * Validates if the slug and headers are valid. + * Errors if slug exists, ends on slash, but ContainerType Link header is NOT present + * @param isContainer - Is the slug supposed to represent a container? + * @param slug - Is the requested slug (if any). + */ + protected validateSlug(isContainer: boolean, slug?: string): void { + if (slug && isContainerPath(slug) && !isContainer) { + throw new BadRequestHttpError('Only slugs used to create containers can end with a `/`.'); + } + } + /** * Clean http Slug to be compatible with the server. Makes sure there are no unwanted characters * e.g.: cleanslug('&%26') returns '%26%26' @@ -501,7 +514,7 @@ export class DataAccessorBasedStore implements ResourceStore { protected async createSafeUri(container: ResourceIdentifier, metadata: RepresentationMetadata): Promise { // Get all values needed for naming the resource - const isContainer = this.isNewContainer(metadata); + const isContainer = this.isContainerType(metadata); const slug = metadata.get(SOLID_HTTP.slug)?.value; metadata.removeAll(SOLID_HTTP.slug); @@ -526,16 +539,11 @@ export class DataAccessorBasedStore implements ResourceStore { /** * Checks if the given metadata represents a (potential) container, - * both based on the metadata and the URI. + * based on the metadata. * @param metadata - Metadata of the (new) resource. - * @param suffix - Suffix of the URI. Can be the full URI, but only the last part is required. */ - protected isNewContainer(metadata: RepresentationMetadata, suffix?: string): boolean { - if (this.hasContainerType(metadata.getAll(RDF.type))) { - return true; - } - const slug = suffix ?? metadata.get(SOLID_HTTP.slug)?.value; - return Boolean(slug && isContainerPath(slug)); + protected isContainerType(metadata: RepresentationMetadata): boolean { + return this.hasContainerType(metadata.getAll(RDF.type)); } /** diff --git a/test/integration/LdpHandlerWithoutAuth.test.ts b/test/integration/LdpHandlerWithoutAuth.test.ts index 12f4f82bd8..a704813ccd 100644 --- a/test/integration/LdpHandlerWithoutAuth.test.ts +++ b/test/integration/LdpHandlerWithoutAuth.test.ts @@ -265,14 +265,14 @@ describe.each(stores)('An LDP handler allowing all requests %s', (name, { storeC }); it('can create a container with a diamond identifier in the data.', async(): Promise => { - const slug = 'my-container'; + const slug = 'my-container/'; const body = '<> "My Container" .'; let response = await postResource(baseUrl, { isContainer: true, contentType: 'text/turtle', slug, body }); - expect(response.headers.get('location')).toBe(`${baseUrl}${slug}/`); + expect(response.headers.get('location')).toBe(`${baseUrl}${slug}`); // GET - const containerUrl = `${baseUrl}${slug}/`; + const containerUrl = `${baseUrl}${slug}`; response = await getResource(containerUrl); await expectQuads(response, [ diff --git a/test/integration/ServerFetch.test.ts b/test/integration/ServerFetch.test.ts index 5118be9c85..eecd1e7e02 100644 --- a/test/integration/ServerFetch.test.ts +++ b/test/integration/ServerFetch.test.ts @@ -1,5 +1,6 @@ import fetch from 'cross-fetch'; import type { App } from '../../src/init/App'; +import { LDP } from '../../src/util/Vocabularies'; import { getPort } from '../util/Util'; import { getDefaultVariables, getTestConfigPath, instantiateFromConfig } from './Config'; @@ -112,6 +113,7 @@ describe('A Solid server', (): void => { headers: { 'content-type': 'text/turtle', slug: 'containerPOST/', + link: `<${LDP.Container}>; rel="type"`, }, body: ' .', }); diff --git a/test/unit/storage/DataAccessorBasedStore.test.ts b/test/unit/storage/DataAccessorBasedStore.test.ts index 4c9bbf59b3..69f7abda9b 100644 --- a/test/unit/storage/DataAccessorBasedStore.test.ts +++ b/test/unit/storage/DataAccessorBasedStore.test.ts @@ -299,6 +299,31 @@ describe('A DataAccessorBasedStore', (): void => { }); }); + it('errors on a slug ending on / without Link rel:type Container header.', async(): Promise => { + const resourceID = { path: root }; + representation.metadata.removeAll(RDF.type); + representation.metadata.add(SOLID_HTTP.slug, 'noContainer/'); + representation.data = guardedStreamFrom([ `` ]); + const result = store.addResource(resourceID, representation); + + await expect(result).rejects.toThrow(BadRequestHttpError); + await expect(result).rejects + .toThrow('Only slugs used to create containers can end with a `/`.'); + }); + + it('creates a URI when the incoming slug does not end with /, ' + + 'but has a Link rel:type Container header.', async(): Promise => { + const resourceID = { path: root }; + representation.metadata.removeAll(RDF.type); + representation.metadata.add(RDF.type, LDP.terms.Container); + representation.metadata.add(SOLID_HTTP.slug, 'newContainer'); + representation.data = guardedStreamFrom([ `` ]); + const result = await store.addResource(resourceID, representation); + expect(result).toEqual({ + path: `${root}newContainer/`, + }); + }); + it('generates a new URI if adding the slug would create an existing URI.', async(): Promise => { const resourceID = { path: root }; representation.metadata.add(SOLID_HTTP.slug, 'newName'); @@ -389,7 +414,7 @@ describe('A DataAccessorBasedStore', (): void => { mock.mockRestore(); }); - it('will error if the ending slash does not match its resource type.', async(): Promise => { + it('will error if path does not end in slash and does not match its resource type.', async(): Promise => { const resourceID = { path: `${root}resource` }; representation.metadata.add(RDF.type, LDP.terms.Container); await expect(store.setRepresentation(resourceID, representation)).rejects.toThrow(