Skip to content

Commit

Permalink
Fix an issue where % was getting double encoded in query params
Browse files Browse the repository at this point in the history
  • Loading branch information
Dhwaneet Bhatt committed Apr 18, 2023
1 parent 989648d commit c076f20
Show file tree
Hide file tree
Showing 25 changed files with 542 additions and 54 deletions.
3 changes: 2 additions & 1 deletion codegens/curl/lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ var self = module.exports = {
},

/**
* Encode param except the following characters- [,{,},]
* Encode param except the following characters- [,{,},],%
*
* @param {String} param
* @returns {String}
Expand All @@ -218,6 +218,7 @@ var self = module.exports = {
.replace(/%7B/g, '{')
.replace(/%5D/g, ']')
.replace(/%7D/g, '}')
.replace(/%25/g, '%')
.replace(/'/g, '%27');
},

Expand Down
7 changes: 7 additions & 0 deletions codegens/curl/test/unit/convert.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,13 @@ describe('curl convert function', function () {
expect(outputUrlString).to.equal('https://postman-echo.com/get?key1={{value}}&key2=%27a%20b%20c%27');
});

it('should not encode query params that are already encoded', function () {
rawUrl = 'https://postman-echo.com/get?query=urn%3Ali%3Afoo%3A62324';
urlObject = new sdk.Url(rawUrl);
outputUrlString = getUrlStringfromUrlObject(urlObject);
expect(outputUrlString).to.equal('https://postman-echo.com/get?query=urn%3Ali%3Afoo%3A62324');
});

it('should discard disabled query params', function () {
urlObject = new sdk.Url({
protocol: 'https',
Expand Down
3 changes: 2 additions & 1 deletion codegens/golang/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ var _ = require('./lodash'),
sanitizeMultiline = require('./util').sanitizeMultiline,
sanitizeOptions = require('./util').sanitizeOptions,
addFormParam = require('./util').addFormParam,
getUrlStringfromUrlObject = require('./util').getUrlStringfromUrlObject,
isFile = false,
self;

Expand Down Expand Up @@ -243,7 +244,7 @@ self = module.exports = {
}
codeSnippet += `${indent}"net/http"\n${indent}"io/ioutil"\n)\n\n`;

codeSnippet += `func main() {\n\n${indent}url := "${encodeURI(request.url.toString())}"\n`;
codeSnippet += `func main() {\n\n${indent}url := "${getUrlStringfromUrlObject(request.url)}"\n`;
codeSnippet += `${indent}method := "${request.method}"\n\n`;

if (bodySnippet !== '') {
Expand Down
116 changes: 101 additions & 15 deletions codegens/golang/lib/util.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
module.exports = {
const _ = require('./lodash');

const self = module.exports = {
/**
* sanitizes input string by handling escape characters eg: converts '''' to '\'\''
* and trim input if required
*
* @param {String} inputString
* @param {Boolean} [trim] - indicates whether to trim string or not
* @returns {String}
*/
* sanitizes input string by handling escape characters eg: converts '''' to '\'\''
* and trim input if required
*
* @param {String} inputString
* @param {Boolean} [trim] - indicates whether to trim string or not
* @returns {String}
*/
sanitize: function (inputString, trim) {
if (typeof inputString !== 'string') {
return '';
Expand All @@ -20,13 +22,13 @@ module.exports = {
},

/**
* sanitizes input string by handling escape characters eg: converts '''' to '\'\''
* and trim input if required
*
* @param {String} inputString
* @param {Boolean} [trim] - indicates whether to trim string or not
* @returns {String}
*/
* sanitizes input string by handling escape characters eg: converts '''' to '\'\''
* and trim input if required
*
* @param {String} inputString
* @param {Boolean} [trim] - indicates whether to trim string or not
* @returns {String}
*/
sanitizeMultiline: function (inputString, trim) {
if (typeof inputString !== 'string') {
return '';
Expand All @@ -38,6 +40,90 @@ module.exports = {

},

/**
*
* @param {Object} urlObject The request sdk request.url object
* @returns {String} The final string after parsing all the parameters of the url including
* protocol, auth, host, port, path, query, hash
* This will be used because the url.toString() method returned the URL with non encoded query string
* and hence a manual call is made to getQueryString() method with encode option set as true.
*/
getUrlStringfromUrlObject: function (urlObject) {
var url = '';
if (!urlObject) {
return url;
}
if (urlObject.protocol) {
url += (urlObject.protocol.endsWith('://') ? urlObject.protocol : urlObject.protocol + '://');
}
if (urlObject.auth && urlObject.auth.user) {
url = url + ((urlObject.auth.password) ?
urlObject.auth.user + ':' + urlObject.auth.password : urlObject.auth.user) + '@';
}
if (urlObject.host) {
url += urlObject.getHost();
}
if (urlObject.port) {
url += ':' + urlObject.port.toString();
}
if (urlObject.path) {
url += urlObject.getPath();
}
if (urlObject.query && urlObject.query.count()) {
let queryString = self.getQueryString(urlObject);
queryString && (url += '?' + queryString);
}
if (urlObject.hash) {
url += '#' + urlObject.hash;
}

return self.sanitize(url, false);
},

/**
* @param {Object} urlObject
* @returns {String}
*/
getQueryString: function (urlObject) {
let isFirstParam = true,
params = _.get(urlObject, 'query.members'),
result = '';
if (Array.isArray(params)) {
result = _.reduce(params, function (result, param) {
if (param.disabled === true) {
return result;
}

if (isFirstParam) {
isFirstParam = false;
}
else {
result += '&';
}

return result + self.encodeParam(param.key) + '=' + self.encodeParam(param.value);
}, result);
}

return result;
},

/**
* Encode param except the following characters- [,{,},],%
*
* @param {String} param
* @returns {String}
*/
encodeParam: function (param) {
return encodeURIComponent(param)
.replace(/%5B/g, '[')
.replace(/%7B/g, '{')
.replace(/%5D/g, ']')
.replace(/%7D/g, '}')
.replace(/%25/g, '%')
.replace(/'/g, '%27');
},

/**
* sanitizes input options
*
Expand Down
2 changes: 1 addition & 1 deletion codegens/java-okhttp/lib/okhttp.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ function convert (request, options, callback) {
if (options.includeBoilerplate) {
headerSnippet = 'import java.io.*;\n' +
'import okhttp3.*;\n' +
'public class main {\n' +
'public class Main {\n' +
indentString + 'public static void main(String []args) throws IOException{\n';
footerSnippet = indentString.repeat(2) + 'System.out.println(response.body().string());\n' +
indentString + '}\n}\n';
Expand Down
6 changes: 3 additions & 3 deletions codegens/java-okhttp/test/newman/newman.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ var runNewmanTest = require('../../../../test/codegen/newman/newmanTestUtil').ru
describe.skip('convert for different request types', function () {
var options = {indentCount: 3, indentType: 'Space', includeBoilerplate: true},
testConfig = {
compileScript: 'javac -cp *: main.java',
runScript: 'java -cp *: main',
fileName: 'main.java',
compileScript: 'javac -cp *: Main.java',
runScript: 'java -cp *: Main',
fileName: 'Main.java',
skipCollections: ['redirectCollection']
};
runNewmanTest(convert, options, testConfig);
Expand Down
4 changes: 2 additions & 2 deletions codegens/java-okhttp/test/unit/convert.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('okhttp convert function', function () {
}
snippetArray = snippet.split('\n');
for (var i = 0; i < snippetArray.length; i++) {
if (snippetArray[i].startsWith('public class main {')) {
if (snippetArray[i].startsWith('public class Main {')) {
expect(snippetArray[i + 1].substr(0, 4)).to.equal(SINGLE_SPACE.repeat(4));
expect(snippetArray[i + 1].charAt(4)).to.not.equal(SINGLE_SPACE);
}
Expand All @@ -39,7 +39,7 @@ describe('okhttp convert function', function () {
expect.fail(null, null, error);
return;
}
expect(snippet).to.include('import java.io.*;\nimport okhttp3.*;\npublic class main {\n');
expect(snippet).to.include('import java.io.*;\nimport okhttp3.*;\npublic class Main {\n');
});
});

Expand Down
5 changes: 2 additions & 3 deletions codegens/java-unirest/lib/parseRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,16 @@ var _ = require('./lodash'),
sanitize = require('./util').sanitize;

/**
* Encode param except the following characters- [,{,},]
* Encode param except the following characters- [,{,},],%
*
* @param {String} param
* @returns {String}
*/
function encodeParam (param) {
return encodeURIComponent(param)
.replace(/%5B/g, '[')
.replace(/%7B/g, '{')
.replace(/%5D/g, ']')
.replace(/%7D/g, '}')
.replace(/%25/g, '%')
.replace(/'/g, '%27');
}

Expand Down
2 changes: 1 addition & 1 deletion codegens/java-unirest/lib/unirest.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ function convert (request, options, callback) {
if (options.includeBoilerplate) {
headerSnippet = 'import com.mashape.unirest.http.*;\n' +
'import java.io.*;\n' +
'public class main {\n' +
'public class Main {\n' +
indentString + 'public static void main(String []args) throws Exception{\n';
footerSnippet = indentString.repeat(2) + 'System.out.println(response.getBody());\n' +
indentString + '}\n}\n';
Expand Down
6 changes: 3 additions & 3 deletions codegens/java-unirest/test/newman/newman.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ var runNewmanTest = require('../../../../test/codegen/newman/newmanTestUtil').ru

describe('Convert for different types of request', function () {
var testConfig = {
runScript: 'java -cp *: main',
compileScript: 'javac -cp *: main.java',
fileName: 'main.java',
runScript: 'java -cp *: Main',
compileScript: 'javac -cp *: Main.java',
fileName: 'Main.java',
skipCollections: ['formdataCollection', 'emptyFormdataCollection', 'unsupportedMethods']
},
options = {includeBoilerplate: true};
Expand Down
21 changes: 14 additions & 7 deletions codegens/java-unirest/test/unit/convert.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ describe('java unirest convert function for test collection', function () {
};
headerSnippet = 'import com.mashape.unirest.http.*;\n' +
'import java.io.*;\n' +
'public class main {\n' +
'public class Main {\n' +
indentString + 'public static void main(String []args) throws Exception{\n';
footerSnippet = indentString.repeat(2) + 'System.out.println(response.getBody());\n' +
indentString + '}\n}\n';
Expand Down Expand Up @@ -218,8 +218,8 @@ describe('java unirest convert function for test collection', function () {
expect.fail(null, null, error);
}
expect(snippet).to.be.a('string');
expect(snippet).to.include('http://postman-echo.com/post?a={{xyz}}');
expect(snippet).to.not.include('http://postman-echo.com/post?a=%7B%7Bxyz%7D%7D');
expect(snippet).to.not.include('http://postman-echo.com/post?a={{xyz}}');
expect(snippet).to.include('http://postman-echo.com/post?a=%7B%7Bxyz%7D%7D');
});
});

Expand Down Expand Up @@ -479,8 +479,8 @@ describe('java unirest convert function for test collection', function () {
rawUrl = 'https://postman-echo.com/get?key={{value}}';
urlObject = new sdk.Url(rawUrl);
outputUrlString = getUrlStringfromUrlObject(urlObject);
expect(outputUrlString).to.not.include('key=%7B%7Bvalue%7B%7B');
expect(outputUrlString).to.equal(rawUrl);
expect(outputUrlString).to.include('key=%7B%7Bvalue%7D%7D');
expect(outputUrlString).to.equal('https://postman-echo.com/get?key=%7B%7Bvalue%7D%7D');
});

it('should encode query params other than unresolved variables', function () {
Expand All @@ -491,14 +491,21 @@ describe('java unirest convert function for test collection', function () {
expect(outputUrlString).to.equal('https://postman-echo.com/get?key=%27a%20b%20c%27');
});

it('should not encode query params that are already encoded', function () {
rawUrl = 'https://postman-echo.com/get?query=urn%3Ali%3Afoo%3A62324';
urlObject = new sdk.Url(rawUrl);
outputUrlString = getUrlStringfromUrlObject(urlObject);
expect(outputUrlString).to.equal('https://postman-echo.com/get?query=urn%3Ali%3Afoo%3A62324');
});

it('should not encode unresolved query params and ' +
'encode every other query param, both present together', function () {
rawUrl = 'https://postman-echo.com/get?key1={{value}}&key2=\'a b c\'';
urlObject = new sdk.Url(rawUrl);
outputUrlString = getUrlStringfromUrlObject(urlObject);
expect(outputUrlString).to.not.include('key1=%7B%7Bvalue%7B%7B');
expect(outputUrlString).to.include('key1=%7B%7Bvalue%7D%7D');
expect(outputUrlString).to.not.include('key2=\'a b c\'');
expect(outputUrlString).to.equal('https://postman-echo.com/get?key1={{value}}&key2=%27a%20b%20c%27');
expect(outputUrlString).to.equal('https://postman-echo.com/get?key1=%7B%7Bvalue%7D%7D&key2=%27a%20b%20c%27');
});

it('should discard disabled query params', function () {
Expand Down
3 changes: 2 additions & 1 deletion codegens/js-xhr/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ var _ = require('./lodash'),
sanitize = require('./util').sanitize,
sanitizeOptions = require('./util').sanitizeOptions,
addFormParam = require('./util').addFormParam,
getUrlStringfromUrlObject = require('./util').getUrlStringfromUrlObject,
path = require('path');

/**
Expand Down Expand Up @@ -278,7 +279,7 @@ function convert (request, options, callback) {
codeSnippet += `${indent.repeat(2)}console.log(this.responseText);\n`;
codeSnippet += `${indent}}\n});\n\n`;

codeSnippet += `xhr.open("${request.method}", "${encodeURI(request.url.toString())}");\n`;
codeSnippet += `xhr.open("${request.method}", "${getUrlStringfromUrlObject(request.url)}");\n`;
if (options.requestTimeout) {
codeSnippet += `xhr.timeout = ${options.requestTimeout};\n`;
codeSnippet += 'xhr.addEventListener("ontimeout", function(e) {\n';
Expand Down
Loading

0 comments on commit c076f20

Please sign in to comment.