Skip to content

Commit

Permalink
feat: Add Allow Header on 405 (#560)
Browse files Browse the repository at this point in the history
* chore: Fix Typos

* test: Add Test Case for Allow Header

The Allow header must be sent if the server responds with 405 to
indicate which request methods can be used.

See #467

* feat: Add Allow Header for 405 - Method Not Allowed

Adds Allow header to 405 error. This is required by RFC 7231
and indicates which request methods can be used.

Resolves #467

* doc: Adjust Error Handler in NestJS Example

When using a custom error handler like in the NestJS example, one needs
to set the headers on the response explicitly. The same is true when
using a custom express error handler.
  • Loading branch information
ahilke authored Mar 13, 2021
1 parent 09980a3 commit 45a40b7
Show file tree
Hide file tree
Showing 8 changed files with 127 additions and 29 deletions.
5 changes: 4 additions & 1 deletion examples/9-nestjs/src/filters/openapi-exception.filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export class OpenApiExceptionFilter implements ExceptionFilter {
const ctx = host.switchToHttp();
const response = ctx.getResponse<Response>();

response.status(error.status).json(error);
response.status(error.status).header(error.headers).json(error);
}
}

Expand All @@ -22,4 +22,7 @@ interface ValidationError {
}>;
path?: string;
name: string;
headers: {
[header: string]: string;
};
}
28 changes: 17 additions & 11 deletions src/framework/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -533,33 +533,37 @@ export interface ValidationErrorItem {
error_code?: string;
}

interface ErrorHeaders {
Allow?: string;
}

export class HttpError extends Error implements ValidationError {
status!: number;
message!: string;
errors!: ValidationErrorItem[];
path?: string;
name!: string;
message!: string;
headers?: ErrorHeaders;
errors!: ValidationErrorItem[];
constructor(err: {
status: number;
path: string;
name: string;
message?: string;
headers?: ErrorHeaders;
errors?: ValidationErrorItem[];
}) {
super(err.name);
this.name = err.name;
this.status = err.status;
this.path = err.path;
this.message = err.message;
this.errors =
err.errors == undefined
? [
{
path: err.path,
message: err.message,
},
]
: err.errors;
this.headers = err.headers;
this.errors = err.errors ?? [
{
path: err.path,
message: err.message,
},
];
}

public static create(err: {
Expand Down Expand Up @@ -634,13 +638,15 @@ export class MethodNotAllowed extends HttpError {
constructor(err: {
path: string;
message?: string;
headers?: ErrorHeaders;
overrideStatus?: number;
}) {
super({
status: err.overrideStatus || 405,
path: err.path,
name: 'Method Not Allowed',
message: err.message,
headers: err.headers,
});
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/middlewares/openapi.metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
OpenApiRequestMetadata,
OpenAPIV3,
} from '../framework/types';
import { httpMethods } from './parsers/schema.preprocessor';

export function applyOpenApiMetadata(
openApiContext: OpenApiContext,
Expand All @@ -30,6 +31,11 @@ export function applyOpenApiMetadata(
throw new MethodNotAllowed({
path: req.path,
message: `${req.method} method not allowed`,
headers: {
Allow: Object.keys(openApiContext.openApiRouteMap[openApiRoute])
.filter((key) => httpMethods.has(key.toLowerCase()))
.join(', '),
},
});
}
req.openapi = {
Expand Down
14 changes: 7 additions & 7 deletions src/middlewares/openapi.request.validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,20 +117,20 @@ export class RequestValidator {
req.params = openapi.pathParams ?? req.params;
}

const schemaPoperties = validator.allSchemaProperties;
const schemaProperties = validator.allSchemaProperties;
const mutator = new RequestParameterMutator(
this.ajv,
apiDoc,
path,
schemaPoperties,
schemaProperties,
);

mutator.modifyRequest(req);

if (!allowUnknownQueryParameters) {
this.processQueryParam(
req.query,
schemaPoperties.query,
schemaProperties.query,
securityQueryParam,
);
}
Expand All @@ -151,15 +151,15 @@ export class RequestValidator {
};
const schemaBody = <any>validator?.schemaBody;
const discriminator = schemaBody?.properties?.body?._discriminator;
const discriminatorValdiator = this.discriminatorValidator(
const discriminatorValidator = this.discriminatorValidator(
req,
discriminator,
);

const validatorBody = discriminatorValdiator ?? validator.validatorBody;
const validatorBody = discriminatorValidator ?? validator.validatorBody;
const valid = validator.validatorGeneral(data);
const validBody = validatorBody(
discriminatorValdiator ? data.body : data,
discriminatorValidator ? data.body : data,
);

if (valid && validBody) {
Expand All @@ -185,7 +185,7 @@ export class RequestValidator {
private discriminatorValidator(req, discriminator) {
if (discriminator) {
const { options, property, validators } = discriminator;
const discriminatorValue = req.body[property]; // TODO may not alwasy be in this position
const discriminatorValue = req.body[property]; // TODO may not always be in this position
if (options.find((o) => o.option === discriminatorValue)) {
return validators[discriminatorValue];
} else {
Expand Down
8 changes: 3 additions & 5 deletions src/middlewares/openapi.security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import {
SecurityHandlers,
OpenApiRequestMetadata,
OpenApiRequestHandler,
NotFound,
MethodNotAllowed,
InternalServerError,
HttpError,
} from '../framework/types';
Expand All @@ -29,7 +27,7 @@ export function security(
securityHandlers: SecurityHandlers,
): OpenApiRequestHandler {
return async (req, res, next) => {
// TODO move the folllowing 3 check conditions to a dedicated upstream middleware
// TODO move the following 3 check conditions to a dedicated upstream middleware
if (!req.openapi) {
// this path was not found in open api and
// this path is not defined under an openapi base path
Expand All @@ -38,7 +36,7 @@ export function security(
}

const openapi = <OpenApiRequestMetadata>req.openapi;
// use the local security object or fallbac to api doc's security or undefined
// use the local security object or fallback to api doc's security or undefined
const securities: OpenAPIV3.SecurityRequirementObject[] =
openapi.schema.security ?? apiDoc.security;

Expand Down Expand Up @@ -152,7 +150,7 @@ class SecuritySchemes {
: null;
const promises = this.securities.map(async (s) => {
if (Util.isEmptyObject(s)) {
// anonumous security
// anonymous security
return [{ success: true }];
}
return Promise.all(
Expand Down
2 changes: 1 addition & 1 deletion src/middlewares/parsers/schema.preprocessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ if (!Array.prototype['flatMap']) {
};
Object.defineProperty(Array.prototype, 'flatMap', { enumerable: false });
}
const httpMethods = new Set([
export const httpMethods = new Set([
'get',
'put',
'post',
Expand Down
8 changes: 4 additions & 4 deletions src/openapi.validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export class OpenApiValidator {
middlewares.push((req, res, next) =>
pContext
.then(({ context, responseApiDoc }) => {
metamw = metamw || this.metadataMiddlware(context, responseApiDoc);
metamw = metamw || this.metadataMiddleware(context, responseApiDoc);
return metamw(req, res, next);
})
.catch(next),
Expand Down Expand Up @@ -252,7 +252,7 @@ export class OpenApiValidator {
}
}

private metadataMiddlware(
private metadataMiddleware(
context: OpenApiContext,
responseApiDoc: OpenAPIV3.Document,
) {
Expand Down Expand Up @@ -371,12 +371,12 @@ export class OpenApiValidator {
}
});
defaultSerDes.forEach((currentDefaultSerDes) => {
let defautSerDesOverride = options.serDes.find(
let defaultSerDesOverride = options.serDes.find(
(currentOptionSerDes) => {
return currentDefaultSerDes.format === currentOptionSerDes.format;
},
);
if (!defautSerDesOverride) {
if (!defaultSerDesOverride) {
options.serDes.push(currentDefaultSerDes);
}
});
Expand Down
85 changes: 85 additions & 0 deletions test/allow.header.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { expect } from 'chai';
import * as express from 'express';
import { Server } from 'http';
import * as request from 'supertest';
import * as packageJson from '../package.json';
import * as OpenApiValidator from '../src';
import { OpenAPIV3 } from '../src/framework/types';
import { startServer } from './common/app.common';

describe(packageJson.name, () => {
let app = null;

before(async () => {
app = await createApp();
});

after(() => {
app.server.close();
});

it('adds allow header to 405 - Method Not Allowed', async () =>
request(app)
.put('/v1/pets/greebo')
.expect(405)
.then((response) => {
expect(response.header).to.include({ allow: 'POST, GET' });
}));
});

async function createApp(): Promise<express.Express & { server?: Server }> {
const app = express();

app.use(
OpenApiValidator.middleware({
apiSpec: createApiSpec(),
validateRequests: true,
}),
);
app.use(
express
.Router()
.get('/v1/pets/:petId', () => ['cat', 'dog'])
.post('/v1/pets/:petId', (req, res) => res.json(req.body)),
);

await startServer(app, 3001);
return app;
}

function createApiSpec(): OpenAPIV3.Document {
return {
openapi: '3.0.3',
info: {
title: 'Petstore API',
version: '1.0.0',
},
servers: [
{
url: '/v1/',
},
],
paths: {
'/pets/{petId}': {
parameters: [
{
in: 'path',
name: 'petId',
required: true,
schema: { type: 'string' },
},
],
get: {
responses: {
'200': { description: 'GET Pet' },
},
},
post: {
responses: {
'200': { description: 'POST Pet' },
},
},
},
},
};
}

0 comments on commit 45a40b7

Please sign in to comment.