ghost-storage-adapter-s3
ghost-storage-adapter-s3 copied to clipboard
Potential problem on stripEndingSlash function
Problem
Note the following code found in /src/index.js
const stripLeadingSlash = s => s.indexOf('/') === 0 ? s.substring(1) : s
const stripEndingSlash = s => s.indexOf('/') === (s.length - 1) ? s.substring(0, s.length - 1) : s
The expression s.indexOf("/")
returns the first index in the string. That means stripEndingSlash
will only strip ending slashes from strings with exactly one slash. A string containing two slashes will return with the trailing slash still in place.
Example
Consider the following cases:
const singleSlash = "index.js/"
// singleSlash.indexOf('/') equals 8
// 8 equals (singleSlash.length -1);
// Slash is removed
console.log(stripEndingSlash(singleSlash));
// "index.js"
const multiSlash = "/src/index.js/";
// multiSlash.indexOf('/') equals 0
// 0 doesn't equal (multiSlash.length -1)
// Slash is not removed
console.log(stripEndingSlash(multiSlash));
// "/src/index.js/"
Suggestion
You could fix this by changing indexOf
to lastIndexOf
, however, using charAt
provides a more readable and performant expression:
const stripLeadingSlash = s => s.charAt(0) === "/" ? s.substring(1) : s
const stripEndingSlash = s => s.charAt(s.length - 1) === "/" ? s.substring(0, s.length - 1) : s