Skip to content

Commit dfac7ce

Browse files
author
dhwaneetbhatt
committed
curl - do not add HTTP method explicitly when not required
1 parent b13e4a5 commit dfac7ce

File tree

4 files changed

+269
-4
lines changed

4 files changed

+269
-4
lines changed

‎codegens/curl/lib/index.js‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ var sanitize = require('./util').sanitize,
33
getUrlStringfromUrlObject = require('./util').getUrlStringfromUrlObject,
44
addFormParam = require('./util').addFormParam,
55
form = require('./util').form,
6+
shouldAddHttpMethod = require('./util').shouldAddHttpMethod,
67
_ = require('./lodash'),
78
self;
89

@@ -44,12 +45,14 @@ self = module.exports = {
4445
else {
4546
indent = ' ';
4647
}
48+
4749
if (request.method === 'HEAD') {
48-
snippet += ` ${form('-I', format)} ${quoteType + url + quoteType}`;
50+
snippet += ` ${form('-I', format)}`;
4951
}
50-
else {
51-
snippet += ` ${form('-X', format)} ${request.method} ${quoteType + url + quoteType}`;
52+
if (shouldAddHttpMethod(request, options)) {
53+
snippet += ` ${form('-X', format)} ${request.method}`;
5254
}
55+
snippet += ` ${quoteType + url + quoteType}`;
5356

5457
if (request.body && !request.headers.has('Content-Type')) {
5558
if (request.body.mode === 'file') {
@@ -236,6 +239,13 @@ self = module.exports = {
236239
default: true,
237240
description: 'Automatically follow HTTP redirects'
238241
},
242+
{
243+
name: 'Follow original HTTP method',
244+
id: 'followOriginalHttpMethod',
245+
type: 'boolean',
246+
default: false,
247+
description: 'Use the original HTTP method when following redirects'
248+
},
239249
{
240250
name: 'Trim request body fields',
241251
id: 'trimRequestBody',

‎codegens/curl/lib/util.js‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
const _ = require('./lodash');
2+
13
var self = module.exports = {
24
/**
35
* sanitizes input string by handling escape characters eg: converts '''' to '\'\'', (" to \" and \ to \\ )
@@ -199,5 +201,77 @@ var self = module.exports = {
199201
contentType: contentType
200202
});
201203
}
204+
},
205+
206+
/**
207+
* @param {Object} body
208+
* @returns {boolean}
209+
*
210+
* Determines if a request body is actually empty.
211+
* This is needed because body.isEmpty() returns false for formdata
212+
* and urlencoded when they contain only disabled params which will not
213+
* be a part of the curl request.
214+
*/
215+
isBodyEmpty (body) {
216+
if (!body) {
217+
return true;
218+
}
219+
220+
if (body.isEmpty()) {
221+
return true;
222+
}
223+
224+
if (body.mode === 'formdata' || body.mode === 'urlencoded') {
225+
let memberCount = 0;
226+
body[body.mode].members && body[body.mode].members.forEach((param) => {
227+
if (!param.disabled) {
228+
memberCount += 1;
229+
}
230+
});
231+
232+
return memberCount === 0;
233+
}
234+
235+
return false;
236+
},
237+
238+
/**
239+
* Decide whether we should add the HTTP method explicitly to the cURL command.
240+
*
241+
* @param {Object} request
242+
* @param {Object} options
243+
*
244+
* @returns {Boolean}
245+
*/
246+
shouldAddHttpMethod: function (request, options) {
247+
const followOriginalHttpMethod =
248+
_.get(request, 'protocolProfileBehavior.followOriginalHttpMethod', options.followOriginalHttpMethod),
249+
disableBodyPruning = _.get(request, 'protocolProfileBehavior.disableBodyPruning', true),
250+
isBodyEmpty = self.isBodyEmpty(request.body);
251+
252+
if (options.followRedirect && followOriginalHttpMethod) {
253+
return true;
254+
}
255+
256+
switch (request.method) {
257+
case 'HEAD':
258+
return false;
259+
case 'GET':
260+
// disableBodyPruning will generally not be present in the request
261+
// the only time it will be present, its value will be _false_
262+
// i.e. the user wants to prune the request body despite it being present
263+
if (!isBodyEmpty && disableBodyPruning) {
264+
return true;
265+
}
266+
267+
return false;
268+
case 'POST':
269+
return isBodyEmpty;
270+
case 'DELETE':
271+
case 'PUT':
272+
case 'PATCH':
273+
default:
274+
return true;
275+
}
202276
}
203277
};

‎codegens/curl/test/unit/convert.test.js‎

Lines changed: 175 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ describe('curl convert function', function () {
277277
expect.fail(null, null, error);
278278
}
279279
expect(snippet).to.be.a('string');
280-
expect(snippet).to.include("GET 'https://google.com'"); // eslint-disable-line quotes
280+
expect(snippet).to.include("'https://google.com'"); // eslint-disable-line quotes
281281
});
282282
});
283283

@@ -635,5 +635,179 @@ describe('curl convert function', function () {
635635
expect(outputUrlString).to.equal(rawUrl);
636636
});
637637
});
638+
639+
it('should not add --request parameter in POST request if body is present', function () {
640+
var request = new sdk.Request({
641+
'method': 'POST',
642+
'header': [],
643+
'body': {
644+
'mode': 'graphql',
645+
'graphql': {
646+
'query': '{\n findScenes(\n filter: {per_page: 0}\n scene_filter: {is_missing: "performers"}){\n count\n scenes {\n id\n title\n path\n }\n }\n}', // eslint-disable-line
647+
'variables': '{\n\t"variable_key": "variable_value"\n}'
648+
}
649+
},
650+
'url': {
651+
'raw': 'https://postman-echo.com/post',
652+
'protocol': 'https',
653+
'host': [
654+
'postman-echo',
655+
'com'
656+
],
657+
'path': [
658+
'post'
659+
]
660+
}
661+
});
662+
663+
convert(request, { followRedirect: true }, function (error, snippet) {
664+
if (error) {
665+
expect.fail(null, null, error);
666+
}
667+
expect(snippet).to.be.a('string');
668+
expect(snippet).to.not.include('--request POST');
669+
});
670+
});
671+
672+
it('should add --request parameter in POST request if body is not present', function () {
673+
var request = new sdk.Request({
674+
'method': 'POST',
675+
'header': [],
676+
'url': {
677+
'raw': 'https://postman-echo.com/post',
678+
'protocol': 'https',
679+
'host': [
680+
'postman-echo',
681+
'com'
682+
],
683+
'path': [
684+
'post'
685+
]
686+
}
687+
});
688+
689+
convert(request, { followRedirect: true }, function (error, snippet) {
690+
if (error) {
691+
expect.fail(null, null, error);
692+
}
693+
expect(snippet).to.be.a('string');
694+
expect(snippet).to.include('--request POST');
695+
});
696+
});
697+
698+
it('should add --request parameter in GET request if body is present', function () {
699+
var request = new sdk.Request({
700+
'method': 'GET',
701+
'header': [],
702+
'body': {
703+
'mode': 'graphql',
704+
'graphql': {
705+
'query': '{\n findScenes(\n filter: {per_page: 0}\n scene_filter: {is_missing: "performers"}){\n count\n scenes {\n id\n title\n path\n }\n }\n}', // eslint-disable-line
706+
'variables': '{\n\t"variable_key": "variable_value"\n}'
707+
}
708+
},
709+
'url': {
710+
'raw': 'https://postman-echo.com/get',
711+
'protocol': 'https',
712+
'host': [
713+
'postman-echo',
714+
'com'
715+
],
716+
'path': [
717+
'get'
718+
]
719+
}
720+
});
721+
722+
convert(request, { followRedirect: true }, function (error, snippet) {
723+
if (error) {
724+
expect.fail(null, null, error);
725+
}
726+
expect(snippet).to.be.a('string');
727+
expect(snippet).to.include('--request GET');
728+
});
729+
});
730+
731+
it('should not add --request parameter in GET request if body is present ' +
732+
'but disableBodyPruning is false', function () {
733+
const request = new sdk.Request({
734+
'method': 'GET',
735+
'header': [],
736+
'body': {
737+
'mode': 'graphql',
738+
'graphql': {
739+
'query': '{\n findScenes(\n filter: {per_page: 0}\n scene_filter: {is_missing: "performers"}){\n count\n scenes {\n id\n title\n path\n }\n }\n}', // eslint-disable-line
740+
'variables': '{\n\t"variable_key": "variable_value"\n}'
741+
}
742+
},
743+
'url': {
744+
'raw': 'https://postman-echo.com/get',
745+
'protocol': 'https',
746+
'host': [
747+
'postman-echo',
748+
'com'
749+
],
750+
'path': [
751+
'get'
752+
]
753+
}
754+
});
755+
756+
// this needs to be done here because protocolProfileBehavior is not in collections SDK
757+
request.protocolProfileBehavior = {
758+
disableBodyPruning: false
759+
};
760+
761+
convert(request, { followRedirect: true }, function (error, snippet) {
762+
if (error) {
763+
expect.fail(null, null, error);
764+
}
765+
expect(snippet).to.be.a('string');
766+
expect(snippet).to.not.include('--request GET');
767+
});
768+
});
769+
770+
it('should add --request parameter when options ' +
771+
'followRedirect and followOriginalHttpMethod are true', function () {
772+
const methods = ['GET', 'HEAD', 'DELETE', 'PUT', 'POST', 'PATCH'],
773+
request = new sdk.Request({
774+
'method': 'POST',
775+
'header': [],
776+
'body': {
777+
'mode': 'graphql',
778+
'graphql': {
779+
'query': '{\n findScenes(\n filter: {per_page: 0}\n scene_filter: {is_missing: "performers"}){\n count\n scenes {\n id\n title\n path\n }\n }\n}', // eslint-disable-line
780+
'variables': '{\n\t"variable_key": "variable_value"\n}'
781+
}
782+
},
783+
'url': {
784+
'raw': 'https://postman-echo.com/post',
785+
'protocol': 'https',
786+
'host': [
787+
'postman-echo',
788+
'com'
789+
],
790+
'path': [
791+
'post'
792+
]
793+
}
794+
});
795+
796+
// this needs to be done here because protocolProfileBehavior is not in collections SDK
797+
request.protocolProfileBehavior = {
798+
followOriginalHttpMethod: true
799+
};
800+
801+
for (let method of methods) {
802+
request.method = method;
803+
convert(request, { followRedirect: true }, function (error, snippet) {
804+
if (error) {
805+
expect.fail(null, null, error);
806+
}
807+
expect(snippet).to.be.a('string');
808+
expect(snippet).to.include(`--request ${method}`);
809+
});
810+
}
811+
});
638812
});
639813
});

‎test/codegen/structure.test.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,12 @@ const expectedOptions = {
5757
default: true,
5858
description: 'Automatically follow HTTP redirects'
5959
},
60+
followOriginalHttpMethod: {
61+
name: 'Follow original HTTP method',
62+
type: 'boolean',
63+
default: false,
64+
description: 'Use the original HTTP method when following redirects'
65+
},
6066
trimRequestBody: {
6167
name: 'Trim request body fields',
6268
type: 'boolean',
@@ -95,6 +101,7 @@ const expectedOptions = {
95101
'silent',
96102
'includeBoilerplate',
97103
'followRedirect',
104+
'followOriginalHttpMethod',
98105
'lineContinuationCharacter',
99106
'protocol',
100107
'useMimeType',

0 commit comments

Comments
 (0)