Skip to content

Commit

Permalink
fix: Correctly handle array header values (#179)
Browse files Browse the repository at this point in the history
* fix: Correctly handle array header values

* fix: Add `_fromType` and add test cases

* docs: Update docs
  • Loading branch information
offirgolan authored and jasonmit committed Feb 4, 2019
1 parent b13c053 commit fb7dbb4
Show file tree
Hide file tree
Showing 9 changed files with 163 additions and 32 deletions.
11 changes: 6 additions & 5 deletions docs/server/request.md
Original file line number Diff line number Diff line change
Expand Up @@ -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**

Expand All @@ -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**
Expand All @@ -48,6 +48,7 @@ removed.

```js
req.setHeaders({
Accept: ['text/html', 'image/*'],
'Content-Type': 'application/json',
'Content-Length': 42
});
Expand Down
11 changes: 6 additions & 5 deletions docs/server/response.md
Original file line number Diff line number Diff line change
Expand Up @@ -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**

Expand All @@ -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**
Expand All @@ -86,6 +86,7 @@ removed.

```js
res.setHeaders({
Accept: ['text/html', 'image/*'],
'Content-Type': 'application/json',
'Content-Length': 42
});
Expand Down
19 changes: 6 additions & 13 deletions packages/@pollyjs/adapter/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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) {
Expand Down
30 changes: 30 additions & 0 deletions packages/@pollyjs/adapter/src/utils/normalize-recorded-response.js
Original file line number Diff line number Diff line change
@@ -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, _fromType }) => {
const existingValue = accum[name];

if (existingValue) {
if (!isArray(existingValue)) {
accum[name] = [existingValue];
}

accum[name].push(value);
} else {
accum[name] = _fromType === 'array' ? [value] : value;
}

return accum;
}, {});
}
9 changes: 6 additions & 3 deletions packages/@pollyjs/persister/src/har/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import getByteLength from 'utf8-byte-length';
import setCookies from 'set-cookie-parser';

import toNVPairs from './utils/to-nv-pairs';
import getFirstHeader from './utils/get-first-header';

function headersSize(request) {
const keys = [];
Expand Down Expand Up @@ -34,7 +35,7 @@ export default class Request {

if (request.body) {
this.postData = {
mimeType: request.getHeader('Content-Type') || 'text/plain',
mimeType: getFirstHeader(request, 'Content-Type') || 'text/plain',
params: []
};

Expand All @@ -43,8 +44,10 @@ export default class Request {
}
}

if (request.hasHeader('Content-Length')) {
this.bodySize = parseInt(request.getHeader('Content-Length'), 10);
const contentLength = getFirstHeader(request, 'Content-Length');

if (contentLength) {
this.bodySize = parseInt(contentLength, 10);
} else {
this.bodySize =
this.postData && this.postData.text
Expand Down
13 changes: 8 additions & 5 deletions packages/@pollyjs/persister/src/har/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import getByteLength from 'utf8-byte-length';
import setCookies from 'set-cookie-parser';

import toNVPairs from './utils/to-nv-pairs';
import getFirstHeader from './utils/get-first-header';

function headersSize(response) {
const keys = [];
Expand All @@ -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 = getFirstHeader(response, 'Location') || '';

this.content = {
mimeType: response.getHeader('Content-Type') || 'text/plain'
mimeType: getFirstHeader(response, '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 = getFirstHeader(response, '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;
}
}
20 changes: 20 additions & 0 deletions packages/@pollyjs/persister/src/har/utils/get-first-header.js
Original file line number Diff line number Diff line change
@@ -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;
}
18 changes: 17 additions & 1 deletion packages/@pollyjs/persister/src/har/utils/to-nv-pairs.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,21 @@
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)) {
// _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 });
}

return pairs;
}, []);
}
64 changes: 64 additions & 0 deletions tests/integration/persister-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit fb7dbb4

Please sign in to comment.