From f0596c2eb8b1f62b31fdcd26070ea9fbf8468a74 Mon Sep 17 00:00:00 2001 From: zg009 <103973669+zg009@users.noreply.github.com> Date: Tue, 28 Mar 2023 02:24:15 -0500 Subject: [PATCH] feat: Support conditions for GET/HEAD requests * fix: updated WrappedExpiringStorage tests and timer.unref calls * fix: removed finalizable configs and inheritors that only used timer * fix: updated test function to test setSafeInterval and timer.unref * fix: added NotModifiedHttpError class * fix: added 304 error test to HttpError test file * fix: 304 errors when making read request with matching ETag * Update src/util/errors/NotModifiedHttpError.ts Co-authored-by: Ted Thibodeau Jr * fix: updated tests * fix: try notMatchesEtag in test * fix: DataAccessorBasedStore test passes * fix: removed conditions check and added extra test --------- Co-authored-by: Ted Thibodeau Jr --- package-lock.json | 14 ++++++------- package.json | 2 +- src/storage/DataAccessorBasedStore.ts | 20 ++++++++++++------ src/util/errors/NotModifiedHttpError.ts | 14 +++++++++++++ test/integration/Conditions.test.ts | 21 +++++++++++++++++++ .../storage/DataAccessorBasedStore.test.ts | 14 +++++++++++++ test/unit/util/errors/HttpError.test.ts | 3 +++ 7 files changed, 74 insertions(+), 14 deletions(-) create mode 100644 src/util/errors/NotModifiedHttpError.ts diff --git a/package-lock.json b/package-lock.json index 5a9b6ad60..be67c5461 100644 --- a/package-lock.json +++ b/package-lock.json @@ -34,7 +34,7 @@ "@types/uuid": "^8.3.4", "@types/ws": "^8.5.3", "@types/yargs": "^17.0.10", - "arrayify-stream": "^2.0.0", + "arrayify-stream": "^2.0.1", "async-lock": "^1.3.2", "bcryptjs": "^2.4.3", "componentsjs": "^5.3.2", @@ -5150,9 +5150,9 @@ } }, "node_modules/arrayify-stream": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/arrayify-stream/-/arrayify-stream-2.0.0.tgz", - "integrity": "sha512-Z2NRtxpWQIz3NRA2bEZOziIungBH+fpsFFEolc5u8uVRheYitvsDNvejlfyh/hjZ9VyS9Ba62oY0zc5oa6Wu7g==" + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/arrayify-stream/-/arrayify-stream-2.0.1.tgz", + "integrity": "sha512-z8fB6PtmnewQpFB53piS2d1KlUi3BPMICH2h7leCOUXpQcwvZ4GbHHSpdKoUrgLMR6b4Qan/uDe1St3Ao3yIHg==" }, "node_modules/arrify": { "version": "1.0.1", @@ -19566,9 +19566,9 @@ } }, "arrayify-stream": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/arrayify-stream/-/arrayify-stream-2.0.0.tgz", - "integrity": "sha512-Z2NRtxpWQIz3NRA2bEZOziIungBH+fpsFFEolc5u8uVRheYitvsDNvejlfyh/hjZ9VyS9Ba62oY0zc5oa6Wu7g==" + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/arrayify-stream/-/arrayify-stream-2.0.1.tgz", + "integrity": "sha512-z8fB6PtmnewQpFB53piS2d1KlUi3BPMICH2h7leCOUXpQcwvZ4GbHHSpdKoUrgLMR6b4Qan/uDe1St3Ao3yIHg==" }, "arrify": { "version": "1.0.1", diff --git a/package.json b/package.json index fdfcf9ad8..f9a58083c 100644 --- a/package.json +++ b/package.json @@ -124,7 +124,7 @@ "@types/uuid": "^8.3.4", "@types/ws": "^8.5.3", "@types/yargs": "^17.0.10", - "arrayify-stream": "^2.0.0", + "arrayify-stream": "^2.0.1", "async-lock": "^1.3.2", "bcryptjs": "^2.4.3", "componentsjs": "^5.3.2", diff --git a/src/storage/DataAccessorBasedStore.ts b/src/storage/DataAccessorBasedStore.ts index 15452a848..bc9860da5 100644 --- a/src/storage/DataAccessorBasedStore.ts +++ b/src/storage/DataAccessorBasedStore.ts @@ -7,6 +7,7 @@ import { BasicRepresentation } from '../http/representation/BasicRepresentation' import type { Patch } from '../http/representation/Patch'; import type { Representation } from '../http/representation/Representation'; import { RepresentationMetadata } from '../http/representation/RepresentationMetadata'; +import type { RepresentationPreferences } from '../http/representation/RepresentationPreferences'; import type { ResourceIdentifier } from '../http/representation/ResourceIdentifier'; import { getLoggerFor } from '../logging/LogUtil'; import { INTERNAL_QUADS } from '../util/ContentTypes'; @@ -17,6 +18,7 @@ import { ForbiddenHttpError } from '../util/errors/ForbiddenHttpError'; import { MethodNotAllowedHttpError } from '../util/errors/MethodNotAllowedHttpError'; import { NotFoundHttpError } from '../util/errors/NotFoundHttpError'; import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError'; +import { NotModifiedHttpError } from '../util/errors/NotModifiedHttpError'; import { PreconditionFailedHttpError } from '../util/errors/PreconditionFailedHttpError'; import type { IdentifierStrategy } from '../util/identifiers/IdentifierStrategy'; import { concat } from '../util/IterableUtil'; @@ -103,7 +105,8 @@ export class DataAccessorBasedStore implements ResourceStore { } } - public async getRepresentation(identifier: ResourceIdentifier): Promise { + public async getRepresentation(identifier: ResourceIdentifier, + preferences?: RepresentationPreferences, conditions?: Conditions): Promise { this.validateIdentifier(identifier); let isMetadata = false; @@ -116,6 +119,8 @@ export class DataAccessorBasedStore implements ResourceStore { let metadata = await this.accessor.getMetadata(identifier); let representation: Representation; + this.validateConditions(true, conditions, metadata); + // Potentially add auxiliary related metadata // Solid, §4.3: "Clients can discover auxiliary resources associated with a subject resource by making an HTTP HEAD // or GET request on the target URL, and checking the HTTP Link header with the rel parameter" @@ -181,7 +186,7 @@ export class DataAccessorBasedStore implements ResourceStore { throw new MethodNotAllowedHttpError([ 'POST' ], 'The given path is not a container.'); } - this.validateConditions(conditions, parentMetadata); + this.validateConditions(false, conditions, parentMetadata); // Solid, §5.1: "Servers MAY allow clients to suggest the URI of a resource created through POST, // using the HTTP Slug header as defined in [RFC5023]. @@ -246,7 +251,7 @@ export class DataAccessorBasedStore implements ResourceStore { await this.accessor.canHandle(representation); } - this.validateConditions(conditions, oldMetadata); + this.validateConditions(false, conditions, oldMetadata); if (this.metadataStrategy.isAuxiliaryIdentifier(identifier)) { return await this.writeMetadata(identifier, representation); @@ -268,7 +273,7 @@ export class DataAccessorBasedStore implements ResourceStore { } } - this.validateConditions(conditions, metadata); + this.validateConditions(false, conditions, metadata); } throw new NotImplementedHttpError('Patches are not supported by the default store.'); @@ -309,7 +314,7 @@ export class DataAccessorBasedStore implements ResourceStore { throw new ConflictHttpError('Can only delete empty containers.'); } - this.validateConditions(conditions, metadata); + this.validateConditions(false, conditions, metadata); // Solid, §5.4: "When a contained resource is deleted, // the server MUST also delete the associated auxiliary resources" @@ -347,10 +352,13 @@ export class DataAccessorBasedStore implements ResourceStore { /** * Verify if the given metadata matches the conditions. */ - protected validateConditions(conditions?: Conditions, metadata?: RepresentationMetadata): void { + protected validateConditions(read: boolean, conditions?: Conditions, metadata?: RepresentationMetadata): void { // The 412 (Precondition Failed) status code indicates // that one or more conditions given in the request header fields evaluated to false when tested on the server. if (conditions && !conditions.matchesMetadata(metadata)) { + if (read) { + throw new NotModifiedHttpError(); + } throw new PreconditionFailedHttpError(); } } diff --git a/src/util/errors/NotModifiedHttpError.ts b/src/util/errors/NotModifiedHttpError.ts new file mode 100644 index 000000000..9504de3b7 --- /dev/null +++ b/src/util/errors/NotModifiedHttpError.ts @@ -0,0 +1,14 @@ +import type { HttpErrorOptions } from './HttpError'; +import { generateHttpErrorClass } from './HttpError'; + +// eslint-disable-next-line @typescript-eslint/naming-convention +const BaseHttpError = generateHttpErrorClass(304, 'NotModifiedHttpError'); + +/** + * An error is thrown when a request conflicts with the current state of the server. + */ +export class NotModifiedHttpError extends BaseHttpError { + public constructor(message?: string, options?: HttpErrorOptions) { + super(message, options); + } +} diff --git a/test/integration/Conditions.test.ts b/test/integration/Conditions.test.ts index 4980a7480..a57cf83ee 100644 --- a/test/integration/Conditions.test.ts +++ b/test/integration/Conditions.test.ts @@ -170,6 +170,27 @@ describe.each(stores)('A server supporting conditions with %s', (name, { storeCo expect(await deleteResource(documentUrl!)).toBeUndefined(); }); + it('throws 304 error if "if-none-match" header matches and request type is GET or HEAD.', async(): Promise => { + // GET root ETag + let response = await getResource(baseUrl); + const eTag = response.headers.get('ETag'); + expect(typeof eTag).toBe('string'); + + // GET fails because of header + response = await fetch(baseUrl, { + method: 'GET', + headers: { 'if-none-match': eTag! }, + }); + expect(response.status).toBe(304); + + // HEAD fails because of header + response = await fetch(baseUrl, { + method: 'HEAD', + headers: { 'if-none-match': eTag! }, + }); + expect(response.status).toBe(304); + }); + it('prevents operations if the "if-unmodified-since" header is before the modified date.', async(): Promise => { const documentUrl = `${baseUrl}document3.txt`; // PUT diff --git a/test/unit/storage/DataAccessorBasedStore.test.ts b/test/unit/storage/DataAccessorBasedStore.test.ts index 5e1ecd2de..8d9f3d543 100644 --- a/test/unit/storage/DataAccessorBasedStore.test.ts +++ b/test/unit/storage/DataAccessorBasedStore.test.ts @@ -19,6 +19,7 @@ import { ForbiddenHttpError } from '../../../src/util/errors/ForbiddenHttpError' import { MethodNotAllowedHttpError } from '../../../src/util/errors/MethodNotAllowedHttpError'; import { NotFoundHttpError } from '../../../src/util/errors/NotFoundHttpError'; import { NotImplementedHttpError } from '../../../src/util/errors/NotImplementedHttpError'; +import { NotModifiedHttpError } from '../../../src/util/errors/NotModifiedHttpError'; import { PreconditionFailedHttpError } from '../../../src/util/errors/PreconditionFailedHttpError'; import type { Guarded } from '../../../src/util/GuardedStream'; import { SingleRootIdentifierStrategy } from '../../../src/util/identifiers/SingleRootIdentifierStrategy'; @@ -26,6 +27,7 @@ import { trimTrailingSlashes } from '../../../src/util/PathUtil'; import { guardedStreamFrom } from '../../../src/util/StreamUtil'; import { CONTENT_TYPE, SOLID_HTTP, LDP, PIM, RDF, SOLID_META, DC, SOLID_AS, AS } from '../../../src/util/Vocabularies'; import { SimpleSuffixStrategy } from '../../util/SimpleSuffixStrategy'; + const { namedNode, quad, literal } = DataFactory; const GENERATED_PREDICATE = namedNode('generated'); @@ -216,6 +218,18 @@ describe('A DataAccessorBasedStore', (): void => { ); expect(result.metadata.contentType).toBe(INTERNAL_QUADS); }); + + it('throws a 304 if the request is a read type error.', async(): Promise => { + const resourceID = { path: root }; + const conditions = new BasicConditions({ notMatchesETag: [ '*' ]}); + await expect(store.getRepresentation(resourceID, undefined, conditions)).rejects.toThrow(NotModifiedHttpError); + }); + + it('has conditions but throws no error.', async(): Promise => { + const resourceID = { path: root }; + const conditions = new BasicConditions({ matchesETag: [ '*' ]}); + await expect(store.getRepresentation(resourceID, undefined, conditions)).resolves.toBeTruthy(); + }); }); describe('adding a Resource', (): void => { diff --git a/test/unit/util/errors/HttpError.test.ts b/test/unit/util/errors/HttpError.test.ts index 46d9a8773..4b2726dd2 100644 --- a/test/unit/util/errors/HttpError.test.ts +++ b/test/unit/util/errors/HttpError.test.ts @@ -9,16 +9,19 @@ import { InternalServerError } from '../../../../src/util/errors/InternalServerE import { MethodNotAllowedHttpError } from '../../../../src/util/errors/MethodNotAllowedHttpError'; import { NotFoundHttpError } from '../../../../src/util/errors/NotFoundHttpError'; import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError'; +import { NotModifiedHttpError } from '../../../../src/util/errors/NotModifiedHttpError'; import { PayloadHttpError } from '../../../../src/util/errors/PayloadHttpError'; import { PreconditionFailedHttpError } from '../../../../src/util/errors/PreconditionFailedHttpError'; import { UnauthorizedHttpError } from '../../../../src/util/errors/UnauthorizedHttpError'; import { UnprocessableEntityHttpError } from '../../../../src/util/errors/UnprocessableEntityHttpError'; import { UnsupportedMediaTypeHttpError } from '../../../../src/util/errors/UnsupportedMediaTypeHttpError'; import { SOLID_ERROR } from '../../../../src/util/Vocabularies'; + const { literal, namedNode, quad } = DataFactory; describe('HttpError', (): void => { const errors: [string, number, HttpErrorClass][] = [ + [ 'NotModifiedHttpError', 304, NotModifiedHttpError ], [ 'BadRequestHttpError', 400, BadRequestHttpError ], [ 'UnauthorizedHttpError', 401, UnauthorizedHttpError ], [ 'ForbiddenHttpError', 403, ForbiddenHttpError ],