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

Fix an issue where % was getting double encoded in query params #698

Merged
merged 1 commit into from
Apr 19, 2023
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
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, '%')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhwaneetbhatt Any reason to not encode { and } here anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VShingala In Java-unirest library, using characters { and } resulted in IllegalURLException being thrown by the library. We caught this now only because we added a Newman test. So we cannot have those characters in the URL. This is not a problem with other Java libraries (OkHttp) so seems like a bug with unirest.

.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