From 507f35ba6843126a218ffc09aa26f2df72bbdfbb Mon Sep 17 00:00:00 2001 From: offirgolan Date: Fri, 1 Feb 2019 17:57:26 -0800 Subject: [PATCH 1/3] fix: Correctly handle array header values --- .../persister-fs-node-fetch-test.js | 18 +++++++++++ packages/@pollyjs/adapter/src/index.js | 19 ++++-------- .../src/utils/normalize-recorded-response.js | 30 +++++++++++++++++++ .../@pollyjs/persister/src/har/request.js | 9 ++++-- .../@pollyjs/persister/src/har/response.js | 13 ++++---- .../src/har/utils/get-last-header.js | 9 ++++++ .../persister/src/har/utils/to-nv-pairs.js | 13 +++++++- 7 files changed, 89 insertions(+), 22 deletions(-) create mode 100644 packages/@pollyjs/adapter-node-http/tests/integration/persister-fs-node-fetch-test.js create mode 100644 packages/@pollyjs/adapter/src/utils/normalize-recorded-response.js create mode 100644 packages/@pollyjs/persister/src/har/utils/get-last-header.js diff --git a/packages/@pollyjs/adapter-node-http/tests/integration/persister-fs-node-fetch-test.js b/packages/@pollyjs/adapter-node-http/tests/integration/persister-fs-node-fetch-test.js new file mode 100644 index 00000000..8e8ffc9a --- /dev/null +++ b/packages/@pollyjs/adapter-node-http/tests/integration/persister-fs-node-fetch-test.js @@ -0,0 +1,18 @@ +import '@pollyjs-tests/helpers/global-fetch'; + +import setupPersister from '@pollyjs-tests/helpers/setup-persister'; +import setupFetchRecord from '@pollyjs-tests/helpers/setup-fetch-record'; +import persisterTests from '@pollyjs-tests/integration/persister-tests'; +import { setupMocha as setupPolly } from '@pollyjs/core'; + +import pollyConfig from '../utils/polly-config'; + +describe.only('Integration | FS Persister | node-fetch', function() { + setupPolly.beforeEach(pollyConfig); + + setupFetchRecord({ host: 'http://localhost:4000' }); + setupPersister(); + setupPolly.afterEach(); + + persisterTests(); +}); diff --git a/packages/@pollyjs/adapter/src/index.js b/packages/@pollyjs/adapter/src/index.js index 13b10364..a46b72b7 100644 --- a/packages/@pollyjs/adapter/src/index.js +++ b/packages/@pollyjs/adapter/src/index.js @@ -3,6 +3,7 @@ import { ACTIONS, MODES, Serializers, assert } from '@pollyjs/utils'; import Interceptor from './-private/interceptor'; import isExpired from './utils/is-expired'; import stringifyRequest from './utils/stringify-request'; +import normalizeRecordedResponse from './utils/normalize-recorded-response'; const REQUEST_HANDLER = Symbol(); @@ -185,19 +186,11 @@ export default class Adapter { await this.timeout(pollyRequest, recordingEntry); pollyRequest.action = ACTIONS.REPLAY; - const { status, statusText, headers, content } = recordingEntry.response; - const normalizedResponse = { - statusText, - statusCode: status, - headers: (headers || []).reduce((accum, { name, value }) => { - accum[name] = value; - - return accum; - }, {}), - body: content && content.text - }; - - return this.onReplay(pollyRequest, normalizedResponse, recordingEntry); + return this.onReplay( + pollyRequest, + normalizeRecordedResponse(recordingEntry.response), + recordingEntry + ); } if (config.recordIfMissing) { diff --git a/packages/@pollyjs/adapter/src/utils/normalize-recorded-response.js b/packages/@pollyjs/adapter/src/utils/normalize-recorded-response.js new file mode 100644 index 00000000..55c29845 --- /dev/null +++ b/packages/@pollyjs/adapter/src/utils/normalize-recorded-response.js @@ -0,0 +1,30 @@ +const { isArray } = Array; + +export default function normalizeRecordedResponse(response) { + const { status, statusText, headers, content } = response; + + return { + statusText, + statusCode: status, + headers: normalizeHeaders(headers), + body: content && content.text + }; +} + +function normalizeHeaders(headers) { + return (headers || []).reduce((accum, { name, value }) => { + const existingValue = accum[name]; + + if (existingValue) { + if (!isArray(existingValue)) { + accum[name] = [existingValue]; + } + + accum[name].push(value); + } else { + accum[name] = value; + } + + return accum; + }, {}); +} diff --git a/packages/@pollyjs/persister/src/har/request.js b/packages/@pollyjs/persister/src/har/request.js index 7ff6fd35..980aa7c7 100644 --- a/packages/@pollyjs/persister/src/har/request.js +++ b/packages/@pollyjs/persister/src/har/request.js @@ -2,6 +2,7 @@ import getByteLength from 'utf8-byte-length'; import setCookies from 'set-cookie-parser'; import toNVPairs from './utils/to-nv-pairs'; +import getLastHeader from './utils/get-last-header'; function headersSize(request) { const keys = []; @@ -34,7 +35,7 @@ export default class Request { if (request.body) { this.postData = { - mimeType: request.getHeader('Content-Type') || 'text/plain', + mimeType: getLastHeader(this, 'Content-Type') || 'text/plain', params: [] }; @@ -43,8 +44,10 @@ export default class Request { } } - if (request.hasHeader('Content-Length')) { - this.bodySize = parseInt(request.getHeader('Content-Length'), 10); + const contentLength = getLastHeader(this, 'Content-Length'); + + if (contentLength) { + this.bodySize = parseInt(contentLength, 10); } else { this.bodySize = this.postData && this.postData.text diff --git a/packages/@pollyjs/persister/src/har/response.js b/packages/@pollyjs/persister/src/har/response.js index 9c9f47da..2c113b99 100644 --- a/packages/@pollyjs/persister/src/har/response.js +++ b/packages/@pollyjs/persister/src/har/response.js @@ -2,6 +2,7 @@ import getByteLength from 'utf8-byte-length'; import setCookies from 'set-cookie-parser'; import toNVPairs from './utils/to-nv-pairs'; +import getLastHeader from './utils/get-last-header'; function headersSize(response) { const keys = []; @@ -28,24 +29,26 @@ export default class Response { this.headers = toNVPairs(response.headers); this.headersSize = headersSize(this); this.cookies = setCookies.parse(response.getHeader('Set-Cookie')); - this.redirectURL = ''; + this.redirectURL = getLastHeader(this, 'Location') || ''; this.content = { - mimeType: response.getHeader('Content-Type') || 'text/plain' + mimeType: getLastHeader(this, 'Content-Type') || 'text/plain' }; if (response.body && typeof response.body === 'string') { this.content.text = response.body; } - if (response.hasHeader('Content-Length')) { - this.content.size = parseInt(response.getHeader('Content-Length'), 10); + const contentLength = getLastHeader(this, 'Content-Length'); + + if (contentLength) { + this.content.size = parseInt(contentLength, 10); } else { this.content.size = this.content.text ? getByteLength(this.content.text) : 0; } - this.bodySize = this.content ? this.content.size : 0; + this.bodySize = this.content.size; } } diff --git a/packages/@pollyjs/persister/src/har/utils/get-last-header.js b/packages/@pollyjs/persister/src/har/utils/get-last-header.js new file mode 100644 index 00000000..414dcc47 --- /dev/null +++ b/packages/@pollyjs/persister/src/har/utils/get-last-header.js @@ -0,0 +1,9 @@ +export default function getLastHeader({ headers }, name) { + name = name.toLowerCase(); + + for (let i = headers.length - 1; i >= 0; i--) { + if (headers[i].name === name) { + return headers[i].value; + } + } +} diff --git a/packages/@pollyjs/persister/src/har/utils/to-nv-pairs.js b/packages/@pollyjs/persister/src/har/utils/to-nv-pairs.js index 5ea8a167..52d4be23 100644 --- a/packages/@pollyjs/persister/src/har/utils/to-nv-pairs.js +++ b/packages/@pollyjs/persister/src/har/utils/to-nv-pairs.js @@ -1,5 +1,16 @@ const { keys } = Object; +const { isArray } = Array; export default function toNVPairs(o) { - return keys(o || {}).map(name => ({ name, value: o[name] })); + return keys(o || {}).reduce((pairs, name) => { + const value = o[name]; + + if (isArray(value)) { + pairs.push(...value.map(v => ({ name, value: v }))); + } else { + pairs.push({ name, value }); + } + + return pairs; + }, []); } From bffe362f8f1e30f0dca35773e36ea773fcf31c01 Mon Sep 17 00:00:00 2001 From: offirgolan Date: Sat, 2 Feb 2019 23:04:55 -0800 Subject: [PATCH 2/3] fix: Add `_fromType` and add test cases --- .../persister-fs-node-fetch-test.js | 18 ------ .../src/utils/normalize-recorded-response.js | 4 +- .../@pollyjs/persister/src/har/request.js | 6 +- .../@pollyjs/persister/src/har/response.js | 8 +-- .../src/har/utils/get-first-header.js | 20 ++++++ .../src/har/utils/get-last-header.js | 9 --- .../persister/src/har/utils/to-nv-pairs.js | 7 +- tests/integration/persister-tests.js | 64 +++++++++++++++++++ 8 files changed, 99 insertions(+), 37 deletions(-) delete mode 100644 packages/@pollyjs/adapter-node-http/tests/integration/persister-fs-node-fetch-test.js create mode 100644 packages/@pollyjs/persister/src/har/utils/get-first-header.js delete mode 100644 packages/@pollyjs/persister/src/har/utils/get-last-header.js diff --git a/packages/@pollyjs/adapter-node-http/tests/integration/persister-fs-node-fetch-test.js b/packages/@pollyjs/adapter-node-http/tests/integration/persister-fs-node-fetch-test.js deleted file mode 100644 index 8e8ffc9a..00000000 --- a/packages/@pollyjs/adapter-node-http/tests/integration/persister-fs-node-fetch-test.js +++ /dev/null @@ -1,18 +0,0 @@ -import '@pollyjs-tests/helpers/global-fetch'; - -import setupPersister from '@pollyjs-tests/helpers/setup-persister'; -import setupFetchRecord from '@pollyjs-tests/helpers/setup-fetch-record'; -import persisterTests from '@pollyjs-tests/integration/persister-tests'; -import { setupMocha as setupPolly } from '@pollyjs/core'; - -import pollyConfig from '../utils/polly-config'; - -describe.only('Integration | FS Persister | node-fetch', function() { - setupPolly.beforeEach(pollyConfig); - - setupFetchRecord({ host: 'http://localhost:4000' }); - setupPersister(); - setupPolly.afterEach(); - - persisterTests(); -}); diff --git a/packages/@pollyjs/adapter/src/utils/normalize-recorded-response.js b/packages/@pollyjs/adapter/src/utils/normalize-recorded-response.js index 55c29845..47b2dea7 100644 --- a/packages/@pollyjs/adapter/src/utils/normalize-recorded-response.js +++ b/packages/@pollyjs/adapter/src/utils/normalize-recorded-response.js @@ -12,7 +12,7 @@ export default function normalizeRecordedResponse(response) { } function normalizeHeaders(headers) { - return (headers || []).reduce((accum, { name, value }) => { + return (headers || []).reduce((accum, { name, value, _fromType }) => { const existingValue = accum[name]; if (existingValue) { @@ -22,7 +22,7 @@ function normalizeHeaders(headers) { accum[name].push(value); } else { - accum[name] = value; + accum[name] = _fromType === 'array' ? [value] : value; } return accum; diff --git a/packages/@pollyjs/persister/src/har/request.js b/packages/@pollyjs/persister/src/har/request.js index 980aa7c7..8e236a51 100644 --- a/packages/@pollyjs/persister/src/har/request.js +++ b/packages/@pollyjs/persister/src/har/request.js @@ -2,7 +2,7 @@ import getByteLength from 'utf8-byte-length'; import setCookies from 'set-cookie-parser'; import toNVPairs from './utils/to-nv-pairs'; -import getLastHeader from './utils/get-last-header'; +import getFirstHeader from './utils/get-first-header'; function headersSize(request) { const keys = []; @@ -35,7 +35,7 @@ export default class Request { if (request.body) { this.postData = { - mimeType: getLastHeader(this, 'Content-Type') || 'text/plain', + mimeType: getFirstHeader(request, 'Content-Type') || 'text/plain', params: [] }; @@ -44,7 +44,7 @@ export default class Request { } } - const contentLength = getLastHeader(this, 'Content-Length'); + const contentLength = getFirstHeader(request, 'Content-Length'); if (contentLength) { this.bodySize = parseInt(contentLength, 10); diff --git a/packages/@pollyjs/persister/src/har/response.js b/packages/@pollyjs/persister/src/har/response.js index 2c113b99..8a70b2ee 100644 --- a/packages/@pollyjs/persister/src/har/response.js +++ b/packages/@pollyjs/persister/src/har/response.js @@ -2,7 +2,7 @@ import getByteLength from 'utf8-byte-length'; import setCookies from 'set-cookie-parser'; import toNVPairs from './utils/to-nv-pairs'; -import getLastHeader from './utils/get-last-header'; +import getFirstHeader from './utils/get-first-header'; function headersSize(response) { const keys = []; @@ -29,17 +29,17 @@ export default class Response { this.headers = toNVPairs(response.headers); this.headersSize = headersSize(this); this.cookies = setCookies.parse(response.getHeader('Set-Cookie')); - this.redirectURL = getLastHeader(this, 'Location') || ''; + this.redirectURL = getFirstHeader(response, 'Location') || ''; this.content = { - mimeType: getLastHeader(this, 'Content-Type') || 'text/plain' + mimeType: getFirstHeader(response, 'Content-Type') || 'text/plain' }; if (response.body && typeof response.body === 'string') { this.content.text = response.body; } - const contentLength = getLastHeader(this, 'Content-Length'); + const contentLength = getFirstHeader(response, 'Content-Length'); if (contentLength) { this.content.size = parseInt(contentLength, 10); diff --git a/packages/@pollyjs/persister/src/har/utils/get-first-header.js b/packages/@pollyjs/persister/src/har/utils/get-first-header.js new file mode 100644 index 00000000..602b2615 --- /dev/null +++ b/packages/@pollyjs/persister/src/har/utils/get-first-header.js @@ -0,0 +1,20 @@ +const { isArray } = Array; + +/** + * Get the value of the given header name. If the value is an array, + * get the first value. + * + * @export + * @param {PollyRequest | PollyResponse} r + * @param {string} name + * @returns {string | undefined} + */ +export default function getFirstHeader(r, name) { + const value = r.getHeader(name); + + if (isArray(value)) { + return value.length > 0 ? value[0] : ''; + } + + return value; +} diff --git a/packages/@pollyjs/persister/src/har/utils/get-last-header.js b/packages/@pollyjs/persister/src/har/utils/get-last-header.js deleted file mode 100644 index 414dcc47..00000000 --- a/packages/@pollyjs/persister/src/har/utils/get-last-header.js +++ /dev/null @@ -1,9 +0,0 @@ -export default function getLastHeader({ headers }, name) { - name = name.toLowerCase(); - - for (let i = headers.length - 1; i >= 0; i--) { - if (headers[i].name === name) { - return headers[i].value; - } - } -} diff --git a/packages/@pollyjs/persister/src/har/utils/to-nv-pairs.js b/packages/@pollyjs/persister/src/har/utils/to-nv-pairs.js index 52d4be23..ede7857d 100644 --- a/packages/@pollyjs/persister/src/har/utils/to-nv-pairs.js +++ b/packages/@pollyjs/persister/src/har/utils/to-nv-pairs.js @@ -6,7 +6,12 @@ export default function toNVPairs(o) { const value = o[name]; if (isArray(value)) { - pairs.push(...value.map(v => ({ name, value: v }))); + // _fromType is used to convert the stored header back into the + // type it originally was. Take the following example: + // { 'content-type': ['text/htm'] } + // => { name: 'content-type', value: 'text/html', _fromType: 'array } + // => { 'content-type': ['text/htm'] } + pairs.push(...value.map(v => ({ name, value: v, _fromType: 'array' }))); } else { pairs.push({ name, value }); } diff --git a/tests/integration/persister-tests.js b/tests/integration/persister-tests.js index cc95042a..8dc02453 100644 --- a/tests/integration/persister-tests.js +++ b/tests/integration/persister-tests.js @@ -129,6 +129,70 @@ export default function persisterTests() { this.polly.recordingName = recordingName; }); + it('should correctly handle array header values', async function() { + const { recordingId, server, persister } = this.polly; + let responseCalled = false; + + this.polly.record(); + + server + .get(this.recordUrl()) + .configure({ matchRequestsBy: { order: false } }) + .once('beforeResponse', (req, res) => { + res.setHeaders({ + string: 'foo', + one: ['foo'], + two: ['foo', 'bar'] + }); + }); + + await this.fetchRecord(); + await persister.persist(); + + const har = await persister.find(recordingId); + const { headers } = har.log.entries[0].response; + + expect(await validate.har(har)).to.be.true; + expect( + headers.filter(({ _fromType }) => _fromType === 'array') + ).to.have.lengthOf(3); + + this.polly.replay(); + + server.get(this.recordUrl()).once('response', (req, res) => { + expect(res.getHeader('string')).to.equal('foo'); + expect(res.getHeader('one')).to.deep.equal(['foo']); + expect(res.getHeader('two')).to.deep.equal(['foo', 'bar']); + responseCalled = true; + }); + + await this.fetchRecord(); + expect(responseCalled).to.be.true; + }); + + it('should correctly handle array header values where a single header is expected', async function() { + const { recordingId, server, persister } = this.polly; + + this.polly.record(); + + server.get(this.recordUrl()).once('beforeResponse', (req, res) => { + res.setHeaders({ + Location: ['./index.html'], + 'Content-Type': ['application/json'] + }); + }); + + await this.fetchRecord(); + await persister.persist(); + + const har = await persister.find(recordingId); + const { content, redirectURL } = har.log.entries[0].response; + + expect(await validate.har(har)).to.be.true; + expect(content.mimeType).to.equal('application/json'); + expect(redirectURL).to.equal('./index.html'); + }); + it('should error when persisting a failed request', async function() { let error; From 7808527c478da13726828a4f43b22b9a614acc74 Mon Sep 17 00:00:00 2001 From: offirgolan Date: Sat, 2 Feb 2019 23:13:12 -0800 Subject: [PATCH 3/3] docs: Update docs --- docs/server/request.md | 11 ++++++----- docs/server/response.md | 11 ++++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/docs/server/request.md b/docs/server/request.md index 209399fd..4258aa4c 100644 --- a/docs/server/request.md +++ b/docs/server/request.md @@ -6,10 +6,10 @@ Get a header with a given name. -| Param | Type | Description | -| ----------- | -------- | ---------------------- | -| name | `String` | The name of the header | -| **Returns** | `String` | The header value | +| Param | Type | Description | +| ----------- | ---------------- | ---------------------- | +| name | `String` | The name of the header | +| **Returns** | `String | Array` | The header value | **Example** @@ -25,7 +25,7 @@ removed. | Param | Type | Description | | ----------- | ------------------------- | ------------------------ | | name | `String` | The name of the header | -| value | `String` | The value for the header | +| value | `String | Array` | The value for the header | | **Returns** | [Request](server/request) | The current request | **Example** @@ -48,6 +48,7 @@ removed. ```js req.setHeaders({ + Accept: ['text/html', 'image/*'], 'Content-Type': 'application/json', 'Content-Length': 42 }); diff --git a/docs/server/response.md b/docs/server/response.md index 2631d13f..d45d4ee2 100644 --- a/docs/server/response.md +++ b/docs/server/response.md @@ -44,10 +44,10 @@ res.status(200); Get a header with a given name. -| Param | Type | Description | -| ----------- | -------- | ---------------------- | -| name | `String` | The name of the header | -| **Returns** | `String` | The header value | +| Param | Type | Description | +| ----------- | ---------------- | ---------------------- | +| name | `String` | The name of the header | +| **Returns** | `String | Array` | The header value | **Example** @@ -63,7 +63,7 @@ removed. | Param | Type | Description | | ----------- | --------------------------- | ------------------------ | | name | `String` | The name of the header | -| value | `String` | The value for the header | +| value | `String | Array` | The value for the header | | **Returns** | [Response](server/response) | The current response | **Example** @@ -86,6 +86,7 @@ removed. ```js res.setHeaders({ + Accept: ['text/html', 'image/*'], 'Content-Type': 'application/json', 'Content-Length': 42 });