From ba18f9dc4c08503c0b678476671949109ae937a7 Mon Sep 17 00:00:00 2001 From: Kyle Shockey Date: Thu, 9 Nov 2017 20:19:40 -0800 Subject: [PATCH 1/3] Disable allowReserved effects for non-query parameters --- src/execute/oas3/parameter-builders.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/execute/oas3/parameter-builders.js b/src/execute/oas3/parameter-builders.js index 8ef8c7c36..d436ddcc9 100644 --- a/src/execute/oas3/parameter-builders.js +++ b/src/execute/oas3/parameter-builders.js @@ -14,7 +14,7 @@ function path({req, value, parameter}) { value, style: style || 'simple', explode: explode || false, - escape: !parameter.allowReserved, + escape: false, }) req.url = req.url.replace(`{${name}}`, styledValue) @@ -110,7 +110,7 @@ function header({req, parameter, value}) { value, style: parameter.style || 'simple', explode: typeof parameter.explode === 'undefined' ? false : parameter.explode, - escape: !parameter.allowReserved, + escape: false, }) } } From 3ae51c63344c058f69fa8b066218318bbfa992d7 Mon Sep 17 00:00:00 2001 From: Kyle Shockey Date: Thu, 9 Nov 2017 20:46:15 -0800 Subject: [PATCH 2/3] Improve handling of query allowReserved flag --- package.json | 1 + src/execute/oas3/style-serializer.js | 43 +++--- test/oas3/execute/style-explode.js | 208 ++++++++++++++++++++++++++- 3 files changed, 232 insertions(+), 20 deletions(-) diff --git a/package.json b/package.json index c87e5b30f..2977ae3ce 100644 --- a/package.json +++ b/package.json @@ -67,6 +67,7 @@ "cookie": "^0.3.1", "cross-fetch": "0.0.8", "deep-extend": "^0.4.1", + "encode-3986": "^1.0.0", "fast-json-patch": "1.1.8", "isomorphic-form-data": "0.0.1", "js-yaml": "^3.8.1", diff --git a/src/execute/oas3/style-serializer.js b/src/execute/oas3/style-serializer.js index d7595dcb1..0e1c21af1 100644 --- a/src/execute/oas3/style-serializer.js +++ b/src/execute/oas3/style-serializer.js @@ -1,3 +1,5 @@ +import encodeToRFC3986 from "encode-3986" + export default function (config) { const {value} = config @@ -14,16 +16,18 @@ export default function (config) { const escapeFn = str => encodeURIComponent(str) function encodeArray({key, value, style, explode, escape}) { + const valueEncoder = escape ? a => encodeToRFC3986(a) : a => a + if (style === 'simple') { - return value.join(',') + return value.map(val => valueEncoder(val)).join(',') } if (style === 'label') { - return `.${value.join('.')}` + return `.${value.map(val => valueEncoder(val)).join('.')}` } if (style === 'matrix') { - return value.reduce((prev, curr) => { + return value.map(val => valueEncoder(val)).reduce((prev, curr) => { if (!prev || explode) { return `${(prev || '')};${key}=${curr}` } @@ -34,27 +38,28 @@ function encodeArray({key, value, style, explode, escape}) { if (style === 'form') { const commaValue = escape ? escapeFn(',') : ',' const after = explode ? `&${key}=` : commaValue - return value.join(after) + return value.map(val => valueEncoder(val)).join(after) } if (style === 'spaceDelimited') { const after = explode ? `${key}=` : '' - return value.join(`${escapeFn(' ')}${after}`) + return value.map(val => valueEncoder(val)).join(`${escapeFn(' ')}${after}`) } if (style === 'pipeDelimited') { const after = explode ? `${key}=` : '' const separator = escape ? escapeFn('|') : '|' - return value.join(`${separator}${after}`) + return value.map(val => valueEncoder(val)).join(`${separator}${after}`) } } -function encodeObject({key, value, style, explode}) { +function encodeObject({key, value, style, explode, escape}) { + const valueEncoder = escape ? a => encodeToRFC3986(a) : a => a const valueKeys = Object.keys(value) if (style === 'simple') { return valueKeys.reduce((prev, curr) => { - const val = value[curr] + const val = valueEncoder(value[curr]) const middleChar = explode ? '=' : ',' const prefix = prev ? `${prev},` : '' @@ -64,7 +69,7 @@ function encodeObject({key, value, style, explode}) { if (style === 'label') { return valueKeys.reduce((prev, curr) => { - const val = value[curr] + const val = valueEncoder(value[curr]) const middleChar = explode ? '=' : '.' const prefix = prev ? `${prev}.` : '.' @@ -74,7 +79,7 @@ function encodeObject({key, value, style, explode}) { if (style === 'matrix' && explode) { return valueKeys.reduce((prev, curr) => { - const val = value[curr] + const val = valueEncoder(value[curr]) const prefix = prev ? `${prev};` : ';' return `${prefix}${curr}=${val}` @@ -84,7 +89,7 @@ function encodeObject({key, value, style, explode}) { if (style === 'matrix') { // no explode return valueKeys.reduce((prev, curr) => { - const val = value[curr] + const val = valueEncoder(value[curr]) const prefix = prev ? `${prev},` : `;${key}=` return `${prefix}${curr},${val}` @@ -93,7 +98,7 @@ function encodeObject({key, value, style, explode}) { if (style === 'form') { return valueKeys.reduce((prev, curr) => { - const val = value[curr] + const val = valueEncoder(value[curr]) const prefix = prev ? `${prev}${explode ? '&' : ','}` : '' const separator = explode ? '=' : ',' @@ -102,24 +107,26 @@ function encodeObject({key, value, style, explode}) { } } -function encodePrimitive({key, value, style, explode}) { +function encodePrimitive({key, value, style, explode, escape}) { + const valueEncoder = escape ? a => encodeToRFC3986(a) : a => a + if (style === 'simple') { - return value + return valueEncoder(value) } if (style === 'label') { - return `.${value}` + return `.${valueEncoder(value)}` } if (style === 'matrix') { - return `;${key}=${value}` + return `;${key}=${valueEncoder(value)}` } if (style === 'form') { - return value + return valueEncoder(value) } if (style === 'deepObject') { - return value + return valueEncoder(value) } } diff --git a/test/oas3/execute/style-explode.js b/test/oas3/execute/style-explode.js index f73cec7fb..3ae45cc28 100644 --- a/test/oas3/execute/style-explode.js +++ b/test/oas3/execute/style-explode.js @@ -946,6 +946,83 @@ describe('buildRequest w/ `style` & `explode` - OpenAPI Specification 3.0', func headers: {}, }) }) + + it('should build a query parameter in form/no-explode format with allowReserved', function () { + // Given + const spec = { + openapi: '3.0.0', + paths: { + '/users': { + get: { + operationId: 'myOperation', + parameters: [ + { + name: 'id', + in: 'query', + style: 'form', + explode: false, + allowReserved: true + } + ] + } + } + } + } + + // when + const req = buildRequest({ + spec, + operationId: 'myOperation', + parameters: { + id: `:/?#[]@!$&'()*+,;=` + } + }) + + expect(req).toEqual({ + method: 'GET', + url: `/users?id=:/?#[]@!$&'()*+,;=`, + credentials: 'same-origin', + headers: {}, + }) + }) + + it('should build a query parameter in form/no-explode format with percent-encoding if allowReserved is not set', function () { + // Given + const spec = { + openapi: '3.0.0', + paths: { + '/users': { + get: { + operationId: 'myOperation', + parameters: [ + { + name: 'id', + in: 'query', + style: 'form', + explode: false + } + ] + } + } + } + } + + // when + const req = buildRequest({ + spec, + operationId: 'myOperation', + parameters: { + id: `:/?#[]@!$&'()*+,;=` + } + }) + + expect(req).toEqual({ + method: 'GET', + url: `/users?id=%3A%2F%3F%23%5B%5D%40%21%24%26%27%28%29%2A%2B%2C%3B%3D`, + credentials: 'same-origin', + headers: {}, + }) + }) }) describe('array values', function () { const VALUE = [3, 4, 5] @@ -1089,13 +1166,57 @@ describe('buildRequest w/ `style` & `explode` - OpenAPI Specification 3.0', func spec, operationId: 'myOperation', parameters: { - id: VALUE + id: [ + ":", "/", "?", "#", "[", "]", "@", "!", "$", "&", "'", + "(", ")", "*", "+", ",", ";", "=" + ] + } + }) + + expect(req).toEqual({ + method: 'GET', + url: `/users?id=:,/,?,#,[,],@,!,$,&,',(,),*,+,,,;,=`, + credentials: 'same-origin', + headers: {}, + }) + }) + + it('should build a query parameter in form/no-explode format without allowReserved', function () { + // Given + const spec = { + openapi: '3.0.0', + paths: { + '/users': { + get: { + operationId: 'myOperation', + parameters: [ + { + name: 'id', + in: 'query', + style: 'form', + explode: false + } + ] + } + } + } + } + + // when + const req = buildRequest({ + spec, + operationId: 'myOperation', + parameters: { + id: [ + ":", "/", "?", "#", "[", "]", "@", "!", "$", "&", "'", + "(", ")", "*", "+", ",", ";", "=" + ] } }) expect(req).toEqual({ method: 'GET', - url: '/users?id=3,4,5', + url: `/users?id=%3A%2C%2F%2C%3F%2C%23%2C%5B%2C%5D%2C%40%2C%21%2C%24%2C%26%2C%27%2C%28%2C%29%2C%2A%2C%2B%2C%2C%2C%3B%2C%3D`, credentials: 'same-origin', headers: {}, }) @@ -1488,6 +1609,89 @@ describe('buildRequest w/ `style` & `explode` - OpenAPI Specification 3.0', func }) }) + it('should build a query parameter in form/no-explode format with allowReserved', function () { + // Given + const spec = { + openapi: '3.0.0', + paths: { + '/users': { + get: { + operationId: 'myOperation', + parameters: [ + { + name: 'id', + in: 'query', + style: 'form', + explode: false, + allowReserved: true + } + ] + } + } + } + } + + // when + const req = buildRequest({ + spec, + operationId: 'myOperation', + parameters: { + id: { + role: 'admin', + firstName: ':/?#[]@!$&\'()*+,;=' + } + } + }) + + expect(req).toEqual({ + method: 'GET', + url: '/users?id=role,admin,firstName,:/?#[]@!$&\'()*+,;=', + credentials: 'same-origin', + headers: {}, + }) + }) + + it('should build a query parameter in form/no-explode format without allowReserved', function () { + // Given + const spec = { + openapi: '3.0.0', + paths: { + '/users': { + get: { + operationId: 'myOperation', + parameters: [ + { + name: 'id', + in: 'query', + style: 'form', + explode: false + } + ] + } + } + } + } + + // when + const req = buildRequest({ + spec, + operationId: 'myOperation', + parameters: { + id: { + role: 'admin', + firstName: ':/?#[]@!$&\'()*+,;=' + } + } + }) + + expect(req).toEqual({ + method: 'GET', + url: '/users?id=role,admin,firstName,%3A%2F%3F%23%5B%5D%40%21%24%26%27%28%29%2A%2B%2C%3B%3D', + credentials: 'same-origin', + headers: {}, + }) + }) + it('should build a query parameter in deepObject/explode format', function () { // Given const spec = { From c469ad7bba414fd97dddce8849b7f94f3db611dd Mon Sep 17 00:00:00 2001 From: Kyle Shockey Date: Thu, 9 Nov 2017 21:07:08 -0800 Subject: [PATCH 3/3] Linter fixes --- src/execute/oas3/style-serializer.js | 2 +- test/oas3/execute/style-explode.js | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/execute/oas3/style-serializer.js b/src/execute/oas3/style-serializer.js index 0e1c21af1..0a6d60082 100644 --- a/src/execute/oas3/style-serializer.js +++ b/src/execute/oas3/style-serializer.js @@ -1,4 +1,4 @@ -import encodeToRFC3986 from "encode-3986" +import encodeToRFC3986 from 'encode-3986' export default function (config) { const {value} = config diff --git a/test/oas3/execute/style-explode.js b/test/oas3/execute/style-explode.js index 3ae45cc28..bcc3e1d00 100644 --- a/test/oas3/execute/style-explode.js +++ b/test/oas3/execute/style-explode.js @@ -974,13 +974,13 @@ describe('buildRequest w/ `style` & `explode` - OpenAPI Specification 3.0', func spec, operationId: 'myOperation', parameters: { - id: `:/?#[]@!$&'()*+,;=` + id: ':/?#[]@!$&\'()*+,;=' } }) expect(req).toEqual({ method: 'GET', - url: `/users?id=:/?#[]@!$&'()*+,;=`, + url: '/users?id=:/?#[]@!$&\'()*+,;=', credentials: 'same-origin', headers: {}, }) @@ -1012,13 +1012,13 @@ describe('buildRequest w/ `style` & `explode` - OpenAPI Specification 3.0', func spec, operationId: 'myOperation', parameters: { - id: `:/?#[]@!$&'()*+,;=` + id: ':/?#[]@!$&\'()*+,;=' } }) expect(req).toEqual({ method: 'GET', - url: `/users?id=%3A%2F%3F%23%5B%5D%40%21%24%26%27%28%29%2A%2B%2C%3B%3D`, + url: '/users?id=%3A%2F%3F%23%5B%5D%40%21%24%26%27%28%29%2A%2B%2C%3B%3D', credentials: 'same-origin', headers: {}, }) @@ -1167,15 +1167,15 @@ describe('buildRequest w/ `style` & `explode` - OpenAPI Specification 3.0', func operationId: 'myOperation', parameters: { id: [ - ":", "/", "?", "#", "[", "]", "@", "!", "$", "&", "'", - "(", ")", "*", "+", ",", ";", "=" + ':', '/', '?', '#', '[', ']', '@', '!', '$', '&', '\'', + '(', ')', '*', '+', ',', ';', '=' ] } }) expect(req).toEqual({ method: 'GET', - url: `/users?id=:,/,?,#,[,],@,!,$,&,',(,),*,+,,,;,=`, + url: '/users?id=:,/,?,#,[,],@,!,$,&,\',(,),*,+,,,;,=', credentials: 'same-origin', headers: {}, }) @@ -1208,15 +1208,15 @@ describe('buildRequest w/ `style` & `explode` - OpenAPI Specification 3.0', func operationId: 'myOperation', parameters: { id: [ - ":", "/", "?", "#", "[", "]", "@", "!", "$", "&", "'", - "(", ")", "*", "+", ",", ";", "=" + ':', '/', '?', '#', '[', ']', '@', '!', '$', '&', '\'', + '(', ')', '*', '+', ',', ';', '=' ] } }) expect(req).toEqual({ method: 'GET', - url: `/users?id=%3A%2C%2F%2C%3F%2C%23%2C%5B%2C%5D%2C%40%2C%21%2C%24%2C%26%2C%27%2C%28%2C%29%2C%2A%2C%2B%2C%2C%2C%3B%2C%3D`, + url: '/users?id=%3A%2C%2F%2C%3F%2C%23%2C%5B%2C%5D%2C%40%2C%21%2C%24%2C%26%2C%27%2C%28%2C%29%2C%2A%2C%2B%2C%2C%2C%3B%2C%3D', credentials: 'same-origin', headers: {}, })