Commit 2fc7c9b9 authored by Paul Slaughter's avatar Paul Slaughter Committed by Natalia Tepluhina

Replace Node `joinPaths` with custom impl

**Why?**
The current implementation of `joinPaths` comes from the
Node core modules `path` and has a number of caveats:

- Does not support non-string values
- Does not handle joining absolute URL's

https://nodejs.org/api/path.html#path_path_join_paths

**Note:**
The new impl of `joinPaths` does not do the same special
normalization which the Node version did. This should be
safe because I'm pretty sure we only use this method to
join paths with `/` that potentially start and end with
a `/`.
parent 1490f0f0
import { join as joinPaths } from 'path'; const PATH_SEPARATOR = '/';
const PATH_SEPARATOR_LEADING_REGEX = new RegExp(`^${PATH_SEPARATOR}+`);
const PATH_SEPARATOR_ENDING_REGEX = new RegExp(`${PATH_SEPARATOR}+$`);
// Returns a decoded url parameter value // Returns a decoded url parameter value
// - Treats '+' as '%20' // - Treats '+' as '%20'
...@@ -6,6 +8,37 @@ function decodeUrlParameter(val) { ...@@ -6,6 +8,37 @@ function decodeUrlParameter(val) {
return decodeURIComponent(val.replace(/\+/g, '%20')); return decodeURIComponent(val.replace(/\+/g, '%20'));
} }
function cleanLeadingSeparator(path) {
return path.replace(PATH_SEPARATOR_LEADING_REGEX, '');
}
function cleanEndingSeparator(path) {
return path.replace(PATH_SEPARATOR_ENDING_REGEX, '');
}
/**
* Safely joins the given paths which might both start and end with a `/`
*
* Example:
* - `joinPaths('abc/', '/def') === 'abc/def'`
* - `joinPaths(null, 'abc/def', 'zoo) === 'abc/def/zoo'`
*
* @param {...String} paths
* @returns {String}
*/
export function joinPaths(...paths) {
return paths.reduce((acc, path) => {
if (!path) {
return acc;
}
if (!acc) {
return path;
}
return [cleanEndingSeparator(acc), PATH_SEPARATOR, cleanLeadingSeparator(path)].join('');
}, '');
}
// Returns an array containing the value(s) of the // Returns an array containing the value(s) of the
// of the key passed as an argument // of the key passed as an argument
export function getParameterValues(sParam, url = window.location) { export function getParameterValues(sParam, url = window.location) {
...@@ -212,5 +245,3 @@ export function objectToQuery(obj) { ...@@ -212,5 +245,3 @@ export function objectToQuery(obj) {
.map(k => `${encodeURIComponent(k)}=${encodeURIComponent(obj[k])}`) .map(k => `${encodeURIComponent(k)}=${encodeURIComponent(obj[k])}`)
.join('&'); .join('&');
} }
export { joinPaths };
...@@ -298,4 +298,28 @@ describe('URL utility', () => { ...@@ -298,4 +298,28 @@ describe('URL utility', () => {
expect(urlUtils.objectToQuery(searchQueryObject)).toEqual('one=1&two=2'); expect(urlUtils.objectToQuery(searchQueryObject)).toEqual('one=1&two=2');
}); });
}); });
describe('joinPaths', () => {
it.each`
paths | expected
${['foo', 'bar']} | ${'foo/bar'}
${['foo/', 'bar']} | ${'foo/bar'}
${['foo//', 'bar']} | ${'foo/bar'}
${['abc/', '/def']} | ${'abc/def'}
${['foo', '/bar']} | ${'foo/bar'}
${['foo', '/bar/']} | ${'foo/bar/'}
${['foo', '//bar/']} | ${'foo/bar/'}
${['foo', '', '/bar']} | ${'foo/bar'}
${['foo', '/bar', '']} | ${'foo/bar'}
${['/', '', 'foo/bar/ ', '', '/ninja']} | ${'/foo/bar/ /ninja'}
${['', '/ninja', '/', ' ', '', 'bar', ' ']} | ${'/ninja/ /bar/ '}
${['http://something/bar/', 'foo']} | ${'http://something/bar/foo'}
${['foo/bar', null, 'ninja', null]} | ${'foo/bar/ninja'}
${[null, 'abc/def', 'zoo']} | ${'abc/def/zoo'}
${['', '', '']} | ${''}
${['///', '/', '//']} | ${'/'}
`('joins paths $paths => $expected', ({ paths, expected }) => {
expect(urlUtils.joinPaths(...paths)).toBe(expected);
});
});
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment