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(core,legacy-scripting-runner): allow GET requests to have body #195

Merged
merged 2 commits into from
Apr 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const coerceBody = req => {
const contentType = getContentType(req.headers || {});

// No need for body on get
if (req.method === 'GET') {
if (req.method === 'GET' && !req.allowGetBody) {
delete req.body;
}

Expand Down
62 changes: 58 additions & 4 deletions packages/core/src/tools/fetch.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,60 @@
'use strict';

// hacky "clone" for fetch so we don't pollute the global library
const fetch = require('node-fetch').bind({});
fetch.Promise = require('./promise');
module.exports = fetch;
const fetch = require('node-fetch');

// XXX: PatchedRequest is to get past node-fetch's check that forbids GET requests
// from having a body here:
// https://github.com/node-fetch/node-fetch/blob/v2.6.0/src/request.js#L75-L78
class PatchedRequest extends fetch.Request {
constructor(url, opts) {
const origMethod = (opts.method || 'GET').toUpperCase();

const isGetWithBody =
(origMethod === 'GET' || origMethod === 'HEAD') && opts.body;
let newOpts = opts;
if (isGetWithBody) {
// Temporary remove body to fool fetch.Request constructor
newOpts = { ...opts, body: null };
}

super(url, newOpts);

this._isGetWithBody = isGetWithBody;

if (isGetWithBody) {
// Restore the body. The body is stored internally using a Symbol key. We
// can't just do this[Symbol('Body internals')] as the symbol is internal.
// We need to use Object.getOwnPropertySymbols() to get all the keys and
// find the one that holds the body.
const keys = Object.getOwnPropertySymbols(this);
for (const k of keys) {
if (this[k].body !== undefined) {
this[k].body = Buffer.from(String(opts.body));
break;
}
}
}
}

get body() {
if (this._isGetWithBody && !this._bodyCalled) {
// This assumes node-fetch's check that disallows a GET request to have a
// body happens on the first time it calls this.body. Might not work if
// node-fetch breaks the assumption.
this._bodyCalled = true;
return null;
}
return super.body;
}
}

const newFetch = (url, opts) => {
const request = new PatchedRequest(url, opts);
// fetch actually accepts a Request object as an argument. It'll clone the
// request internally, that's why the PatchedRequest.body hack works.
return fetch(request);
};

newFetch.Promise = require('./promise');

module.exports = newFetch;
29 changes: 29 additions & 0 deletions packages/core/test/create-request-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,35 @@ describe('request client', () => {
});
});

it('should delete GET body by default', async () => {
const request = createAppRequestClient(input);
const response = await request({
// Use postman-echo because httpbin doesn't echo GET body
method: 'GET',
url: 'https://postman-echo.com/get',
body: {
name: 'Darth Vader'
}
});
response.status.should.eql(200);
response.json.args.should.deepEqual({});
});

it('should allow GET with body', async () => {
const request = createAppRequestClient(input);
const response = await request({
// Use postman-echo because httpbin doesn't echo GET body
method: 'GET',
url: 'https://postman-echo.com/get',
body: {
name: 'Darth Vader'
},
allowGetBody: true
});
response.status.should.eql(200);
response.json.args.should.deepEqual({ name: 'Darth Vader' });
});

describe('adds query params', () => {
it('should replace remaining curly params with empty string by default', () => {
const request = createAppRequestClient(input);
Expand Down
1 change: 1 addition & 0 deletions packages/legacy-scripting-runner/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,7 @@ const legacyScriptingRunner = (Zap, zcli, input) => {
}

request.headers = cleanHeaders(request.headers);
request.allowGetBody = true;

const response = await zcli.request(request);

Expand Down
10 changes: 10 additions & 0 deletions packages/legacy-scripting-runner/test/example-app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,16 @@ const legacyScriptingSource = `
return bundle.request;
},

movie_pre_poll_GET_with_body: function(bundle) {
// Use postman-echo because httpbin doesn't echo a GET request's body
bundle.request.url = 'https://postman-echo.com/get';
bundle.request.method = 'GET';
bundle.request.data = JSON.stringify({
name: 'Luke Skywalker'
});
return bundle.request;
},

movie_post_poll_request_options: function(bundle) {
// To make sure bundle.request is still available in post_poll
return [bundle.request];
Expand Down
24 changes: 24 additions & 0 deletions packages/legacy-scripting-runner/test/integration-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,30 @@ describe('Integration Test', () => {
});
});

it('KEY_pre_poll, GET with body', () => {
const appDef = _.cloneDeep(appDefinition);
appDef.legacy.scriptingSource = appDef.legacy.scriptingSource.replace(
'movie_pre_poll_GET_with_body',
'movie_pre_poll'
);
appDef.legacy.scriptingSource = appDef.legacy.scriptingSource.replace(
'movie_post_poll_make_array',
'movie_post_poll'
);
const _appDefWithAuth = withAuth(appDef, apiKeyAuth);
const _compiledApp = schemaTools.prepareApp(_appDefWithAuth);
const _app = createApp(_appDefWithAuth);

const input = createTestInput(
_compiledApp,
'triggers.movie.operation.perform'
);
return _app(input).then(output => {
const echoed = output.results[0];
should.equal(echoed.args.name, 'Luke Skywalker');
});
});

it('KEY_post_poll, jQuery utils', () => {
const input = createTestInput(
compiledApp,
Expand Down