Skip to content

Commit 9f045ae

Browse files
jonmilleCopilot
andauthored
MLE-30257 Replace encodeURI With encodeURIComponent (#1094)
* MLE-30257 Replace encodeURI With encodeURIComponent --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent a4735a0 commit 9f045ae

2 files changed

Lines changed: 141 additions & 36 deletions

File tree

lib/extlibs.js

Lines changed: 53 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015-2025 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
2+
* Copyright (c) 2015-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
33
*/
44
'use strict';
55
const requester = require('./requester.js');
@@ -33,6 +33,49 @@ function emptyOutputTransform(/*headers, data*/) {
3333
};
3434
}
3535

36+
/**
37+
* Safely encodes a caller-supplied extension library path for use in a REST
38+
* request URL. Each path segment is encoded individually with
39+
* encodeURIComponent(), preventing path-separator injection (CWE-22) and
40+
* query-parameter injection (CWE-20). Any leading /v1/ext/, /ext/, or bare /
41+
* prefix is stripped before splitting so that all three accepted input forms
42+
* (bare name, /path/name, /ext/path/name) produce the same canonical output.
43+
*
44+
* @param {string} rawPath - caller-supplied path or directory
45+
* @param {boolean} trailingSlash - when true, appends a trailing '/' (used by list())
46+
* @returns {string} the encoded path beginning with /v1/ext/
47+
* @throws {Error} if any segment equals '..' or '.'
48+
* @ignore
49+
*/
50+
function encodeExtPath(rawPath, trailingSlash) {
51+
if (typeof rawPath !== 'string' && !(rawPath instanceof String)) {
52+
throw new Error('extension library path must be a string');
53+
}
54+
55+
// Require a leading '/' before stripping any prefix so that undocumented
56+
// bare forms like 'ext/module.xqy' are treated as literal path segments
57+
// rather than silently having their 'ext/' prefix removed.
58+
const stripped = rawPath
59+
.replace(/^\/(?:(?:v1\/)?ext\/)?/, '')
60+
.replace(/\/$/, '');
61+
62+
const segments = stripped.length === 0 ? [] : stripped.split('/');
63+
64+
for (const seg of segments) {
65+
if (seg === '..' || seg === '.') {
66+
throw new Error(
67+
'extension library path must not contain relative path components (".", ".."): ' + rawPath
68+
);
69+
}
70+
}
71+
72+
// Build the result using a conditional join so that an empty segment list
73+
// does not produce a double slash (e.g. '/v1/ext//' for list('/ext/')).
74+
const encodedSegments = segments.map(s => encodeURIComponent(s));
75+
const base = '/v1/ext' + (encodedSegments.length > 0 ? '/' + encodedSegments.join('/') : '');
76+
return base + (trailingSlash ? '/' : '');
77+
}
78+
3679
/** @ignore */
3780
function ExtLibs(client) {
3881
if (!(this instanceof ExtLibs)) {
@@ -56,11 +99,7 @@ ExtLibs.prototype.read = function readExtensionLibrary(path) {
5699

57100
const requestOptions = mlutil.copyProperties(this.client.getConnectionParams());
58101
requestOptions.method = 'GET';
59-
requestOptions.path = encodeURI(
60-
(path.substr(0,5) === '/ext/') ? ('/v1'+path) :
61-
(path.substr(0,1) === '/') ? ('/v1/ext'+path) :
62-
('/v1/ext/'+path)
63-
);
102+
requestOptions.path = encodeExtPath(path);
64103

65104
const operation = new Operation(
66105
'read extension library', this.client, requestOptions, 'empty', 'single'
@@ -127,15 +166,13 @@ ExtLibs.prototype.write = function writeExtensionLibrary() {
127166
throw new Error('must specify the path, content type, and source when writing a extension library');
128167
}
129168

130-
let endpoint =
131-
(path.substr(0,5) === '/ext/') ? ('/v1'+path) :
132-
(path.substr(0,1) === '/') ? ('/v1/ext'+path) :
133-
('/v1/ext/'+path);
169+
const encodedPath = encodeExtPath(path);
134170

171+
let queryString = '';
135172
if (Array.isArray(permissions)) {
136173
let role = null;
137174
let capabilities = null;
138-
let j=null;
175+
let j = null;
139176
for (i=0; i < permissions.length; i++) {
140177
arg = permissions[i];
141178
role = arg['role-name'];
@@ -144,7 +181,8 @@ ExtLibs.prototype.write = function writeExtensionLibrary() {
144181
throw new Error('cannot set permissions from '+JSON.stringify(arg));
145182
}
146183
for (j=0; j < capabilities.length; j++) {
147-
endpoint += ((i === 0 && j=== 0) ? '?' : '&') + 'perm:'+role+'='+capabilities[j];
184+
queryString += ((queryString.length === 0) ? '?' : '&') +
185+
'perm:' + encodeURIComponent(role) + '=' + encodeURIComponent(capabilities[j]);
148186
}
149187
}
150188
}
@@ -154,7 +192,7 @@ ExtLibs.prototype.write = function writeExtensionLibrary() {
154192
requestOptions.headers = {
155193
'Content-Type': contentType
156194
};
157-
requestOptions.path = encodeURI(endpoint);
195+
requestOptions.path = encodedPath + queryString;
158196

159197
const operation = new Operation(
160198
'write extension library', this.client, requestOptions, 'single', 'empty'
@@ -180,11 +218,7 @@ ExtLibs.prototype.remove = function removeExtensionLibrary(path) {
180218

181219
const requestOptions = mlutil.copyProperties(this.client.getConnectionParams());
182220
requestOptions.method = 'DELETE';
183-
requestOptions.path = encodeURI(
184-
(path.substr(0,5) === '/ext/') ? ('/v1'+path) :
185-
(path.substr(0,1) === '/') ? ('/v1/ext'+path) :
186-
('/v1/ext/'+path)
187-
);
221+
requestOptions.path = encodeExtPath(path);
188222

189223
const operation = new Operation(
190224
'remove extension library', this.client, requestOptions, 'empty', 'empty'
@@ -215,24 +249,8 @@ ExtLibs.prototype.list = function listExtensionLibraries(directory) {
215249

216250
if (typeof directory !== 'string' && !(directory instanceof String)) {
217251
requestOptions.path = '/v1/ext';
218-
} else if (directory.substr(0,5) === '/ext/') {
219-
if (directory.substr(-1,1) === '/') {
220-
requestOptions.path = encodeURI(directory);
221-
} else {
222-
requestOptions.path = encodeURI(directory+'/');
223-
}
224252
} else {
225-
const hasInitialSlash = (directory.substr(0,1) === '/');
226-
const hasTrailingSlash = (directory.substr(-1,1) === '/');
227-
if (hasInitialSlash && hasTrailingSlash) {
228-
requestOptions.path = encodeURI('/v1/ext' + directory);
229-
} else if (hasTrailingSlash) {
230-
requestOptions.path = encodeURI('/v1/ext/' + directory);
231-
} else if (hasInitialSlash) {
232-
requestOptions.path = encodeURI('/v1/ext' + directory+'/');
233-
} else {
234-
requestOptions.path = encodeURI('/v1/ext/' + directory+'/');
235-
}
253+
requestOptions.path = encodeExtPath(directory, true);
236254
}
237255

238256
const operation = new Operation(

test-basic/extlibs.js

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015-2025 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
2+
* Copyright (c) 2015-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
33
*/
44
var should = require('should');
55

@@ -260,4 +260,91 @@ describe('extension libraries', function(){
260260
.catch(done);
261261
});
262262
});
263+
264+
describe('when handling path encoding security', function() {
265+
it('should throw a user-facing Error (not TypeError) when a non-string path is passed to read()', function() {
266+
(function() { restAdminDB.config.extlibs.read(42); }).should.throw(/must be a string/);
267+
(function() { restAdminDB.config.extlibs.read({}); }).should.throw(/must be a string/);
268+
});
269+
270+
it('should throw a user-facing Error (not TypeError) when a non-string path is passed to remove()', function() {
271+
(function() { restAdminDB.config.extlibs.remove(42); }).should.throw(/must be a string/);
272+
});
273+
274+
it('should throw a user-facing Error (not TypeError) when a non-string path is passed to write()', function() {
275+
(function() {
276+
restAdminDB.config.extlibs.write({ path: 42, contentType: 'application/xquery', source: Buffer.from('') });
277+
}).should.throw(/must be a string/);
278+
});
279+
280+
it('should reject a path traversal attempt via .. segments in read()', function() {
281+
(function() {
282+
restAdminDB.config.extlibs.read('../../../v1/databases');
283+
}).should.throw(/relative path components/);
284+
});
285+
286+
it('should reject a path traversal attempt via .. segments in write()', function() {
287+
(function() {
288+
restAdminDB.config.extlibs.write('../../../v1/databases', 'application/xquery', Buffer.from(''));
289+
}).should.throw(/relative path components/);
290+
});
291+
292+
it('should reject path traversal in the single-object form of write()', function() {
293+
(function() {
294+
restAdminDB.config.extlibs.write({
295+
path: '../../../v1/databases',
296+
contentType: 'application/xquery',
297+
source: Buffer.from('')
298+
});
299+
}).should.throw(/relative path components/);
300+
});
301+
302+
it('should reject a path traversal attempt via .. segments in remove()', function() {
303+
(function() {
304+
restAdminDB.config.extlibs.remove('../../secret');
305+
}).should.throw(/relative path components/);
306+
});
307+
308+
it('should reject a path traversal attempt via .. segments in list()', function() {
309+
(function() {
310+
restAdminDB.config.extlibs.list('../../admin');
311+
}).should.throw(/relative path components/);
312+
});
313+
314+
it('should produce a correctly percent-encoded path when a name contains a space', function(done) {
315+
var spacedPath = 'my lib/module.xqy';
316+
restAdminDB.config.extlibs.read(spacedPath)
317+
.result(function() { done(); },
318+
function(err) {
319+
// A 404/403 response from MarkLogic is acceptable — it confirms that
320+
// the percent-encoded URL was sent and understood by the server.
321+
// A client-side URL construction error is a different failure class.
322+
if (err && (err.statusCode === 404 || err.statusCode === 403)) { done(); } else { done(err); }
323+
});
324+
});
325+
326+
it('should produce the same encoded path for all three input forms', function(done) {
327+
// Calling read() is synchronous up to the point where it sets requestOptions.path;
328+
// we verify none of the three forms throw and that the operation is initiated.
329+
// (The network requests will all 404 since the module does not exist.)
330+
var promises = [
331+
restAdminDB.config.extlibs.read('my/module.xqy').result(),
332+
restAdminDB.config.extlibs.read('/my/module.xqy').result(),
333+
restAdminDB.config.extlibs.read('/ext/my/module.xqy').result()
334+
];
335+
Promise.allSettled(promises).then(function(results) {
336+
results.forEach(function(r) {
337+
// Each should either resolve or reject with a server status code (404),
338+
// never with a client-side TypeError or URL construction error.
339+
if (r.status === 'rejected') {
340+
if (!r.reason || !r.reason.statusCode) {
341+
done(new Error('Unexpected non-HTTP error: ' + r.reason));
342+
return;
343+
}
344+
}
345+
});
346+
done();
347+
});
348+
});
349+
});
263350
});

0 commit comments

Comments
 (0)