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

feat(rulesets): validate oas3 runtime expressions #1608

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"no-param-reassign": "off",
"no-restricted-syntax": "off",
"no-continue": "off",
"no-control-regex": "off",
"no-label-var": "off",
"no-void": "off",
"no-undefined": "off",
Expand Down
178 changes: 178 additions & 0 deletions src/rulesets/oas/functions/__tests__/runtimeExpression.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
import { DiagnosticSeverity } from '@stoplight/types';
import { functions } from '../../../../functions';
import { runtimeExpression } from '../runtimeExpression';
import { DocumentInventory } from '../../../../documentInventory';
import { Document } from '../../../../document';
import * as Parsers from '../../../../parsers';
import { isOpenApiv3, RuleType, Spectral } from '../../../..';
import { rules } from '../../index.json';

function runRuntimeExpression(targetVal: any) {
const doc = new Document(JSON.stringify(targetVal), Parsers.Json);

return runtimeExpression.call(
{ functions },
targetVal,
null,
{ given: ['paths', '/path', 'get', 'responses', '200', 'links', 'link', 'parameters', 'param'] },
{ given: null, original: null, documentInventory: new DocumentInventory(doc, {} as any), rule: {} as any }, // TODO
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably don't need to bother about this todo comment.
As a part of the V6 release, a custom function will receive some lifting here and there.

);
}

describe('runtimeExpression', () => {
describe('valid expressions, negative tests', () => {
test.each(['$url', '$method', '$statusCode'])('no messages for valid expressions', expr => {
expect(runRuntimeExpression(expr)).toBeUndefined();
});

test.each([{ obj: 'object' }, ['1'], 1])('no messages for non-strings', expr => {
expect(runRuntimeExpression(expr)).toBeUndefined();
});

test.each(['$request.body', '$response.body'])('no messages for valid expressions', expr => {
expect(runRuntimeExpression(expr)).toBeUndefined();
});

test.each(['$request.body#/chars/in/range/0x00-0x2E/0x30-0x7D/0x7F-0x10FFFF', '$response.body#/simple/path'])(
'no messages for valid expressions',
expr => {
expect(runRuntimeExpression(expr)).toBeUndefined();
},
);

test.each(['$request.body#/~0', '$response.body#/~1'])('no messages for valid expressions', expr => {
expect(runRuntimeExpression(expr)).toBeUndefined();
});

test.each([
'$request.query.query-name',
'$response.query.QUERY-NAME',
'$request.path.path-name',
'$response.path.PATH-NAME',
])('no messages for valid expressions', expr => {
expect(runRuntimeExpression(expr)).toBeUndefined();
});

test.each(["$request.header.a-zA-Z0-9!#$%&'*+-.^_`|~"])('no messages for valid expressions', expr => {
expect(runRuntimeExpression(expr)).toBeUndefined();
});
});

describe('invalid expressions, postive tests', () => {
test.each(['$invalidkeyword'])('error for invalid base keyword', expr => {
const results = runRuntimeExpression(expr);
expect(results['length']).toBe(1);
expect(results[0].message).toEqual(
'expressions must start with one of: `$url`, `$method`, `$statusCode`, `$request.`,`$response.`',
);
});

test.each(['$request.invalidkeyword', '$response.invalidkeyword'])('second key invalid', expr => {
const results = runRuntimeExpression(expr);
expect(results['length']).toBe(1);
expect(results[0].message).toEqual(
'`$request.` and `$response.` must be followed by one of: `header.`, `query.`, `body`, `body#`',
);
});

test.each(['$request.body#.uses.dots.as.delimiters', '$response.body#.uses.dots.as.delimiters'])(
'should error for using `.` as delimiter in json pointer',
expr => {
const results = runRuntimeExpression(expr);
expect(results['length']).toBe(1);
expect(results[0].message).toEqual('`body#` must be followed by `/`');
},
);

test.each(['$request.body#/no/tilde/tokens/in/unescaped~', '$response.body#/invalid/escaped/~01'])(
'errors for incorrect reference tokens',
expr => {
const results = runRuntimeExpression(expr);
expect(results['length']).toBe(1);
expect(results[0].message).toEqual(
'string following `body#` is not a valid JSON pointer, see https://spec.openapis.org/oas/v3.1.0#runtime-expressions for more information',
);
},
);

test.each(['$request.query.', '$response.query.'])('error for invalid name', expr => {
const invalidString = String.fromCodePoint(0x80);
const results = runRuntimeExpression(expr + invalidString);
expect(results['length']).toBe(1);
expect(results[0].message).toEqual(
'string following `query.` and `path.` must only include ascii characters 0x01-0x7F.',
);
});

test.each(['$request.header.', '$request.header.(invalid-parentheses)', '$response.header.no,commas'])(
'error for invalid tokens',
expr => {
const results = runRuntimeExpression(expr);
expect(results).toBeDefined();
expect(results['length']).toBe(1);
expect(results[0].message).toEqual('must provide valid header name after `header.`');
},
);
});
});

describe('runtimeExpression acceptance test', () => {
test('all link objects are validated and correct error object produced', async () => {
const s: Spectral = new Spectral();

s.registerFormat('oas3', isOpenApiv3);
s.setRules({
'links-parameters-expression': {
...rules['links-parameters-expression'],
type: RuleType[rules['links-parameters-expression'].type],
then: {
...rules['links-parameters-expression'].then,
},
},
});

expect(
await s.run({
openapi: '3.0.1',
info: {
title: 'response example',
version: '1.0',
},
paths: {
'/user': {
get: {
responses: {
200: {
description: 'dummy description',
links: {
link1: {
parameters: '$invalidkeyword',
},
link2: {
parameters: '$invalidkeyword',
},
},
},
},
},
},
},
}),
).toEqual([
{
code: 'links-parameters-expression',
message: 'expressions must start with one of: `$url`, `$method`, `$statusCode`, `$request.`,`$response.`',
path: ['paths', '/user', 'get', 'responses', 'linkName', 'link1', 'parameters'],
severity: DiagnosticSeverity.Error,
range: expect.any(Object),
},
{
code: 'links-parameters-expression',
message: 'expressions must start with one of: `$url`, `$method`, `$statusCode`, `$request.`,`$response.`',
path: ['paths', '/user', 'get', 'responses', 'linkName', 'link2', 'parameters'],
severity: DiagnosticSeverity.Error,
range: expect.any(Object),
},
]);
});
});
102 changes: 102 additions & 0 deletions src/rulesets/oas/functions/runtimeExpression.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import { isString } from 'lodash';
import type { IFunction, IFunctionResult } from '../../../types';

export const runtimeExpression: IFunction = function (exp): void | IFunctionResult[] {
// oas3 spec allows for type Any, so only validate when exp is a string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A default export is needed too.

if (!isString(exp)) return;
if (['$url', '$method', '$statusCode'].includes(exp)) {
// valid expression
return;
} else if (exp.startsWith('$request.') || exp.startsWith('$response.')) {
return validateSource(exp.replace(/^\$(request\.|response\.)/, ''));
}

return [
{
message: 'expressions must start with one of: `$url`, `$method`, `$statusCode`, `$request.`,`$response.`',
},
];
};

function validateSource(source: string): void | IFunctionResult[] {
if (source === 'body') {
// valid expression
return;
} else if (source.startsWith('body#')) {
return validateJsonPointer(source.replace(/^body#/, ''));
} else if (source.startsWith('query.') || source.startsWith('path.')) {
return validateName(source.replace(/^(query\.|path\.)/, ''));
} else if (source.startsWith('header.')) {
return validateToken(source.replace(/^header\./, ''));
}

return [
{
message: '`$request.` and `$response.` must be followed by one of: `header.`, `query.`, `body`, `body#`',
},
];
}

function validateJsonPointer(jsonPointer: string): void | IFunctionResult[] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually pretty cool util that's worth porting to our @stoplight/json package.
We have a set of functions that deal with various json pointers scenarios, and this would be a perfect fit.

Do you mind if I port it over there, so we can use it here and elsewhere in our ecosystem?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely, please feel free to do that!

I apologize for not following up sooner. This PR just slipped my mind.

if (!jsonPointer.startsWith('/')) {
return [
{
message: '`body#` must be followed by `/`',
},
];
}
while (jsonPointer.includes('/')) {
// remove everything up to and including the first `/`
jsonPointer = jsonPointer.replace(/[^/]*\//, '');
// get substring before the next `/`
const referenceToken: string = jsonPointer.includes('/')
? jsonPointer.slice(0, jsonPointer.indexOf('/'))
: jsonPointer;
if (!isValidReferenceToken(referenceToken)) {
return [
{
message:
'string following `body#` is not a valid JSON pointer, see https://spec.openapis.org/oas/v3.1.0#runtime-expressions for more information',
},
];
}
}
}

function validateName(name: string): void | IFunctionResult[] {
// zero or more of characters in the ASCII range 0x01-0x7F
const validName = /^[\x01-\x7F]*$/;
if (!validName.test(name)) {
return [
{
message: 'string following `query.` and `path.` must only include ascii characters 0x01-0x7F.',
},
];
}
}

function validateToken(token: string): void | IFunctionResult[] {
// one or more of the given tchar characters
const validTCharString = /^[a-zA-Z0-9!#$%&'*+\-.^_`|~]+$/;
if (!validTCharString.test(token)) {
return [
{
message: 'must provide valid header name after `header.`',
},
];
}
}

function isValidReferenceToken(referenceToken: string): boolean {
return isValidEscaped(referenceToken) || isValidUnescaped(referenceToken);
}

function isValidEscaped(escaped: string): boolean {
// escaped must be empty/null or match the given pattern
return !escaped || !!/^~(0|1)$/.exec(escaped);
barrett-schonefeld marked this conversation as resolved.
Show resolved Hide resolved
}

function isValidUnescaped(unescaped: string): boolean {
// unescaped may be empty/null, expect no `/` and no `~` chars
return !unescaped || !/(\/|~)/.exec(unescaped);
barrett-schonefeld marked this conversation as resolved.
Show resolved Hide resolved
}
12 changes: 12 additions & 0 deletions src/rulesets/oas/index.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"oasPathParam",
"oasTagDefined",
"oasUnusedComponent",
"runtimeExpression",
"typedEnum",
"refSiblings"
],
Expand Down Expand Up @@ -163,6 +164,17 @@
"function": "truthy"
}
},
"links-parameters-expression": {
"description": "The links.parameters object's values should be valid runtime expressions.",
"recommended": true,
"type": "validation",
"formats": ["oas3"],
"message": "{{error}}",
"given": "$.paths.*.*.responses.*.links.*.parameters.*",
"then": {
"function": "runTimeExpression"
barrett-schonefeld marked this conversation as resolved.
Show resolved Hide resolved
}
},
"no-eval-in-markdown": {
"description": "Markdown descriptions should not contain `eval(`.",
"recommended": true,
Expand Down